feat: add enforcement gates, spec artifact linter, and workflow verification steps#69
Open
feat: add enforcement gates, spec artifact linter, and workflow verification steps#69
Conversation
…ication steps Convert ~26 advisory behavioral mechanisms into deterministic enforcement: new lint-spec-artifacts.py (checkbox staleness, docs review, version validation), Phase 1 Context Summary gate, Phase 4 Documentation Review gate, Config-to-Workflow binding annotations, COHERENCE_MARKERS in validate.py, pre-task anchoring in task-tracking.md, and vertical vocabulary verification.
📝 WalkthroughWalkthroughAdds Plan→SpecOps automation and a Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer/CI
participant Lint as Linter (scripts/lint-spec-artifacts.py)
participant Specs as Specs Dir (.specops)
participant Validator as generator/validate.py
rect rgba(100,149,237,0.5)
Dev->>Lint: Run linter (local or CI)
Lint->>Specs: Enumerate specs, read spec.json, tasks.md, implementation.md
Lint->>Lint: Run checks (checkbox staleness, docs review, task-tracking, version fields)
Lint-->>Dev: PASS/FAIL + exit code
end
rect rgba(60,179,113,0.5)
Dev->>Validator: Run generator validation
Validator->>Specs: Check COHERENCE_MARKERS across generated platform outputs
Validator-->>Dev: Report consistency results
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Pull request overview
This PR strengthens the SpecOps workflow by introducing new enforcement gates (Context Summary, Coherence/Vocabulary verification, Documentation Review) and adding a repository-level linter to deterministically validate completed spec artifacts under .specops/.
Changes:
- Added a new spec artifact linter (
scripts/lint-spec-artifacts.py) and integrated it into the test suite. - Introduced new workflow enforcement steps (Context Summary entry gate, Coherence Verification markers, Vocabulary Verification, Documentation Review gate) and propagated them into generated platform outputs.
- Updated validation/checksum infrastructure (new validator markers, regenerated outputs and CHECKSUMS).
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/specops/SKILL.md | Adds enforcement-gate instructions (Context Summary, Coherence/Vocabulary verification, Documentation Review) and workflow-impact annotations content. |
| scripts/run-tests.sh | Adds conditional linter invocation during test runs. |
| scripts/lint-spec-artifacts.py | New linter for checkbox staleness, documentation review presence, and version-field validation. |
| platforms/cursor/specops.mdc | Regenerated Cursor output reflecting new enforcement steps and config workflow-impact sections. |
| platforms/copilot/specops.instructions.md | Regenerated Copilot output reflecting new enforcement steps and config workflow-impact sections. |
| platforms/codex/SKILL.md | Regenerated Codex output reflecting new enforcement steps and config workflow-impact sections. |
| platforms/claude/SKILL.md | Regenerated Claude output reflecting new enforcement steps and config workflow-impact sections. |
| generator/validate.py | Adds COHERENCE_MARKERS and validates presence across each platform output + cross-platform consistency. |
| core/workflow.md | Adds Phase 1 Context Summary gate, Phase 2 entry gate, Coherence + Vocabulary verification steps, and Phase 4 Documentation Review gate. |
| core/verticals.md | Adds Vocabulary Verification rules and prohibited-terms table for non-default verticals. |
| core/templates/implementation.md | Extends implementation journal template with mandatory Context Summary and Documentation Review sections. |
| core/task-tracking.md | Adds Pre-Task Anchoring and strengthens Pivot Check to compare against anchored scope. |
| core/config-handling.md | Adds “Workflow Impact” subsections documenting how config values affect phases/steps. |
| CLAUDE.md | Documents the new linter command and adds the /resume-plan command to the table. |
| CHECKSUMS.sha256 | Regenerated checksums, but appears to have dropped some canonically-checked files. |
| .specops/memory/patterns.json | Updates local memory patterns/lessons and overlap counts. |
| .specops/memory/context.md | Adds completion summary entry for the new enforcement-roadmap spec. |
| .specops/index.json | Registers the new enforcement-roadmap spec in the global index. |
| .specops/enforcement-roadmap/tasks.md | Adds the completed task plan for this feature/spec. |
| .specops/enforcement-roadmap/spec.json | Adds new completed spec metadata, including SpecOps version fields. |
| .specops/enforcement-roadmap/requirements.md | Documents requirements/ACs for the enforcement roadmap work. |
| .specops/enforcement-roadmap/implementation.md | Records Context Summary + Documentation Review and completion summary for the spec. |
| .specops/enforcement-roadmap/design.md | Captures design decisions/architecture and testing strategy. |
| .claude/commands/resume-plan.md | Adds a new Claude command for resuming plans and syncing with main. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
scripts/run-tests.sh
Outdated
Comment on lines
+53
to
+54
| if [ -d ".specops" ]; then | ||
| run_test "Spec artifact lint" "python3 scripts/lint-spec-artifacts.py .specops" |
Comment on lines
10
to
15
| a33350311d72f19385b7c3dde8c0d01f5a2bb658af059df71889e405db9abdfc platforms/copilot/platform.json | ||
| b446bf118c773756697d0224eafb7e477d90faff5a373dd9180a1229fcc88b45 core/workflow.md | ||
| bd2f5b53ac11666b41e85d25f37c63e1b52456ed52980b820d4e8ab80cceb16f core/workflow.md | ||
| 8321d265f5879b3d3030fbef7f6300b7f1321ee6e178a9addd49c9e789004068 core/safety.md | ||
| fda1cece0f1831537973fddff45178c7a666409b45eac265835968d87f609859 core/reconciliation.md | ||
| cb16d5811c721022976937becc10a8b2604daa584f85dba9257456dd8603f777 core/task-delegation.md | ||
| dc89dce4d0abfa0ce60a5ca13cc4335e32cc0d0d8f93c53b9fc1f0b832a6a2c8 hooks/pre-commit | ||
| 066ca0f0831132ae6652b7548d14b0bf4f035bd3bd3da5fe880c6a76fa10c41b hooks/pre-push | ||
| 3ca5932fc7b5145d33d9380e01b836a643f711d6ada01888aef5680be9b2126e scripts/install-hooks.sh |
Comment on lines
+130
to
+166
| Legacy specs (completed before this gate was introduced, identified by | ||
| specopsCreatedWith < 1.4.0 or absent) are skipped. | ||
| """ | ||
| errors = [] | ||
|
|
||
| for spec_name in sorted(os.listdir(specs_dir)): | ||
| spec_dir = os.path.join(specs_dir, spec_name) | ||
| if not os.path.isdir(spec_dir): | ||
| continue | ||
|
|
||
| spec_json_path = os.path.join(spec_dir, "spec.json") | ||
| if not os.path.exists(spec_json_path): | ||
| continue | ||
|
|
||
| try: | ||
| with open(spec_json_path, "r", encoding="utf-8") as f: | ||
| spec = json.load(f) | ||
| except (json.JSONDecodeError, OSError): | ||
| continue | ||
|
|
||
| if spec.get("status") != "completed": | ||
| continue | ||
|
|
||
| # Skip legacy specs (created before this gate existed) | ||
| created_with = spec.get("specopsCreatedWith", "") | ||
| if not created_with or created_with == "unknown": | ||
| continue | ||
| # Parse version — skip if < 1.4.0 | ||
| try: | ||
| parts = [int(x) for x in created_with.split(".")] | ||
| if len(parts) == 3 and ( | ||
| parts[0] < 1 or (parts[0] == 1 and parts[1] < 4) | ||
| ): | ||
| continue | ||
| except (ValueError, IndexError): | ||
| continue | ||
|
|
scripts/lint-spec-artifacts.py
Outdated
Comment on lines
+256
to
+275
| # Check 3: Version field validation | ||
| print("\nCheck 3: Version field validation") | ||
| version_warnings = lint_version_fields(specs_dir) | ||
| if version_warnings: | ||
| for warn in version_warnings: | ||
| print(f" WARN: {warn}") | ||
| # Version issues are warnings, not errors | ||
| else: | ||
| print(" PASS: All version fields are valid") | ||
|
|
||
| print(f"\n{'=' * 40}") | ||
| if all_errors: | ||
| print(f"FAILED: {len(all_errors)} error(s) found") | ||
| return 1 | ||
| else: | ||
| if version_warnings: | ||
| print(f"PASSED with {len(version_warnings)} warning(s)") | ||
| else: | ||
| print("PASSED: All checks clean") | ||
| return 0 |
| <!-- Event-Driven: WHEN [event] THE SYSTEM SHALL [behavior] --> | ||
| - WHEN a completed task in tasks.md has unchecked `- [ ]` items outside a Deferred Criteria subsection THE SYSTEM SHALL report a linting error | ||
| - WHEN a completed spec's implementation.md lacks a `## Documentation Review` section THE SYSTEM SHALL report a linting error | ||
| - WHEN a spec.json contains `specopsCreatedWith` or `specopsUpdatedWith` that does not match a valid semver pattern THE SYSTEM SHALL report a linting error |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/run-tests.sh (1)
9-9:⚠️ Potential issue | 🟠 MajorMake the spec-lint gate config-aware and enable required shell strict mode.
The new lint gate is hardcoded to
.specops, so it can silently skip validation whenspecsDiris customized in.specops.json. Additionally, the script is missing the requiredset -eflag mandated by the coding guidelines.🔧 Proposed patch
-set -u +set -eu @@ -# Conditional: spec artifact linting (only if specsDir exists) -if [ -d ".specops" ]; then - run_test "Spec artifact lint" "python3 scripts/lint-spec-artifacts.py .specops" +SPECOPS_DIR="$(python3 - <<'PY' +import json +from pathlib import Path + +config_path = Path(".specops.json") +if not config_path.exists(): + print(".specops") +else: + try: + print(json.loads(config_path.read_text()).get("specsDir", ".specops")) + except Exception: + print(".specops") +PY +)" + +# Conditional: spec artifact linting (only if specsDir exists) +if [ -d "$SPECOPS_DIR" ]; then + run_test "Spec artifact lint" "python3 scripts/lint-spec-artifacts.py \"$SPECOPS_DIR\"" fiPer coding guidelines, shell scripts must pass ShellCheck without warnings, use
set -e, and quote all variable expansions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/run-tests.sh` at line 9, The script currently hardcodes the lint gate to ".specops" and uses only "set -u"; update the gate lookup to read the configured specsDir from the project's .specops.json (or default to ".specops") so spec-lint runs against the correct directory, and enable strict shell mode by adding "set -e" alongside "set -u"; also ensure all variable expansions in run-tests.sh are quoted per ShellCheck (e.g., references to the gate path and any vars used when invoking spec-lint) so the script is config-aware and ShellCheck-compliant.
🧹 Nitpick comments (6)
.claude/commands/resume-plan.md (1)
25-32: Add language specifiers to output example blocks.Multiple fenced code blocks displaying example outputs lack language specifiers (markdownlint MD040). Consider using
textorplaintextfor consistency.📝 Suggested fixes
4. Display the list as a numbered table: -``` +```text Available plans (most recent first):If any referenced files are missing, display a warning: -``` +```text Warning: The following files referenced in the plan may no longer exist:Display the full plan with context: -``` +```text ========================================Also applies to: 94-101, 109-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/resume-plan.md around lines 25 - 32, Update the Markdown fenced output blocks in .claude/commands/resume-plan.md to include a language specifier (use `text` or `plaintext`) so they satisfy markdownlint MD040; specifically change the opening fences for the example output blocks (e.g., the "Available plans (most recent first):" block and the other output blocks around the ranges mentioned) from triple backticks with no language to triple backticks with `text` (and leave the closing triple backticks unchanged), ensuring all similar output/example blocks (including those around lines 25-32, 94-101, and 109-117) are updated for consistency.generator/validate.py (1)
680-680: Consider extracting marker list concatenation for readability.This line is extremely long due to the accumulated marker lists. While it follows the existing pattern, this might be a good opportunity to refactor into a combined constant or helper.
♻️ Optional: Extract combined markers
# Define at module level after all marker lists ALL_CROSS_PLATFORM_MARKERS = ( WORKFLOW_MARKERS + SAFETY_MARKERS + TEMPLATE_MARKERS + VERTICAL_MARKERS + INTERVIEW_MARKERS + STEERING_MARKERS + REVIEW_MARKERS + VIEW_MARKERS + UPDATE_MARKERS + TASK_TRACKING_MARKERS + EXTERNAL_TRACKING_MARKERS + REGRESSION_MARKERS + RECONCILIATION_MARKERS + FROM_PLAN_MARKERS + MEMORY_MARKERS + REPO_MAP_MARKERS + DELEGATION_MARKERS + COHERENCE_MARKERS ) # Then in main(): for marker in ALL_CROSS_PLATFORM_MARKERS:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@generator/validate.py` at line 680, Extract the long concatenation into a module-level constant (e.g., ALL_CROSS_PLATFORM_MARKERS) defined after the individual marker lists by combining WORKFLOW_MARKERS, SAFETY_MARKERS, TEMPLATE_MARKERS, VERTICAL_MARKERS, INTERVIEW_MARKERS, STEERING_MARKERS, REVIEW_MARKERS, VIEW_MARKERS, UPDATE_MARKERS, TASK_TRACKING_MARKERS, EXTERNAL_TRACKING_MARKERS, REGRESSION_MARKERS, RECONCILIATION_MARKERS, FROM_PLAN_MARKERS, MEMORY_MARKERS, REPO_MAP_MARKERS, DELEGATION_MARKERS and COHERENCE_MARKERS, then replace the long for loop header with: for marker in ALL_CROSS_PLATFORM_MARKERS: to improve readability and maintainability..specops/enforcement-roadmap/design.md (1)
70-77: Add language specifier to fenced code block.The sequence diagram code block lacks a language specifier, which triggers markdownlint MD040. Consider adding a language identifier for consistency.
📝 Suggested fix
-``` +```text Linter -> specsDir: scan for spec directories🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.specops/enforcement-roadmap/design.md around lines 70 - 77, The fenced sequence diagram block beginning with "Linter -> specsDir: scan for spec directories" lacks a language identifier and triggers markdownlint MD040; fix it by updating the opening fence from ``` to a language-specified fence such as ```text (or another appropriate identifier) so the block becomes ```text ... ```, and apply the same change to any other similar fenced blocks in the document to satisfy MD040.core/workflow.md (1)
146-146: Run a focused security review before merge.This file now introduces stricter phase gates and write-path behavior; a dedicated
/security-reviewpass is warranted before release.Based on learnings: Security-sensitive files require extra scrutiny when modified (including
core/workflow.md) and suggest running/security-reviewbefore pushing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/workflow.md` at line 146, Update the workflow to mandate a focused security review before merge by adding a `/security-review` pass to the "Documentation check (enforcement gate)" section in core/workflow.md and note that core/workflow.md is security-sensitive; explicitly require the `/security-review` approval step prior to release and document this change in implementation.md under the `## Documentation Review` section (add an entry for core/workflow.md marked "security-reviewed" and list any follow-up actions).platforms/claude/SKILL.md (1)
1-3672: Run a focused security review pass before merge.Given this file is security-sensitive and changes execution protocol/gating behavior, run the repo’s
/security-reviewworkflow (or equivalent) before merging to catch prompt-injection and unsafe-command regressions in instruction text.Based on learnings: "Security-sensitive files require extra scrutiny when modified (… platforms/claude/SKILL.md …). Consider running /security-review before pushing."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/claude/SKILL.md` around lines 1 - 3672, Summary: This PR modifies the SKILL.md instructions which are security-sensitive and must be reviewed by the repo's security audit before merging. Fix: before merging, run the repository’s /security-review workflow and address any findings (prompt-injection, unsafe shell commands, meta-instruction exposures) against the SpecOps skill text (SKILL.md) and related instruction blocks (e.g., Version Extraction Protocol, Repo Map generation, Task Delegation rules); add/update a PR checklist entry requiring the /security-review run and/or add a CI gate that fails merges until the security-review job passes; ensure reviewers explicitly confirm prompt-injection and unsafe-command checks in the PR description or review checklist.platforms/copilot/specops.instructions.md (1)
2773-2773: Template comment could be more precise about multi-phase population.The comment states "Populated during Phase 1" but Phase 2 steps 5.5 (line 114) and 5.6 (line 116) also append coherence and vocabulary check results to this section. Consider updating the comment to reflect that both phases contribute.
♻️ Suggested clarification
-<!-- Populated during Phase 1. Proceeding to Phase 2 without this section is a protocol breach. --> +<!-- Created during Phase 1; coherence and vocabulary checks appended in Phase 2. Proceeding to Phase 2 without this section is a protocol breach. -->Note: Since this is a generated file, the fix should be applied to the source template in
core/templates/implementation.md, then platforms regenerated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/copilot/specops.instructions.md` at line 2773, Update the placeholder comment that currently reads "Populated during Phase 1. Proceeding to Phase 2 without this section is a protocol breach." so it accurately states that content is populated across both Phase 1 and Phase 2 (specifically including the append operations from Phase 2 steps 5.5 and 5.6 which add coherence and vocabulary check results); modify the template entry that contains that exact comment text to mention both phases contribute (e.g., "Populated during Phase 1 and appended to in Phase 2 (steps 5.5 and 5.6) with coherence and vocabulary checks"), then regenerate platform docs so the change appears in the generated specops.instructions.md.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.specops/enforcement-roadmap/implementation.md:
- Line 29: Edit the Documentation Review entry that currently reads
"`CLAUDE.md`: Updated — added `scripts/lint-spec-artifacts.py` to Key Commands
section (pending below)`" and remove the stale phrase " (pending below)" so the
line simply states the update; specifically locate the string containing
`scripts/lint-spec-artifacts.py` in the Documentation Review/implementation note
and delete the parenthetical to reflect there are no outstanding follow-ups.
In `@core/task-tracking.md`:
- Around line 30-39: Clarify that "Pre-Task Anchoring" is a preparatory step
that must occur before starting work, and then require an immediate update of
the task's status to In Progress in tasks.md per the Write Ordering Protocol:
update the Pre-Task Anchoring section to state that after appending the Task
Scope note to implementation.md, the assignee MUST set the task status to "In
Progress" in tasks.md (or follow Write Ordering Protocol) so anchoring does not
conflict with the existing "In Progress first" rule.
In `@core/workflow.md`:
- Line 38: The lifecycle gating currently forces writing implementation.md too
early and uses destructive "WRITE_FILE" semantics that can clobber resumed
journal content; update the protocol so the Phase 1 Context Summary is written
only after the spec workspace and file lifecycle are fully established (i.e.,
after context recovery and steering files are loaded) and replace the plain
WRITE_FILE instruction with a non-destructive "Ensure section" or "UPSERT
SECTION '## Phase 1 Context Summary' into implementation.md using template from
core/templates/implementation.md" semantics that append/merge into existing
implementation.md (preserving prior journal content) rather than overwriting the
whole file; apply the same lifecycle ordering and non-destructive write
semantics to the other occurrences mentioned (lines referencing
implementation.md around the other commented spots).
In `@platforms/claude/SKILL.md`:
- Line 45: The Phase gating is deadlocked because the "Phase 1 Context Summary"
requirement in implementation.md demands "detected vertical" and "affected
files" (items produced later in Phase 1 steps 7 and 9), while Phase 2 enforces
that implementation.md already contains that section; reorder the checks so
Phase 1 produces and writes the "## Phase 1 Context Summary" (using the
core/templates/implementation.md template) after steps that determine detected
vertical and affected files, and change the Phase 2 gate to only require the
existence of implementation.md (not the full Phase 1 section) or to validate the
Phase 1 section only after Phase 1 completes; update references to "detected
vertical", "affected files", "Phase 1 Context Summary", "implementation.md", and
"Phase 2" accordingly.
In `@platforms/copilot/specops.instructions.md`:
- Around line 2772-2781: The Phase 1 Context Summary template is missing
placeholders for the coherence and vocabulary check results referenced by Phase
2 steps 5.5 and 5.6; update the source template
(core/templates/implementation.md) so the "Phase 1 Context Summary" section
includes two new placeholder lines (e.g., "Coherence check: [result]" and
"Vocabulary check: [result]") beneath "Memory" or before "Affected files", then
regenerate platform files so platforms/copilot/specops.instructions.md reflects
the added placeholders and you do not edit generated files directly.
In `@scripts/lint-spec-artifacts.py`:
- Around line 34-38: The task-name extraction using line.strip("# ") in the
current_task construction is unsafe because strip removes all '#' and spaces
from both ends; instead, detect the leading "###" explicitly (e.g., use
line.lstrip("# ").lstrip() or better use a regex) and capture only the text
after the prefix; update the code around current_task creation to use
re.match(r"^###\s*(.+)", line) (or line[3:].strip() if you validate the prefix)
and set current_task["name"] to the matched group (or the fallback original
line) so names that contain '#' are preserved.
---
Outside diff comments:
In `@scripts/run-tests.sh`:
- Line 9: The script currently hardcodes the lint gate to ".specops" and uses
only "set -u"; update the gate lookup to read the configured specsDir from the
project's .specops.json (or default to ".specops") so spec-lint runs against the
correct directory, and enable strict shell mode by adding "set -e" alongside
"set -u"; also ensure all variable expansions in run-tests.sh are quoted per
ShellCheck (e.g., references to the gate path and any vars used when invoking
spec-lint) so the script is config-aware and ShellCheck-compliant.
---
Nitpick comments:
In @.claude/commands/resume-plan.md:
- Around line 25-32: Update the Markdown fenced output blocks in
.claude/commands/resume-plan.md to include a language specifier (use `text` or
`plaintext`) so they satisfy markdownlint MD040; specifically change the opening
fences for the example output blocks (e.g., the "Available plans (most recent
first):" block and the other output blocks around the ranges mentioned) from
triple backticks with no language to triple backticks with `text` (and leave the
closing triple backticks unchanged), ensuring all similar output/example blocks
(including those around lines 25-32, 94-101, and 109-117) are updated for
consistency.
In @.specops/enforcement-roadmap/design.md:
- Around line 70-77: The fenced sequence diagram block beginning with "Linter ->
specsDir: scan for spec directories" lacks a language identifier and triggers
markdownlint MD040; fix it by updating the opening fence from ``` to a
language-specified fence such as ```text (or another appropriate identifier) so
the block becomes ```text ... ```, and apply the same change to any other
similar fenced blocks in the document to satisfy MD040.
In `@core/workflow.md`:
- Line 146: Update the workflow to mandate a focused security review before
merge by adding a `/security-review` pass to the "Documentation check
(enforcement gate)" section in core/workflow.md and note that core/workflow.md
is security-sensitive; explicitly require the `/security-review` approval step
prior to release and document this change in implementation.md under the `##
Documentation Review` section (add an entry for core/workflow.md marked
"security-reviewed" and list any follow-up actions).
In `@generator/validate.py`:
- Line 680: Extract the long concatenation into a module-level constant (e.g.,
ALL_CROSS_PLATFORM_MARKERS) defined after the individual marker lists by
combining WORKFLOW_MARKERS, SAFETY_MARKERS, TEMPLATE_MARKERS, VERTICAL_MARKERS,
INTERVIEW_MARKERS, STEERING_MARKERS, REVIEW_MARKERS, VIEW_MARKERS,
UPDATE_MARKERS, TASK_TRACKING_MARKERS, EXTERNAL_TRACKING_MARKERS,
REGRESSION_MARKERS, RECONCILIATION_MARKERS, FROM_PLAN_MARKERS, MEMORY_MARKERS,
REPO_MAP_MARKERS, DELEGATION_MARKERS and COHERENCE_MARKERS, then replace the
long for loop header with: for marker in ALL_CROSS_PLATFORM_MARKERS: to improve
readability and maintainability.
In `@platforms/claude/SKILL.md`:
- Around line 1-3672: Summary: This PR modifies the SKILL.md instructions which
are security-sensitive and must be reviewed by the repo's security audit before
merging. Fix: before merging, run the repository’s /security-review workflow and
address any findings (prompt-injection, unsafe shell commands, meta-instruction
exposures) against the SpecOps skill text (SKILL.md) and related instruction
blocks (e.g., Version Extraction Protocol, Repo Map generation, Task Delegation
rules); add/update a PR checklist entry requiring the /security-review run
and/or add a CI gate that fails merges until the security-review job passes;
ensure reviewers explicitly confirm prompt-injection and unsafe-command checks
in the PR description or review checklist.
In `@platforms/copilot/specops.instructions.md`:
- Line 2773: Update the placeholder comment that currently reads "Populated
during Phase 1. Proceeding to Phase 2 without this section is a protocol
breach." so it accurately states that content is populated across both Phase 1
and Phase 2 (specifically including the append operations from Phase 2 steps 5.5
and 5.6 which add coherence and vocabulary check results); modify the template
entry that contains that exact comment text to mention both phases contribute
(e.g., "Populated during Phase 1 and appended to in Phase 2 (steps 5.5 and 5.6)
with coherence and vocabulary checks"), then regenerate platform docs so the
change appears in the generated specops.instructions.md.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 123ad271-5138-4aec-9739-82cd54fcca1e
📒 Files selected for processing (24)
.claude/commands/resume-plan.md.specops/enforcement-roadmap/design.md.specops/enforcement-roadmap/implementation.md.specops/enforcement-roadmap/requirements.md.specops/enforcement-roadmap/spec.json.specops/enforcement-roadmap/tasks.md.specops/index.json.specops/memory/context.md.specops/memory/patterns.jsonCHECKSUMS.sha256CLAUDE.mdcore/config-handling.mdcore/task-tracking.mdcore/templates/implementation.mdcore/verticals.mdcore/workflow.mdgenerator/validate.pyplatforms/claude/SKILL.mdplatforms/codex/SKILL.mdplatforms/copilot/specops.instructions.mdplatforms/cursor/specops.mdcscripts/lint-spec-artifacts.pyscripts/run-tests.shskills/specops/SKILL.md
| |---------|------------|--------|------| | ||
|
|
||
| ## Documentation Review | ||
| - `CLAUDE.md`: Updated — added `scripts/lint-spec-artifacts.py` to Key Commands section (pending below) |
There was a problem hiding this comment.
Remove stale “pending below” wording in Documentation Review entry.
The phrase suggests unresolved follow-up that is not actually present later in the file.
🧹 Proposed fix
-- `CLAUDE.md`: Updated — added `scripts/lint-spec-artifacts.py` to Key Commands section (pending below)
+- `CLAUDE.md`: Updated — added `scripts/lint-spec-artifacts.py` to Key Commands section📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - `CLAUDE.md`: Updated — added `scripts/lint-spec-artifacts.py` to Key Commands section (pending below) | |
| - `CLAUDE.md`: Updated — added `scripts/lint-spec-artifacts.py` to Key Commands section |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.specops/enforcement-roadmap/implementation.md at line 29, Edit the
Documentation Review entry that currently reads "`CLAUDE.md`: Updated — added
`scripts/lint-spec-artifacts.py` to Key Commands section (pending below)`" and
remove the stale phrase " (pending below)" so the line simply states the update;
specifically locate the string containing `scripts/lint-spec-artifacts.py` in
the Documentation Review/implementation note and delete the parenthetical to
reflect there are no outstanding follow-ups.
Comment on lines
+30
to
+39
| ### Pre-Task Anchoring | ||
|
|
||
| Before setting a task to `In Progress`, anchor the task's expected scope in `implementation.md`: | ||
|
|
||
| 1. READ_FILE the task's **Acceptance Criteria** and **Tests Required** sections from `tasks.md` | ||
| 2. READ_FILE the relevant requirements from `requirements.md`/`bugfix.md`/`refactor.md` and the matching design section from `design.md` | ||
| 3. EDIT_FILE `implementation.md` — append a brief Task Scope note to the Session Log: `Task N scope: [1-2 sentence summary of expected changes and acceptance criteria]` | ||
|
|
||
| This anchored scope is used by the Pivot Check (below) to detect drift between planned and actual changes. Without the anchor, pivot detection has nothing to compare against. | ||
|
|
There was a problem hiding this comment.
Clarify ordering to avoid conflict with the existing “In Progress first” rule.
Please explicitly state that anchoring is a pre-work preparation step, then tasks.md must be updated to In Progress immediately per Write Ordering Protocol.
✏️ Suggested wording tweak
This anchored scope is used by the Pivot Check (below) to detect drift between planned and actual changes. Without the anchor, pivot detection has nothing to compare against.
+After writing the scope anchor, immediately update the task status to `In Progress` in `tasks.md` (Write Ordering Protocol) before implementation work begins.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ### Pre-Task Anchoring | |
| Before setting a task to `In Progress`, anchor the task's expected scope in `implementation.md`: | |
| 1. READ_FILE the task's **Acceptance Criteria** and **Tests Required** sections from `tasks.md` | |
| 2. READ_FILE the relevant requirements from `requirements.md`/`bugfix.md`/`refactor.md` and the matching design section from `design.md` | |
| 3. EDIT_FILE `implementation.md` — append a brief Task Scope note to the Session Log: `Task N scope: [1-2 sentence summary of expected changes and acceptance criteria]` | |
| This anchored scope is used by the Pivot Check (below) to detect drift between planned and actual changes. Without the anchor, pivot detection has nothing to compare against. | |
| ### Pre-Task Anchoring | |
| Before setting a task to `In Progress`, anchor the task's expected scope in `implementation.md`: | |
| 1. READ_FILE the task's **Acceptance Criteria** and **Tests Required** sections from `tasks.md` | |
| 2. READ_FILE the relevant requirements from `requirements.md`/`bugfix.md`/`refactor.md` and the matching design section from `design.md` | |
| 3. EDIT_FILE `implementation.md` — append a brief Task Scope note to the Session Log: `Task N scope: [1-2 sentence summary of expected changes and acceptance criteria]` | |
| This anchored scope is used by the Pivot Check (below) to detect drift between planned and actual changes. Without the anchor, pivot detection has nothing to compare against. | |
| After writing the scope anchor, immediately update the task status to `In Progress` in `tasks.md` (Write Ordering Protocol) before implementation work begins. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/task-tracking.md` around lines 30 - 39, Clarify that "Pre-Task
Anchoring" is a preparatory step that must occur before starting work, and then
require an immediate update of the task's status to In Progress in tasks.md per
the Write Ordering Protocol: update the Pre-Task Anchoring section to state that
after appending the Task Scope note to implementation.md, the assignee MUST set
the task status to "In Progress" in tasks.md (or follow Write Ordering Protocol)
so anchoring does not conflict with the existing "In Progress first" rule.
Comment on lines
+2772
to
+2781
| ## Phase 1 Context Summary | ||
| <!-- Populated during Phase 1. Proceeding to Phase 2 without this section is a protocol breach. --> | ||
| - Config: [loaded from `.specops.json` or defaults — vertical, specsDir, taskTracking] | ||
| - Context recovery: [none / resuming <spec-name>] | ||
| - Steering files: [loaded N files (names)] | ||
| - Repo map: [loaded / generated / stale-refreshed / not available] | ||
| - Memory: [loaded N decisions from M specs, P patterns / no memory files] | ||
| - Vertical: [detected or configured vertical] | ||
| - Affected files: [list of affected file paths] | ||
|
|
There was a problem hiding this comment.
Template should include placeholders for coherence and vocabulary checks.
The Phase 1 Context Summary template lists 7 items (Config through Affected files), but Phase 2 steps 5.5 (line 114) and 5.6 (line 116) instruct users to record coherence check and vocabulary check results in this same section. The template should include placeholder lines for these to guide users on where to append them.
📝 Suggested template addition
## Phase 1 Context Summary
<!-- Populated during Phase 1. Proceeding to Phase 2 without this section is a protocol breach. -->
- Config: [loaded from `.specops.json` or defaults — vertical, specsDir, taskTracking]
- Context recovery: [none / resuming <spec-name>]
- Steering files: [loaded N files (names)]
- Repo map: [loaded / generated / stale-refreshed / not available]
- Memory: [loaded N decisions from M specs, P patterns / no memory files]
- Vertical: [detected or configured vertical]
- Affected files: [list of affected file paths]
+- Coherence check: [populated in Phase 2 step 5.5]
+- Vocabulary check: [populated in Phase 2 step 5.6]Note: Since this is a generated file, the fix should be applied to the source template in core/templates/implementation.md, then platforms regenerated. As per coding guidelines, generated platform files should not be edited directly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## Phase 1 Context Summary | |
| <!-- Populated during Phase 1. Proceeding to Phase 2 without this section is a protocol breach. --> | |
| - Config: [loaded from `.specops.json` or defaults — vertical, specsDir, taskTracking] | |
| - Context recovery: [none / resuming <spec-name>] | |
| - Steering files: [loaded N files (names)] | |
| - Repo map: [loaded / generated / stale-refreshed / not available] | |
| - Memory: [loaded N decisions from M specs, P patterns / no memory files] | |
| - Vertical: [detected or configured vertical] | |
| - Affected files: [list of affected file paths] | |
| ## Phase 1 Context Summary | |
| <!-- Populated during Phase 1. Proceeding to Phase 2 without this section is a protocol breach. --> | |
| - Config: [loaded from `.specops.json` or defaults — vertical, specsDir, taskTracking] | |
| - Context recovery: [none / resuming <spec-name>] | |
| - Steering files: [loaded N files (names)] | |
| - Repo map: [loaded / generated / stale-refreshed / not available] | |
| - Memory: [loaded N decisions from M specs, P patterns / no memory files] | |
| - Vertical: [detected or configured vertical] | |
| - Affected files: [list of affected file paths] | |
| - Coherence check: [populated in Phase 2 step 5.5] | |
| - Vocabulary check: [populated in Phase 2 step 5.6] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/copilot/specops.instructions.md` around lines 2772 - 2781, The
Phase 1 Context Summary template is missing placeholders for the coherence and
vocabulary check results referenced by Phase 2 steps 5.5 and 5.6; update the
source template (core/templates/implementation.md) so the "Phase 1 Context
Summary" section includes two new placeholder lines (e.g., "Coherence check:
[result]" and "Vocabulary check: [result]") beneath "Memory" or before "Affected
files", then regenerate platform files so
platforms/copilot/specops.instructions.md reflects the added placeholders and
you do not edit generated files directly.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+144
to
+148
| try: | ||
| with open(spec_json_path, "r", encoding="utf-8") as f: | ||
| spec = json.load(f) | ||
| except (json.JSONDecodeError, OSError): | ||
| continue |
There was a problem hiding this comment.
spec.json in docs review check
When spec.json is invalid JSON or unreadable, lint_docs_review silently continues. This is inconsistent with lint_version_fields, which at least emits a WARN for the same condition. A corrupted or truncated spec.json in a completed spec would suppress the Documentation Review check without any signal.
try:
with open(spec_json_path, "r", encoding="utf-8") as f:
spec = json.load(f)
except (json.JSONDecodeError, OSError) as e:
errors.append(f" {spec_name}: spec.json could not be read: {e}")
continue…es, and dependency ordering Replace naive 4+ task count trigger with weighted complexity score (effort sizing + file breadth). Add post-delegation quality gate (file existence, checkbox consistency, session log presence). Add context refresh between delegates to prevent stale handoff bundles. Make task ordering dependency-aware with ready-set algorithm and deadlock detection.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
platforms/claude/SKILL.md (1)
45-45:⚠️ Potential issue | 🔴 CriticalPhase gate ordering is still self-blocking (and can clobber context files).
Line 45 requires writing
## Phase 1 Context Summarywithdetected verticalandaffected filesbefore those are produced, and Line 64 blocks Phase 2 unless that section already exists. This creates a circular gate on fresh specs. Also, “Use the Write tool to createimplementation.md” at Line 45 risks overwriting an existing journal on resumed flows.Suggested fix
-5.5. **Context Summary (enforcement gate)**: Use the Write tool to create `implementation.md` with a `## Phase 1 Context Summary` section recording: config status, context recovery result, steering files loaded, repo map status, memory loaded, detected vertical, and affected files. This section is mandatory — proceeding to Phase 2 without writing it is a protocol breach. Use the template from `core/templates/implementation.md`. +5.5. **Context Summary pre-check (enforcement gate)**: Confirm required inputs are available for summary writing (config/context recovery/steering/repo map/memory). Do not write final summary fields that are computed later in Phase 1. ... 9. Identify affected files, components, and dependencies — produce a concrete list of affected file paths for `fileMatch` steering file evaluation +10. **Write Context Summary (enforcement gate)**: Use the Edit tool to modify `implementation.md` and write `## Phase 1 Context Summary` after steps 6–9 are complete, including detected vertical and affected files. -0. **Phase 2 entry gate**: Use the Read tool to read `implementation.md` and verify it contains `## Phase 1 Context Summary`. If missing, STOP — return to Phase 1 step 5.5 and write it. Proceeding without the Context Summary is a protocol breach. +0. **Phase 2 entry gate**: Use the Read tool to read `implementation.md` and verify it contains `## Phase 1 Context Summary`. If missing, STOP — return to Phase 1 step 10 and write it.Also applies to: 64-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/claude/SKILL.md` at line 45, The Phase 1 gate currently requires creating an `implementation.md` with a `## Phase 1 Context Summary` (including `detected vertical` and `affected files`) before those items exist and also mandates it for Phase 2, causing a circular dependency and risking overwrites; change the protocol so the gate checks for the presence or appends/updates the `## Phase 1 Context Summary` only after `detected vertical` and `affected files` have been produced, and replace the unconditional "Use the Write tool to create `implementation.md`" with a safe operation (e.g., create-if-missing or append/update) that preserves existing journal content when resuming flows and only writes the required fields after they are available.platforms/copilot/specops.instructions.md (1)
2772-2780:⚠️ Potential issue | 🟡 MinorContext Summary template is missing required coherence/vocabulary result placeholders.
Phase 2 (Lines 114–115) requires writing both checks into
## Phase 1 Context Summary, but the template block at Line 2772+ still omits explicit placeholder rows for them.Suggested template fix
## Phase 1 Context Summary <!-- Populated during Phase 1. Proceeding to Phase 2 without this section is a protocol breach. --> - Config: [loaded from `.specops.json` or defaults — vertical, specsDir, taskTracking] - Context recovery: [none / resuming <spec-name>] - Steering files: [loaded N files (names)] - Repo map: [loaded / generated / stale-refreshed / not available] - Memory: [loaded N decisions from M specs, P patterns / no memory files] - Vertical: [detected or configured vertical] +- Coherence check: [pass / N contradiction(s) found — details] +- Vocabulary check: [pass / N term(s) replaced / not applicable] - Affected files: [list of affected file paths]Also applies to: 114-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/copilot/specops.instructions.md` around lines 2772 - 2780, The Phase 1 Context Summary template under the "## Phase 1 Context Summary" heading is missing explicit placeholders for the coherence and vocabulary result checks required by Phase 2; update the template to include two new placeholder rows (e.g., "Coherence check: [pass / fail / details]" and "Vocabulary result: [pass / fail / details]") so Phase 2 can write its checks into the summary; modify the template block in platforms/copilot/specops.instructions.md around the existing summary lines and ensure the labels match the Phase 2 expectations so the checks inserted later (Phase 2 checks referenced at lines 114–115) can find and populate them.
🧹 Nitpick comments (1)
platforms/cursor/specops.mdc (1)
116-117: Naming inconsistency: "Phase 1 Context Summary" contains Phase 2 check results.The
## Phase 1 Context Summarysection is created at the end of Phase 1 (line 45), but Phase 2 steps 5.5 (Coherence Verification, line 116) and 5.6 (Vocabulary Verification, line 117) append their results to this "Phase 1" section. This is functionally correct but semantically confusing.Consider renaming to
## Pre-Implementation Context Summaryor## Setup & Validation Summaryto clarify that it aggregates context from both Phase 1 and early Phase 2 checks.Since this is a generated platform file, the fix should be made in
core/templates/implementation.mdor the relevant core/ module, then regenerated.Based on learnings: This is a generated platform file — edit core/ modules or generator/templates/*.j2 instead, then regenerate.
Also applies to: 2774-2782
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/cursor/specops.mdc` around lines 116 - 117, The problem is a naming inconsistency: Phase 2 checks (steps 5.5 Coherence Verification and 5.6 Vocabulary Verification) append results into the "## Phase 1 Context Summary", which is semantically misleading; update the template that produces the section header (search for the literal "## Phase 1 Context Summary" in the generator/template or core module) and rename it to a clearer header such as "## Pre-Implementation Context Summary" or "## Setup & Validation Summary" in core/templates/implementation.md (or the corresponding generator/templates/*.j2), then regenerate the platform files so steps 5.5 and 5.6 write their outputs under the new header; ensure any references to the old header string in code that appends entries (e.g., the code that records the `- Coherence check:` and vocabulary entries) are updated to use the new header name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/claude/SKILL.md`:
- Around line 493-495: The workflow description for taskDelegation is outdated:
update the text under "Workflow Impact: taskDelegation" to match the actual
activation logic used in the code (the complexity-based rule): state that "auto"
triggers delegation when complexity = sum(effort) + floor(files/5) is >= 6,
"always" forces delegation regardless of complexity, and "never" uses sequential
execution; ensure you reference the same complexity formula and threshold used
in the active logic (the sum(effort) + floor(files/5) >= 6 rule) so the doc and
the taskDelegation behavior are consistent.
In `@platforms/codex/SKILL.md`:
- Around line 3235-3239: The quality-gate can downgrade a task from "Completed"
to "In Progress" but the orchestrator loop only picks "Pending" tasks, risking
stranded tasks; update the orchestration logic so that when the quality-gate
rules (file existence checks, checkbox consistency, Session Log presence in
implementation.md) set a task back to "In Progress" the task is explicitly
re-enqueued or flagged for immediate re-evaluation (e.g., add "In Progress" to
the orchestrator's selection criteria or push the task ID back into the pending
queue) and ensure the fallback Session Log append ("Task N: completed by
delegate (no session log written — quality gate backfill)") is written before
re-enqueueing to avoid race conditions.
- Around line 492-494: The documentation for "Workflow Impact: taskDelegation"
conflicts with the Task Delegation algorithm: the doc states auto mode triggers
at "4+ pending tasks" while the algorithm uses a weighted complexity threshold
("score >= 6"); update the doc so both speak the same rule. Replace the "4+
pending tasks" description with the algorithm's rule (or vice versa) and clearly
document the three modes ("auto", "always", "never") with the consistent
condition reference to the Task Delegation algorithm's threshold ("score >= 6"
in the taskDelegation logic) so readers/agents see one source of truth.
In `@platforms/copilot/specops.instructions.md`:
- Around line 492-493: Update the "Phase 3 step 2" Workflow Impact text so it
matches the actual auto-delegation gate used in the algorithm: replace the
phrase "If 'auto' and 4+ pending tasks, activate delegation" with wording that
states auto activates based on the weighted complexity score (e.g., "auto:
activate delegation when weighted complexity score >= 6"), and ensure the
options "always" and "never" remain described as before; reference the algorithm
check `score >= 6` in the text so the documentation and code (the
auto-delegation rule) are consistent.
In `@skills/specops/SKILL.md`:
- Line 45: The instruction is ambiguous about where implementation.md should be
read/written for the Phase 1/Phase 2 enforcement gates; update the checks and
write operations that reference "implementation.md" so they operate on the
spec-scoped path (<specsDir>/<spec-name>/implementation.md) rather than a
repo-root file; locate code handling the "Phase 1 Context Summary", the gating
logic that enforces writing/reading implementation.md, and any use of the
template from core/templates/implementation.md and ensure those
functions/classes use the spec directory (specsDir and spec-name) to construct
the full path before reading or writing.
---
Duplicate comments:
In `@platforms/claude/SKILL.md`:
- Line 45: The Phase 1 gate currently requires creating an `implementation.md`
with a `## Phase 1 Context Summary` (including `detected vertical` and `affected
files`) before those items exist and also mandates it for Phase 2, causing a
circular dependency and risking overwrites; change the protocol so the gate
checks for the presence or appends/updates the `## Phase 1 Context Summary` only
after `detected vertical` and `affected files` have been produced, and replace
the unconditional "Use the Write tool to create `implementation.md`" with a safe
operation (e.g., create-if-missing or append/update) that preserves existing
journal content when resuming flows and only writes the required fields after
they are available.
In `@platforms/copilot/specops.instructions.md`:
- Around line 2772-2780: The Phase 1 Context Summary template under the "##
Phase 1 Context Summary" heading is missing explicit placeholders for the
coherence and vocabulary result checks required by Phase 2; update the template
to include two new placeholder rows (e.g., "Coherence check: [pass / fail /
details]" and "Vocabulary result: [pass / fail / details]") so Phase 2 can write
its checks into the summary; modify the template block in
platforms/copilot/specops.instructions.md around the existing summary lines and
ensure the labels match the Phase 2 expectations so the checks inserted later
(Phase 2 checks referenced at lines 114–115) can find and populate them.
---
Nitpick comments:
In `@platforms/cursor/specops.mdc`:
- Around line 116-117: The problem is a naming inconsistency: Phase 2 checks
(steps 5.5 Coherence Verification and 5.6 Vocabulary Verification) append
results into the "## Phase 1 Context Summary", which is semantically misleading;
update the template that produces the section header (search for the literal "##
Phase 1 Context Summary" in the generator/template or core module) and rename it
to a clearer header such as "## Pre-Implementation Context Summary" or "## Setup
& Validation Summary" in core/templates/implementation.md (or the corresponding
generator/templates/*.j2), then regenerate the platform files so steps 5.5 and
5.6 write their outputs under the new header; ensure any references to the old
header string in code that appends entries (e.g., the code that records the `-
Coherence check:` and vocabulary entries) are updated to use the new header
name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6db56efe-5e25-4a5f-8917-ae05e2dcda8d
📒 Files selected for processing (9)
CHECKSUMS.sha256core/task-delegation.mddocs/REFERENCE.mdplatforms/claude/SKILL.mdplatforms/codex/SKILL.mdplatforms/copilot/specops.instructions.mdplatforms/cursor/specops.mdcschema.jsonskills/specops/SKILL.md
…enforcement Add plan file input and auto-discovery to from-plan mode, post-plan-acceptance gate (step 10.5) with protocol breach enforcement in workflow.md, and SpecOps handoff step in resume-plan.md. Add planFileDirectory to Claude platform config. Add task tracking IssueID validation to spec artifact linter (Check 3).
There was a problem hiding this comment.
Pull request overview
This PR strengthens the SpecOps workflow with new enforcement gates and validation tooling, adds plan→spec automation support, and regenerates platform outputs/spec artifacts to reflect the updated core modules.
Changes:
- Added a spec artifact linter and wired it into the test runner.
- Introduced new workflow enforcement gates (Context Summary, Documentation Review) plus additional verification steps (coherence, vocabulary, pre-task anchoring, delegation updates).
- Enhanced From Plan mode (file-path + auto-discovery) and added a
/resume-plancommand + platform config support.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| TODO.txt | Adds two TODO entries. |
| skills/specops/SKILL.md | Propagates updated workflow instructions into the canonical skill doc. |
| scripts/run-tests.sh | Adds conditional invocation of the new spec artifact linter. |
| scripts/lint-spec-artifacts.py | New linter for spec artifact enforcement checks. |
| schema.json | Updates config schema description for complexity-based delegation behavior. |
| platforms/cursor/specops.mdc | Regenerated Cursor rules reflecting updated core modules. |
| platforms/copilot/specops.instructions.md | Regenerated Copilot instructions reflecting updated core modules. |
| platforms/codex/SKILL.md | Regenerated Codex skill reflecting updated core modules. |
| platforms/claude/SKILL.md | Regenerated Claude skill reflecting updated core modules. |
| platforms/claude/platform.json | Adds planFileDirectory for plan auto-discovery. |
| generator/validate.py | Adds validation markers for new plan and coherence content. |
| docs/REFERENCE.md | Updates docs for the new complexity-based delegation behavior. |
| core/workflow.md | Adds Context Summary gate, Phase 2 checks, and docs review enforcement gate. |
| core/verticals.md | Adds post-generation vocabulary verification guidance. |
| core/templates/implementation.md | Adds new required template sections (Context Summary, Documentation Review). |
| core/task-tracking.md | Adds pre-task anchoring and strengthens pivot check to reference it. |
| core/task-delegation.md | Updates delegation decision logic and adds post-delegation quality gates. |
| core/from-plan.md | Adds plan file-path support and platform plan auto-discovery branches. |
| core/config-handling.md | Adds “Workflow Impact” annotations across config sections. |
| CLAUDE.md | Documents the new linter command and /resume-plan. |
| CHECKSUMS.sha256 | Regenerated checksums list. |
| .specops/plan-to-specops-automation/tasks.md | New completed dogfood spec: tasks. |
| .specops/plan-to-specops-automation/spec.json | New completed dogfood spec: metadata. |
| .specops/plan-to-specops-automation/requirements.md | New completed dogfood spec: requirements. |
| .specops/plan-to-specops-automation/implementation.md | New completed dogfood spec: implementation journal (incl. required sections). |
| .specops/plan-to-specops-automation/design.md | New completed dogfood spec: design. |
| .specops/memory/patterns.json | Updates memory patterns based on new enforcement work. |
| .specops/memory/context.md | Adds completion summaries for new specs. |
| .specops/index.json | Adds entries for the newly completed specs. |
| .specops/enforcement-roadmap/tasks.md | New completed dogfood spec: tasks. |
| .specops/enforcement-roadmap/spec.json | New completed dogfood spec: metadata. |
| .specops/enforcement-roadmap/requirements.md | New completed dogfood spec: requirements. |
| .specops/enforcement-roadmap/implementation.md | New completed dogfood spec: implementation journal. |
| .specops/enforcement-roadmap/design.md | New completed dogfood spec: design. |
| .claude/commands/resume-plan.md | Adds new /resume-plan command workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+154
to
+166
| # Skip legacy specs (created before this gate existed) | ||
| created_with = spec.get("specopsCreatedWith", "") | ||
| if not created_with or created_with == "unknown": | ||
| continue | ||
| # Parse version — skip if < 1.4.0 | ||
| try: | ||
| parts = [int(x) for x in created_with.split(".")] | ||
| if len(parts) == 3 and ( | ||
| parts[0] < 1 or (parts[0] == 1 and parts[1] < 4) | ||
| ): | ||
| continue | ||
| except (ValueError, IndexError): | ||
| continue |
Comment on lines
+110
to
+116
| tasks_path = os.path.join(spec_dir, "tasks.md") | ||
| if not os.path.exists(tasks_path): | ||
| continue | ||
|
|
||
| with open(tasks_path, "r", encoding="utf-8") as f: | ||
| content = f.read() | ||
|
|
Comment on lines
+168
to
+177
| impl_path = os.path.join(spec_dir, "implementation.md") | ||
| if not os.path.exists(impl_path): | ||
| errors.append( | ||
| f" {spec_name}: completed spec missing implementation.md" | ||
| ) | ||
| continue | ||
|
|
||
| with open(impl_path, "r", encoding="utf-8") as f: | ||
| impl_content = f.read() | ||
|
|
scripts/lint-spec-artifacts.py
Outdated
Comment on lines
+186
to
+220
| def lint_version_fields(specs_dir): | ||
| """Validate specopsCreatedWith and specopsUpdatedWith in spec.json.""" | ||
| semver_re = re.compile(r"^\d+\.\d+\.\d+$") | ||
| warnings = [] | ||
|
|
||
| for spec_name in sorted(os.listdir(specs_dir)): | ||
| spec_dir = os.path.join(specs_dir, spec_name) | ||
| if not os.path.isdir(spec_dir): | ||
| continue | ||
|
|
||
| spec_json_path = os.path.join(spec_dir, "spec.json") | ||
| if not os.path.exists(spec_json_path): | ||
| continue | ||
|
|
||
| try: | ||
| with open(spec_json_path, "r", encoding="utf-8") as f: | ||
| spec = json.load(f) | ||
| except (json.JSONDecodeError, OSError): | ||
| warnings.append(f" {spec_name}: spec.json is not valid JSON") | ||
| continue | ||
|
|
||
| for field in ["specopsCreatedWith", "specopsUpdatedWith"]: | ||
| value = spec.get(field) | ||
| if value is None: | ||
| # Absent is OK (legacy spec) | ||
| continue | ||
| if value == "unknown": | ||
| # Acceptable fallback | ||
| continue | ||
| if not semver_re.match(value): | ||
| warnings.append( | ||
| f" {spec_name}: {field} has invalid version format: '{value}'" | ||
| ) | ||
|
|
||
| return warnings |
Comment on lines
+5
to
+10
| Validates spec artifacts in <specsDir>/ for: | ||
| 1. Checkbox staleness: completed tasks with unchecked items (excluding Deferred Criteria) | ||
| 2. Documentation Review: completed specs must have ## Documentation Review in implementation.md | ||
| 3. Task tracking: when taskTracking is configured, High/Medium tasks must have valid IssueIDs | ||
| 4. Version validation: specopsCreatedWith/specopsUpdatedWith must be valid semver, absent, or "unknown" | ||
|
|
skills/specops/SKILL.md
Outdated
Comment on lines
+499
to
+500
| ### Workflow Impact: taskDelegation | ||
| - **Phase 3 step 2**: If `"auto"` and 4+ pending tasks, activate delegation. If `"always"`, activate regardless. If `"never"`, use sequential execution. |
generator/validate.py
Outdated
Comment on lines
+212
to
+214
| "Coherence Verification", | ||
| "cross-check", | ||
| "contradictions", |
| a33350311d72f19385b7c3dde8c0d01f5a2bb658af059df71889e405db9abdfc platforms/copilot/platform.json | ||
| b446bf118c773756697d0224eafb7e477d90faff5a373dd9180a1229fcc88b45 core/workflow.md | ||
| b73bf7e75e5554e4f93135737f241c25dd2b3fbd8459664c982f7e19fa77fe22 core/workflow.md | ||
| 8321d265f5879b3d3030fbef7f6300b7f1321ee6e178a9addd49c9e789004068 core/safety.md |
.claude/commands/resume-plan.md
Outdated
Comment on lines
+51
to
+60
| 1. Run `git status` to check the working tree. If there are uncommitted changes (modified, staged, or untracked files — excluding untracked files under `.claude/`), warn the user: | ||
|
|
||
| "There are uncommitted changes in the working tree. These should be committed or stashed before resuming a plan to avoid conflicts." | ||
|
|
||
| Suggest: "Commit with `/commit`, stash with `git stash`, or continue at your own risk." | ||
|
|
||
| Ask: "Continue with uncommitted changes? (yes/no)". If the user says no, stop. | ||
|
|
||
| 2. Check for detached HEAD: run `git symbolic-ref -q HEAD`. If this fails (exit code non-zero), report: "You are in a detached HEAD state. Please checkout a branch before resuming a plan (`git checkout main` or `git checkout -b <branch-name>`)." and stop. | ||
|
|
scripts/lint-spec-artifacts.py
Outdated
Comment on lines
+320
to
+327
| # Check if ANY eligible task has a valid IssueID | ||
| has_any_valid = any( | ||
| i and valid_issue_re.match(i) for _, _, i in eligible | ||
| ) | ||
|
|
||
| # Skip specs where no tasks have valid IssueIDs (pre-enforcement legacy) | ||
| if not has_any_valid: | ||
| continue |
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (8)
scripts/lint-spec-artifacts.py (1)
32-37:⚠️ Potential issue | 🟡 MinorTask name parsing still strips trailing
#characters from titles.
line.strip("# ")removes#from both ends, so task names containing#can be mutated in lint output.#!/bin/bash rg -n 'line\.strip\("# "\)' scripts/lint-spec-artifacts.py✅ Minimal fix
- current_task = { - "name": line.strip("# ").strip(), + task_match = re.match(r"^###\s*(.+)", line) + current_task = { + "name": task_match.group(1).strip() if task_match else line.strip(), "status": None, "unchecked": [], }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lint-spec-artifacts.py` around lines 32 - 37, The task-name parsing uses line.strip("# ") which removes '#' from both ends and corrupts titles containing '#' — change the assignment of "name" in the block where re.match(r"^### Task \d+:", line) is handled (the current_task dict initialization) to only remove the leading markdown header prefix; e.g. replace line.strip("# ").strip() with a regex-based trim that removes only leading hashes and spaces such as re.sub(r"^#+\s*", "", line).strip() (ensure re is available) so trailing '#' characters in titles are preserved.platforms/copilot/specops.instructions.md (2)
2797-2805:⚠️ Potential issue | 🟡 MinorPhase 1 Context Summary template still lacks coherence/vocabulary placeholders.
Line 114 and Line 115 require recording coherence/vocabulary results in Phase 1 Context Summary, but Line 2797–Line 2805 template still doesn’t include explicit placeholder entries.
As per coding guidelines, "Never edit generated platform output files directly (SKILL.md, specops.mdc, specops.instructions.md). Edit core/ modules or generator/templates/*.j2 instead, then regenerate with python3 generator/generate.py --all."
Also applies to: 114-116, 2914-2915
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/copilot/specops.instructions.md` around lines 2797 - 2805, The Phase 1 Context Summary template is missing explicit placeholders for recording the coherence and vocabulary results (the items referenced by the Phase 1 lines about coherence/vocabulary), but this is a generated file and should not be edited directly; instead update the generator template (add explicit placeholders named like "coherence" and "vocabulary" inside the Phase 1 Context Summary section of the corresponding Jinja2 template in generator/templates/*.j2), then run the regeneration command (python3 generator/generate.py --all) so the changes propagate to specops.instructions.md; ensure the template fields map to the code that writes the Phase 1 context (so the entries referenced by the Phase 1 coherence/vocabulary lines appear at the Phase 1 Context Summary block).
497-499:⚠️ Potential issue | 🟠 MajorWorkflow Impact text still conflicts with the actual auto-delegation algorithm.
Line 498 says auto mode triggers at “4+ pending tasks,” but Line 3224–Line 3230 defines the gate as weighted score
>= 6.Also applies to: 3224-3230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/copilot/specops.instructions.md` around lines 497 - 499, The Workflow Impact blurb for taskDelegation is inconsistent with the implementation: update the Phase 3 step 2 text so the "auto" mode description matches the actual gate used by the delegation algorithm (i.e., it triggers based on the computed weighted delegation score being >= 6), or alternatively change the algorithm to use a 4+ pending-tasks threshold; reference the taskDelegation workflow, the "auto" mode, and the algorithm's gate (weighted score >= 6) when making the change so the documentation and code stay in sync.core/workflow.md (1)
38-38:⚠️ Potential issue | 🔴 CriticalFix
implementation.mdlifecycle + non-destructive write semantics.Line 38 and Line 57 require writing/reading
implementation.mdbefore the spec-scoped file lifecycle is guaranteed, and Line 146 uses destructive write wording for a section update. This can create/read the wrong file and overwrite existing journal content on resume.🛠 Proposed wording fix
-5.5. **Context Summary (enforcement gate)**: WRITE_FILE `implementation.md` with a `## Phase 1 Context Summary` section ... +5.5. **Context Summary (enforcement gate)**: Capture Phase 1 summary data for persistence. + - If continuing an existing spec: EDIT_FILE `<specsDir>/<spec-name>/implementation.md` to upsert `## Phase 1 Context Summary`. + - If starting a new spec: persist this section immediately after `<specsDir>/<spec-name>/implementation.md` is created in Phase 2. -0. **Phase 2 entry gate**: READ_FILE `implementation.md` and verify it contains `## Phase 1 Context Summary`. +0. **Phase 2 entry gate**: After `<specsDir>/<spec-name>/implementation.md` exists, READ_FILE `<specsDir>/<spec-name>/implementation.md` and verify `## Phase 1 Context Summary` exists. -4. **Documentation check (enforcement gate)**: ... WRITE to `implementation.md` a `## Documentation Review` section ... +4. **Documentation check (enforcement gate)**: ... EDIT_FILE `<specsDir>/<spec-name>/implementation.md` to append/update `## Documentation Review` ...Also applies to: 57-57, 146-146
platforms/claude/SKILL.md (2)
45-45:⚠️ Potential issue | 🔴 CriticalPhase 1 Context Summary gating order deadlock remains unfixed.
This issue was flagged in previous reviews and marked as "Addressed in commit fe2f154", but the ordering problem persists. Line 45 (Phase 1 step 5.5) writes the Phase 1 Context Summary requiring "detected vertical" and "affected files", but these values are produced later in Phase 1 step 7 (line 47) and step 9 (line 60). Fresh specs cannot satisfy this gate without reordering.
🔧 Suggested fix: Move Context Summary write after data is available
Reorder Phase 1 steps so the Context Summary is written after vertical detection and affected files identification:
-5.5. **Context Summary (enforcement gate)**: Use the Write tool to create `implementation.md` with a `## Phase 1 Context Summary` section recording: config status, context recovery result, steering files loaded, repo map status, memory loaded, detected vertical, and affected files. This section is mandatory — proceeding to Phase 2 without writing it is a protocol breach. Use the template from `core/templates/implementation.md`. 6. Analyze the user's request to determine type (feature, bugfix, refactor) 7. Determine the project vertical: ... 8. Explore codebase to understand existing patterns and architecture 9. Identify affected files, components, and dependencies — produce a concrete list of affected file paths for `fileMatch` steering file evaluation +9.5. **Context Summary (enforcement gate)**: Use the Write tool to create `implementation.md` with a `## Phase 1 Context Summary` section recording: config status, context recovery result, steering files loaded, repo map status, memory loaded, detected vertical, and affected files. This section is mandatory — proceeding to Phase 2 without writing it is a protocol breach. Use the template from `core/templates/implementation.md`.Also update the Phase 2 entry gate reference from "step 5.5" to "step 9.5".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/claude/SKILL.md` at line 45, The Phase 1 Context Summary gating is written too early (step 5.5) because "detected vertical" and "affected files" are produced later in Phase 1 (step 7 and step 9); move the write of the Context Summary (the requirement to create implementation.md with "## Phase 1 Context Summary") to after the vertical detection and affected-files identification steps so those values are available, and update the Phase 2 entry gate reference from "step 5.5" to "step 9.5" wherever it points to the old step.
499-501:⚠️ Potential issue | 🟠 MajortaskDelegation workflow-impact text is still stale and contradicts active logic.
This issue was flagged in previous reviews but remains unfixed. Line 500 states that
"auto"triggers delegation with "4+ pending tasks", but the actual activation logic at lines 3492-3498 uses a complexity-based formula:score = sum(effort_weights) + floor(distinct_files / 5)with threshold>= 6. This mismatch can lead agents to implement incorrect delegation behavior.🔧 Suggested fix: Align workflow impact with active logic
Update the workflow impact text to match the complexity formula used in lines 3492-3498:
### Workflow Impact: taskDelegation -- **Phase 3 step 2**: If `"auto"` and 4+ pending tasks, activate delegation. If `"always"`, activate regardless. If `"never"`, use sequential execution. +- **Phase 3 step 2**: If `"auto"`, compute delegation complexity score from pending tasks (`sum(Estimated Effort weights: S=1, M=2, L=3) + floor(distinct Files to Modify / 5)`) and activate delegation when score >= 6. If `"always"`, activate regardless. If `"never"`, use sequential execution.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/claude/SKILL.md` around lines 499 - 501, The taskDelegation workflow description is out of sync with the actual activation logic; update the text under the "taskDelegation" workflow-impact section to reflect the complexity-based formula used in the code (score = sum(effort_weights) + floor(distinct_files / 5) with activation when score >= 6) instead of saying `"auto"` triggers on "4+ pending tasks"; explicitly document the formula and threshold and note that `"auto"` uses this complexity calculation while `"always"` forces delegation and `"never"` disables it.platforms/codex/SKILL.md (2)
498-500:⚠️ Potential issue | 🟠 MajorUnify task delegation auto-mode rule with the actual algorithm.
Workflow Impact: taskDelegationsays auto mode triggers at “4+ pending tasks”, but the Task Delegation logic uses weighted complexity (score >= 6). Keep one source of truth to avoid behavior drift for implementers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/codex/SKILL.md` around lines 498 - 500, The doc's "Workflow Impact: taskDelegation" description (auto = "4+ pending tasks") is inconsistent with the implementation that uses a weighted complexity check (score >= 6); update the SKILL.md entry for taskDelegation so the "auto" rule matches the algorithm by describing the weighted complexity threshold (e.g., "auto: activate delegation when computed task delegation score >= 6 based on weighted complexity"), or alternatively change the code's decision logic to use the 4+ pending-tasks rule—ensure the text references the taskDelegation "auto" mode and the implementation's score >= 6 criterion so both remain in sync.
3260-3263:⚠️ Potential issue | 🟠 MajorPrevent stranded tasks after quality-gate downgrade.
The quality gate can set a task from
Completedback toIn Progress, but the orchestrator selects onlyPendingtasks (Line 3254), so downgraded tasks can stall indefinitely.Suggested protocol fix
- a.5. **Quality gate** (if status is `Completed`): Check for degradation signals before accepting the result: + a.5. **Quality gate** (if status is `Completed`): Check for degradation signals before accepting the result: - **File existence**: ... - **Checkbox consistency**: ... - **Session Log presence**: ... + - If any quality check fails, immediately re-dispatch the same task (do not continue to next ready task), or set it to `Blocked` with a blocker reason. Do not leave it in `In Progress` while selecting a different task.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/codex/SKILL.md` around lines 3260 - 3263, The quality-gate downgrade currently moves tasks from "Completed" to "In Progress" which can leave them stranded because the orchestrator only picks "Pending" tasks; modify the quality-gate logic so that when any of the checks (Files to Modify existence, Acceptance Criteria/Tests Required checkboxes, Session Log in implementation.md) fail it sets the task status to "Pending" (or alternatively change the orchestrator's selector to include "In Progress"); update the code that appends the fallback "Task N: completed by delegate..." entry in implementation.md to also set the task to "Pending" rather than "In Progress" so the orchestrator will re-pick it.
🧹 Nitpick comments (3)
CHECKSUMS.sha256 (1)
11-17: Run a security-focused review pass before merge.These checksum updates include multiple security-sensitive artifacts (
core/workflow.md, Claude skill/plugin manifests, hooks). Please run a dedicated security review pass for this PR before merging.Based on learnings "Security-sensitive files require extra scrutiny when modified (core/workflow.md, ... .claude-plugin/plugin.json, .claude-plugin/marketplace.json, hooks/pre-commit, hooks/pre-push). Consider running /security-review before pushing."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHECKSUMS.sha256` around lines 11 - 17, This change updates checksums for several security-sensitive files (core/workflow.md, .claude-plugin/plugin.json, .claude-plugin/marketplace.json, hooks/pre-commit, hooks/pre-push, scripts/install-hooks.sh), so before merging run a dedicated security review: execute the repository's security review workflow (e.g., run /security-review or the equivalent CI job), manually audit the listed files for unintended secrets or permission changes, verify hook scripts (hooks/pre-commit, hooks/pre-push) and install script (scripts/install-hooks.sh) are safe, confirm plugin manifests (.claude-plugin/plugin.json and marketplace.json) do not expose credentials or unsafe integrations, and update the PR with the security-review results and an explicit sign-off comment referencing these files and the checksum changes.core/workflow.md (1)
211-217: Run an explicit security review before merge for this workflow change set.
core/workflow.mdis a security-sensitive control surface; these new routing/enforcement gates are worth a dedicated/security-reviewpass before release.Based on learnings: Security-sensitive files require extra scrutiny when modified (including
core/workflow.md) and should be security-reviewed before pushing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/workflow.md` around lines 211 - 217, Add a mandatory security review step before merging this change to core/workflow.md: update the Post-plan acceptance gate section and repository merge policy to require an explicit /security-review approval and checklist for changes to security-sensitive control surfaces (mention core/workflow.md, From Plan Mode, and FILE_EXISTS(`.specops.json`) as examples), and ensure CI or PR templates enforce this (e.g., a REQUIRED security-review label or a blocking CI job) so edits cannot be merged until a documented security reviewer has signed off.platforms/claude/SKILL.md (1)
2154-2154: Clarify path validation logic in From Plan Mode.Line 2154 instructs: "reject absolute paths (starting with
/), paths containing../traversal sequences, and paths outside the project root." The first two checks already ensure paths are within the project root, making the third clause redundant. Additionally, "outside the project root" is not defined procedurally — should this be a filesystem check after path resolution, or is it implied by the../and absolute path rejections?♻️ Proposed clarification
Clarify the validation logic to remove ambiguity:
-for each file path mentioned in the plan, validate the path before reading: reject absolute paths (starting with `/`), paths containing `../` traversal sequences, and paths outside the project root. For each valid relative path, check FILE_EXISTS(`<path>`) and if it exists Use the Read tool to read(`<path>`) to examine its current content and identify any additional affected files not already listed. Skip invalid or non-existent paths with a warning in the mapping summary. +for each file path mentioned in the plan, validate the path before reading: reject absolute paths (starting with `/`) and reject paths containing `../` traversal sequences. These checks ensure paths are relative and confined to the project root. For each valid relative path, check FILE_EXISTS(`<path>`) and if it exists Use the Read tool to read(`<path>`) to examine its current content and identify any additional affected files not already listed. Skip invalid or non-existent paths with a warning in the mapping summary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/claude/SKILL.md` at line 2154, Update the "Detect vertical and codebase context" step to remove the redundant "outside the project root" clause and make path validation procedural: explicitly reject paths starting with "/" and any containing "../", then normalize/resolve the candidate relative path and perform a filesystem containment check against the project root (e.g., ensure resolvedPath.startsWith(projectRoot) concept) before calling FILE_EXISTS or Read; reference the step heading "Detect vertical and codebase context" and the tools FILE_EXISTS and Read when editing the text to clarify the exact order: reject absolute and traversal patterns, resolve the path, assert resolved path is within project root, then check FILE_EXISTS and Read.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/resume-plan.md:
- Around line 25-33: Add explicit language identifiers to the three fenced code
blocks in the resume-plan content: change the block that starts with "Available
plans (most recent first):" to use ```text, change the block that begins
"Warning: The following files referenced in the plan may no longer exist:" to
use ```text, and change the block that shows "Plan: <PLAN_TITLE>" (the separator
block with "========================================") to use ```text so the
MD040 lint warnings are resolved.
- Around line 38-40: The code currently accepts any existing full path in
ARGUMENTS as PLAN_PATH; restrict this by canonicalizing both the candidate path
and the allowed plans directory (e.g., PLANS_DIR="$HOME/.claude/plans") with
realpath (or readlink -f) and only accept the candidate if its realpath starts
with the PLANS_DIR realpath; if the containment check fails, treat it as "not
found" and print the same "Plan file not found: $ARGUMENTS..." message and stop.
Ensure you reference/modify the branch that sets PLAN_PATH from ARGUMENTS and
add the realpath containment check using variables like PLAN_PATH, ARGUMENTS and
PLANS_DIR.
In @.specops/plan-to-specops-automation/implementation.md:
- Around line 16-25: The markdown tables under the initial decision table, the
"## Deviations from Design" table, and the "## Blockers Encountered" table are
not followed by a blank line; add a single trailing blank line after each table
block so there is an empty line between the end of each table and the next
heading or paragraph to satisfy markdownlint MD058 (update the tables near the
headings "## Deviations from Design" and "## Blockers Encountered" accordingly).
In `@core/from-plan.md`:
- Around line 19-24: The current validation only blocks "../" and non-.md
extensions but still allows absolute or out-of-scope paths; update the path
validation used before FILE_EXISTS and READ_FILE to (1) normalize/resolve the
provided path, (2) reject absolute paths or any path whose resolved absolute
form is outside the intended base directory (e.g., ensure
resolvedPath.startsWith(baseDir)), (3) keep rejecting any path containing ".."
segments and any path not ending in ".md", and (4) if the resolved path fails
FILE_EXISTS, call NOTIFY_USER("Plan file not found: <path>") and stop; apply
these checks to the same validation locations referenced in the diff.
In `@platforms/codex/SKILL.md`:
- Line 44: The protocol currently enforces a mandatory "## Phase 1 Context
Summary" in implementation.md before Phase 2 but also states implementation.md
is created empty in Phase 2, causing a contradiction; update the SKILL.md flow
so implementation.md is created/initialized in Phase 1 (using
core/templates/implementation.md) and the Phase 1 gate writes the "## Phase 1
Context Summary" there, then allow Phase 2 to append or modify it rather than
creating a new empty file; update any references to the "Context Summary
(enforcement gate)" step and the Phase 2 creation step to reflect that
implementation.md is initialized in Phase 1 and that Phase 2 operates on the
existing file.
In `@platforms/cursor/specops.mdc`:
- Line 64: The Phase 2 entry gate currently only looks for the header "## Phase
1 Context Summary" in implementation.md which allows template/placeholder-only
files to pass; update the validator that enforces the Phase 2 gate (the logic
that reads implementation.md and checks for the header) to instead verify
required bullet keys and non-placeholder values under that heading (e.g., ensure
bullets like "Problem Statement:", "Stakeholders:", "Constraints:" exist and are
not empty or equal to common placeholders like "TBD"/"TODO"/"<insert>").
Implement robust checks for non-placeholder content (trimmed length > N and not
matching placeholder regex) and return the specified STOP behavior if any
required bullet is missing or invalid; make these changes in the core/ or
generator/templates/*.j2 code that produces/validates implementation.md (do NOT
edit specops.mdc directly), then regenerate outputs with python3
generator/generate.py --all.
- Around line 2133-2137: The RUN_COMMAND that builds the shell pipeline to find
recent plan files uses an unquoted <planFileDirectory> (the ls -t ... | head -5
invocation) which allows unsafe interpolation for paths with spaces or
metacharacters; edit the RUN_COMMAND entry in the from-plan source so the
directory variable is properly quoted while preserving the trailing *.md glob
(i.e., quote the directory portion only so the wildcard still expands), then
regenerate the platform artifacts with the generator (python3
generator/generate.py --all) and commit the updated generated outputs.
In `@scripts/lint-spec-artifacts.py`:
- Around line 155-166: The linter crashes when non-string types appear in
spec["specopsCreatedWith"] or spec["specopsUpdatedWith"]; update the checks
around created_with and the updated_with/value handling to first verify
isinstance(created_with, str) and isinstance(value, str) (or
isinstance(updated_with, str)) before calling created_with.split(".") or
semver_re.match(value), and treat non-strings the same as falsy/unknown by
continuing/skipping the entry; adjust the existing truthiness checks so only
string values reach the string operations (references: variables created_with,
updated_with/value, and semver_re).
- Around line 274-277: The lint_task_tracking function currently treats any spec
with empty/"unknown" created_with as legacy and uses the has_any_valid heuristic
to skip enforcement; update it to properly gate legacy behavior by parsing and
comparing the spec's created_with semver (e.g., treat spec as legacy only when
created_with < "1.3.0") instead of string emptiness, remove the has_any_valid
short-circuit so modern specs are always validated, and change the behavior to
iterate each task and emit a lint error for every task missing or having an
invalid IssueID (referencing created_with, has_any_valid, IssueID, and
lint_task_tracking to locate the logic and use the existing lint reporting
helper the file already uses to flag individual task issues).
---
Duplicate comments:
In `@platforms/claude/SKILL.md`:
- Line 45: The Phase 1 Context Summary gating is written too early (step 5.5)
because "detected vertical" and "affected files" are produced later in Phase 1
(step 7 and step 9); move the write of the Context Summary (the requirement to
create implementation.md with "## Phase 1 Context Summary") to after the
vertical detection and affected-files identification steps so those values are
available, and update the Phase 2 entry gate reference from "step 5.5" to "step
9.5" wherever it points to the old step.
- Around line 499-501: The taskDelegation workflow description is out of sync
with the actual activation logic; update the text under the "taskDelegation"
workflow-impact section to reflect the complexity-based formula used in the code
(score = sum(effort_weights) + floor(distinct_files / 5) with activation when
score >= 6) instead of saying `"auto"` triggers on "4+ pending tasks";
explicitly document the formula and threshold and note that `"auto"` uses this
complexity calculation while `"always"` forces delegation and `"never"` disables
it.
In `@platforms/codex/SKILL.md`:
- Around line 498-500: The doc's "Workflow Impact: taskDelegation" description
(auto = "4+ pending tasks") is inconsistent with the implementation that uses a
weighted complexity check (score >= 6); update the SKILL.md entry for
taskDelegation so the "auto" rule matches the algorithm by describing the
weighted complexity threshold (e.g., "auto: activate delegation when computed
task delegation score >= 6 based on weighted complexity"), or alternatively
change the code's decision logic to use the 4+ pending-tasks rule—ensure the
text references the taskDelegation "auto" mode and the implementation's score >=
6 criterion so both remain in sync.
- Around line 3260-3263: The quality-gate downgrade currently moves tasks from
"Completed" to "In Progress" which can leave them stranded because the
orchestrator only picks "Pending" tasks; modify the quality-gate logic so that
when any of the checks (Files to Modify existence, Acceptance Criteria/Tests
Required checkboxes, Session Log in implementation.md) fail it sets the task
status to "Pending" (or alternatively change the orchestrator's selector to
include "In Progress"); update the code that appends the fallback "Task N:
completed by delegate..." entry in implementation.md to also set the task to
"Pending" rather than "In Progress" so the orchestrator will re-pick it.
In `@platforms/copilot/specops.instructions.md`:
- Around line 2797-2805: The Phase 1 Context Summary template is missing
explicit placeholders for recording the coherence and vocabulary results (the
items referenced by the Phase 1 lines about coherence/vocabulary), but this is a
generated file and should not be edited directly; instead update the generator
template (add explicit placeholders named like "coherence" and "vocabulary"
inside the Phase 1 Context Summary section of the corresponding Jinja2 template
in generator/templates/*.j2), then run the regeneration command (python3
generator/generate.py --all) so the changes propagate to
specops.instructions.md; ensure the template fields map to the code that writes
the Phase 1 context (so the entries referenced by the Phase 1
coherence/vocabulary lines appear at the Phase 1 Context Summary block).
- Around line 497-499: The Workflow Impact blurb for taskDelegation is
inconsistent with the implementation: update the Phase 3 step 2 text so the
"auto" mode description matches the actual gate used by the delegation algorithm
(i.e., it triggers based on the computed weighted delegation score being >= 6),
or alternatively change the algorithm to use a 4+ pending-tasks threshold;
reference the taskDelegation workflow, the "auto" mode, and the algorithm's gate
(weighted score >= 6) when making the change so the documentation and code stay
in sync.
In `@scripts/lint-spec-artifacts.py`:
- Around line 32-37: The task-name parsing uses line.strip("# ") which removes
'#' from both ends and corrupts titles containing '#' — change the assignment of
"name" in the block where re.match(r"^### Task \d+:", line) is handled (the
current_task dict initialization) to only remove the leading markdown header
prefix; e.g. replace line.strip("# ").strip() with a regex-based trim that
removes only leading hashes and spaces such as re.sub(r"^#+\s*", "",
line).strip() (ensure re is available) so trailing '#' characters in titles are
preserved.
---
Nitpick comments:
In `@CHECKSUMS.sha256`:
- Around line 11-17: This change updates checksums for several
security-sensitive files (core/workflow.md, .claude-plugin/plugin.json,
.claude-plugin/marketplace.json, hooks/pre-commit, hooks/pre-push,
scripts/install-hooks.sh), so before merging run a dedicated security review:
execute the repository's security review workflow (e.g., run /security-review or
the equivalent CI job), manually audit the listed files for unintended secrets
or permission changes, verify hook scripts (hooks/pre-commit, hooks/pre-push)
and install script (scripts/install-hooks.sh) are safe, confirm plugin manifests
(.claude-plugin/plugin.json and marketplace.json) do not expose credentials or
unsafe integrations, and update the PR with the security-review results and an
explicit sign-off comment referencing these files and the checksum changes.
In `@core/workflow.md`:
- Around line 211-217: Add a mandatory security review step before merging this
change to core/workflow.md: update the Post-plan acceptance gate section and
repository merge policy to require an explicit /security-review approval and
checklist for changes to security-sensitive control surfaces (mention
core/workflow.md, From Plan Mode, and FILE_EXISTS(`.specops.json`) as examples),
and ensure CI or PR templates enforce this (e.g., a REQUIRED security-review
label or a blocking CI job) so edits cannot be merged until a documented
security reviewer has signed off.
In `@platforms/claude/SKILL.md`:
- Line 2154: Update the "Detect vertical and codebase context" step to remove
the redundant "outside the project root" clause and make path validation
procedural: explicitly reject paths starting with "/" and any containing "../",
then normalize/resolve the candidate relative path and perform a filesystem
containment check against the project root (e.g., ensure
resolvedPath.startsWith(projectRoot) concept) before calling FILE_EXISTS or
Read; reference the step heading "Detect vertical and codebase context" and the
tools FILE_EXISTS and Read when editing the text to clarify the exact order:
reject absolute and traversal patterns, resolve the path, assert resolved path
is within project root, then check FILE_EXISTS and Read.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6a46199-2f4e-40ec-a790-18c18b9301a9
📒 Files selected for processing (19)
.claude/commands/resume-plan.md.specops/index.json.specops/memory/context.md.specops/plan-to-specops-automation/design.md.specops/plan-to-specops-automation/implementation.md.specops/plan-to-specops-automation/requirements.md.specops/plan-to-specops-automation/spec.json.specops/plan-to-specops-automation/tasks.mdCHECKSUMS.sha256core/from-plan.mdcore/workflow.mdgenerator/validate.pyplatforms/claude/SKILL.mdplatforms/claude/platform.jsonplatforms/codex/SKILL.mdplatforms/copilot/specops.instructions.mdplatforms/cursor/specops.mdcscripts/lint-spec-artifacts.pyskills/specops/SKILL.md
✅ Files skipped from review due to trivial changes (2)
- .specops/plan-to-specops-automation/requirements.md
- .specops/plan-to-specops-automation/spec.json
Comment on lines
+25
to
+33
| ``` | ||
| Available plans (most recent first): | ||
|
|
||
| # Plan file Last modified Title | ||
| 1 serene-hugging-orbit.md 2025-03-18 23:14 Plan: Resolve PR #56 Conflicts | ||
| 2 majestic-knitting-haven.md 2025-03-17 14:22 Plan: Update README | ||
| ... | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
Static analysis is correct here: Line 25, Line 94, and Line 109 use fenced blocks without a language, triggering MD040.
Suggested fix
-```
+```text
Available plans (most recent first):
@@
-```
+```text
Warning: The following files referenced in the plan may no longer exist:
@@
-```
+```text
========================================
Plan: <PLAN_TITLE>Also applies to: 94-101, 109-117
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 25-25: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/resume-plan.md around lines 25 - 33, Add explicit language
identifiers to the three fenced code blocks in the resume-plan content: change
the block that starts with "Available plans (most recent first):" to use
```text, change the block that begins "Warning: The following files referenced
in the plan may no longer exist:" to use ```text, and change the block that
shows "Plan: <PLAN_TITLE>" (the separator block with
"========================================") to use ```text so the MD040 lint
warnings are resolved.
Comment on lines
+16
to
+25
| | # | Decision | Rationale | Task | Timestamp | | ||
| |---|----------|-----------|------|-----------| | ||
|
|
||
| ## Deviations from Design | ||
| | Planned | Actual | Reason | Task | | ||
| |---------|--------|--------|------| | ||
|
|
||
| ## Blockers Encountered | ||
| | Blocker | Resolution | Impact | Task | | ||
| |---------|------------|--------|------| |
There was a problem hiding this comment.
Add blank separators around table blocks (MD058).
The tables are immediately followed by the next section heading without a trailing blank line, which triggers markdownlint MD058.
🧹 Suggested markdown fix
## Decision Log
| # | Decision | Rationale | Task | Timestamp |
|---|----------|-----------|------|-----------|
+
## Deviations from Design
| Planned | Actual | Reason | Task |
|---------|--------|--------|------|
+
## Blockers Encountered
| Blocker | Resolution | Impact | Task |
|---------|------------|--------|------|📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | # | Decision | Rationale | Task | Timestamp | | |
| |---|----------|-----------|------|-----------| | |
| ## Deviations from Design | |
| | Planned | Actual | Reason | Task | | |
| |---------|--------|--------|------| | |
| ## Blockers Encountered | |
| | Blocker | Resolution | Impact | Task | | |
| |---------|------------|--------|------| | |
| | # | Decision | Rationale | Task | Timestamp | | |
| |---|----------|-----------|------|-----------| | |
| ## Deviations from Design | |
| | Planned | Actual | Reason | Task | | |
| |---------|--------|--------|------| | |
| ## Blockers Encountered | |
| | Blocker | Resolution | Impact | Task | | |
| |---------|------------|--------|------| |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 16-16: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 20-20: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 24-24: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.specops/plan-to-specops-automation/implementation.md around lines 16 - 25,
The markdown tables under the initial decision table, the "## Deviations from
Design" table, and the "## Blockers Encountered" table are not followed by a
blank line; add a single trailing blank line after each table block so there is
an empty line between the end of each table and the next heading or paragraph to
satisfy markdownlint MD058 (update the tables near the headings "## Deviations
from Design" and "## Blockers Encountered" accordingly).
platforms/cursor/specops.mdc
Outdated
|
|
||
| **Phase 2: Create Specification** | ||
|
|
||
| 0. **Phase 2 entry gate**: Read the file at `implementation.md` and verify it contains `## Phase 1 Context Summary`. If missing, STOP — return to Phase 1 step 5.5 and write it. Proceeding without the Context Summary is a protocol breach. |
There was a problem hiding this comment.
Phase 2 entry gate is too weak and can be bypassed by template-only content.
Line 64 only checks for the ## Phase 1 Context Summary heading, so placeholder-only implementation.md content can pass without actual context capture. Consider validating required bullets (and non-placeholder values) instead of heading presence only.
As per coding guidelines "Never edit generated platform output files directly (SKILL.md, specops.mdc, specops.instructions.md). Edit core/ modules or generator/templates/*.j2 instead, then regenerate with python3 generator/generate.py --all."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/cursor/specops.mdc` at line 64, The Phase 2 entry gate currently
only looks for the header "## Phase 1 Context Summary" in implementation.md
which allows template/placeholder-only files to pass; update the validator that
enforces the Phase 2 gate (the logic that reads implementation.md and checks for
the header) to instead verify required bullet keys and non-placeholder values
under that heading (e.g., ensure bullets like "Problem Statement:",
"Stakeholders:", "Constraints:" exist and are not empty or equal to common
placeholders like "TBD"/"TODO"/"<insert>"). Implement robust checks for
non-placeholder content (trimmed length > N and not matching placeholder regex)
and return the specified STOP behavior if any required bullet is missing or
invalid; make these changes in the core/ or generator/templates/*.j2 code that
produces/validates implementation.md (do NOT edit specops.mdc directly), then
regenerate outputs with python3 generator/generate.py --all.
platforms/cursor/specops.mdc
Outdated
Comment on lines
+2133
to
+2137
| - Run the terminal command(`ls -t <planFileDirectory>/*.md 2>/dev/null | head -5`) to find the 5 most recently modified plan files. | ||
| - If no files found, fall through to Branch D. | ||
| - If `canAskInteractive`: present the file list to the user with modification dates and Ask the user: "Which plan would you like to convert? Enter a number, or paste a plan below." | ||
| - If `canAskInteractive` is false: Tell the user with the list of discovered plan files and stop ("From Plan mode found these recent plans but requires interactive input to select one."). | ||
| - Once the user selects a file, validate the path (no `../`, must be `.md`, FILE_EXISTS check) and Read the file at it. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "ls -t <planFileDirectory>/\\*\\.md" platforms/cursor/specops.mdcRepository: sanmak/specops
Length of output: 197
🏁 Script executed:
fd -t f "\.j2$" generator/templates/Repository: sanmak/specops
Length of output: 177
🏁 Script executed:
rg -l "planFileDirectory" core/ generator/Repository: sanmak/specops
Length of output: 97
🏁 Script executed:
rg -n "ls -t.*planFileDirectory" core/ generator/Repository: sanmak/specops
Length of output: 200
🏁 Script executed:
sed -n '20,35p' core/from-plan.mdRepository: sanmak/specops
Length of output: 1555
Fix unquoted path variable in shell command — edit source, then regenerate.
In core/from-plan.md line 26, quote <planFileDirectory> in the RUN_COMMAND to prevent unsafe interpolation with paths containing spaces or shell metacharacters:
Suggested fix
- - RUN_COMMAND(`ls -t <planFileDirectory>/*.md 2>/dev/null | head -5`) to find the 5 most recently modified plan files.
+ - RUN_COMMAND(`ls -t "<planFileDirectory>"/*.md 2>/dev/null | head -5`) to find the 5 most recently modified plan files.After editing core/from-plan.md, regenerate platform files with python3 generator/generate.py --all and commit the updated generated outputs (platforms/*/specops.mdc, SKILL.md, specops.instructions.md).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/cursor/specops.mdc` around lines 2133 - 2137, The RUN_COMMAND that
builds the shell pipeline to find recent plan files uses an unquoted
<planFileDirectory> (the ls -t ... | head -5 invocation) which allows unsafe
interpolation for paths with spaces or metacharacters; edit the RUN_COMMAND
entry in the from-plan source so the directory variable is properly quoted while
preserving the trailing *.md glob (i.e., quote the directory portion only so the
wildcard still expands), then regenerate the platform artifacts with the
generator (python3 generator/generate.py --all) and commit the updated generated
outputs.
Comment on lines
+155
to
+166
| created_with = spec.get("specopsCreatedWith", "") | ||
| if not created_with or created_with == "unknown": | ||
| continue | ||
| # Parse version — skip if < 1.4.0 | ||
| try: | ||
| parts = [int(x) for x in created_with.split(".")] | ||
| if len(parts) == 3 and ( | ||
| parts[0] < 1 or (parts[0] == 1 and parts[1] < 4) | ||
| ): | ||
| continue | ||
| except (ValueError, IndexError): | ||
| continue |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'created_with\.split|semver_re\.match\(value\)' scripts/lint-spec-artifacts.pyRepository: sanmak/specops
Length of output: 170
🏁 Script executed:
wc -l scripts/lint-spec-artifacts.pyRepository: sanmak/specops
Length of output: 92
🏁 Script executed:
cat -n scripts/lint-spec-artifacts.py | sed -n '150,220p'Repository: sanmak/specops
Length of output: 2952
🏁 Script executed:
python3 << 'EOF'
# Test if non-string values raise exceptions
import re
# Test 1: split() on non-string
try:
value = 123
parts = value.split(".")
print("split() on int: No exception")
except AttributeError as e:
print(f"split() on int: AttributeError - {e}")
# Test 2: split() on dict
try:
value = {"version": "1.0"}
parts = value.split(".")
print("split() on dict: No exception")
except AttributeError as e:
print(f"split() on dict: AttributeError - {e}")
# Test 3: regex match on non-string
try:
semver_re = re.compile(r'^\d+\.\d+\.\d+$')
value = 123
result = semver_re.match(value)
print("match() on int: No exception")
except TypeError as e:
print(f"match() on int: TypeError - {e}")
# Test 4: regex match on dict
try:
value = {"version": "1.0"}
result = semver_re.match(value)
print("match() on dict: No exception")
except TypeError as e:
print(f"match() on dict: TypeError - {e}")
EOFRepository: sanmak/specops
Length of output: 355
Non-string version fields in spec.json can crash lint execution.
If malformed spec.json contains non-string values for specopsCreatedWith or specopsUpdatedWith (e.g., 123, {}, []), the linter crashes at lines 160 and 215:
- Line 160:
created_with.split(".")raisesAttributeErroron non-strings - Line 215:
semver_re.match(value)raisesTypeErroron non-strings
The truthiness checks at lines 156 and 209-214 do not filter non-string types (e.g., 0, False, numeric values pass through). Add isinstance() checks before string operations:
Suggested fix
created_with = spec.get("specopsCreatedWith", "")
if not created_with or created_with == "unknown":
continue
+ if not isinstance(created_with, str):
+ continue
for field in ["specopsCreatedWith", "specopsUpdatedWith"]:
value = spec.get(field)
if value is None:
continue
if value == "unknown":
continue
+ if not isinstance(value, str):
+ continue
if not semver_re.match(value):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/lint-spec-artifacts.py` around lines 155 - 166, The linter crashes
when non-string types appear in spec["specopsCreatedWith"] or
spec["specopsUpdatedWith"]; update the checks around created_with and the
updated_with/value handling to first verify isinstance(created_with, str) and
isinstance(value, str) (or isinstance(updated_with, str)) before calling
created_with.split(".") or semver_re.match(value), and treat non-strings the
same as falsy/unknown by continuing/skipping the entry; adjust the existing
truthiness checks so only string values reach the string operations (references:
variables created_with, updated_with/value, and semver_re).
…ing, path containment, task stranding - Fix lint version thresholds: use <= 1.3.0 (specs at that version predate enforcement) - Upgrade invalid version format from WARN to ERROR (now fails CI) - Add version gate and remove has_any_valid bypass in lint_task_tracking - Add try/except around file reads in linter (lint_checkbox_staleness, lint_docs_review, lint_task_tracking) - Fix run-tests.sh: use $ROOT_DIR for .specops existence check - Fix taskDelegation Workflow Impact text: complexity score >= 6, not "4+ pending tasks" - Fix resume-plan.md: check detached HEAD before prompting about uncommitted changes - Fix implementation.md lifecycle: use EDIT_FILE upsert semantics, clarify spec-scoped paths - Harden plan path containment: reject absolute paths, validate within project root, quote shell variable - Fix task stranding: orchestrator selects In Progress (quality-gate downgrades) alongside Pending - Make COHERENCE_MARKERS more specific to avoid incidental matches - Regenerate all platform outputs and checksums
Comment on lines
+316
to
+356
| current_task_name = None | ||
| current_priority = None | ||
| current_issue_id = None | ||
|
|
||
| for line in content.split("\n"): | ||
| task_match = re.match(r"^### Task \d+:\s*(.+)", line) | ||
| if task_match: | ||
| if current_task_name: | ||
| task_entries.append((current_task_name, current_priority, current_issue_id)) | ||
| current_task_name = task_match.group(1).strip() | ||
| current_priority = None | ||
| current_issue_id = None | ||
| continue | ||
|
|
||
| priority_match = re.match(r"\*\*Priority:\*\*\s*(.+)", line) | ||
| if priority_match: | ||
| current_priority = priority_match.group(1).strip() | ||
| continue | ||
|
|
||
| issue_match = re.match(r"\*\*IssueID:\*\*\s*(.+)", line) | ||
| if issue_match: | ||
| current_issue_id = issue_match.group(1).strip() | ||
| continue | ||
|
|
||
| if current_task_name: | ||
| task_entries.append((current_task_name, current_priority, current_issue_id)) | ||
|
|
||
| # Filter to eligible tasks (High/Medium priority) | ||
| eligible = [(n, p, i) for n, p, i in task_entries if p in ("High", "Medium")] | ||
| if not eligible: | ||
| continue | ||
|
|
||
| # Flag tasks with missing/invalid IssueIDs | ||
| for task_name, priority, issue_id in eligible: | ||
| if not issue_id or not valid_issue_re.match(issue_id): | ||
| errors.append( | ||
| f" {spec_name} > {task_name}: " | ||
| f"{priority}-priority task missing valid IssueID " | ||
| f"(got '{issue_id or 'None'}', " | ||
| f"taskTracking={task_tracking})" | ||
| ) |
There was a problem hiding this comment.
lint_task_tracking ignores per-task status
The manual parsing loop at lines 320–341 extracts name, priority, and issue_id for each task, but never captures **Status:**. As a result, eligible at line 344 includes all High/Medium tasks regardless of their individual status — including any tasks that are still Pending or In Progress.
While only completed specs (spec-level "status": "completed") are evaluated, a spec can technically reach completed before every task inside it has had an IssueID recorded (e.g., if an agent set the spec status prematurely, or if the spec was created before the tracking gate existed). In those edge cases the linter will produce false-positive errors for tasks that were legitimately never tracked.
Contrast with lint_checkbox_staleness, which uses parse_tasks() and explicitly gates on task["status"].lower() == "completed". lint_task_tracking should apply the same per-task filter:
# After building task_entries, also capture status per task:
# task_entries.append((current_task_name, current_priority, current_issue_id, current_status))
# Then filter:
eligible = [
(n, p, i)
for n, p, i, s in task_entries
if p in ("High", "Medium") and (s or "").lower() == "completed"
]This makes the enforcement behaviour consistent across all four checks.
Comment on lines
+83
to
+93
| - If the merge fails with conflicts: | ||
| - Report: "Merge conflicts detected while syncing with `<MAIN_BRANCH>`. Resolve conflicts first (consider `/resolve-conflicts`) or start from a clean branch." | ||
| - Run `git merge --abort` to restore the working tree. | ||
| - Stop. | ||
| - If the merge succeeds, report: "Merged latest `origin/<MAIN_BRANCH>` into `<CURRENT_BRANCH>`." | ||
|
|
||
| ### Step 6: Validate plan file references | ||
|
|
||
| Scan `PLAN_CONTENT` for file paths — look for paths in backtick-quoted strings, markdown links, and "Files to Modify" tables that reference project files. | ||
|
|
||
| For each referenced file path, check if the file exists in the current working tree. |
There was a problem hiding this comment.
Step 3.2 lets the user confirm they want to continue with uncommitted changes, but Step 5.3 then attempts git merge origin/<MAIN_BRANCH> --no-edit. Git will refuse to merge when the working tree is dirty and the incoming changes overlap with the modified files, returning a non-zero exit code with an error like:
error: Your local changes to the following files would be overwritten by merge:
path/to/file.md
Please commit your changes or stash them before you merge.
The command document only handles the merge-conflict failure (step 5.3 bullet 3 — "Merge conflicts detected") and calls git merge --abort. It does not catch this distinct pre-merge dirty-tree failure, so the user gets a raw git error with no guidance.
Consider adding an explicit pre-merge check when CURRENT_BRANCH != MAIN_BRANCH and uncommitted changes exist:
If the working tree has uncommitted changes AND CURRENT_BRANCH does not equal MAIN_BRANCH:
- Warn: "Uncommitted changes detected. Stashing temporarily to allow merge."
- Run `git stash --include-untracked`
- Run `git merge origin/<MAIN_BRANCH> --no-edit`
- If merge succeeds, run `git stash pop`
- If merge fails or stash pop fails, report the error and stop
Alternatively, block the merge entirely when uncommitted changes are present and instruct the user to stash before proceeding.
Comment on lines
+198
to
+232
| def lint_version_fields(specs_dir): | ||
| """Validate specopsCreatedWith and specopsUpdatedWith in spec.json.""" | ||
| semver_re = re.compile(r"^\d+\.\d+\.\d+$") | ||
| errors = [] | ||
|
|
||
| for spec_name in sorted(os.listdir(specs_dir)): | ||
| spec_dir = os.path.join(specs_dir, spec_name) | ||
| if not os.path.isdir(spec_dir): | ||
| continue | ||
|
|
||
| spec_json_path = os.path.join(spec_dir, "spec.json") | ||
| if not os.path.exists(spec_json_path): | ||
| continue | ||
|
|
||
| try: | ||
| with open(spec_json_path, "r", encoding="utf-8") as f: | ||
| spec = json.load(f) | ||
| except (json.JSONDecodeError, OSError): | ||
| errors.append(f" {spec_name}: spec.json is not valid JSON") | ||
| continue | ||
|
|
||
| for field in ["specopsCreatedWith", "specopsUpdatedWith"]: | ||
| value = spec.get(field) | ||
| if value is None: | ||
| # Absent is OK (legacy spec) | ||
| continue | ||
| if value == "unknown": | ||
| # Acceptable fallback | ||
| continue | ||
| if not semver_re.match(value): | ||
| errors.append( | ||
| f" {spec_name}: {field} has invalid version format: '{value}'" | ||
| ) | ||
|
|
||
| return errors |
There was a problem hiding this comment.
The requirements in tasks.md (Task 8, Implementation Step 5) and the acceptance criteria both state that invalid version patterns should be "reported as warnings". However, lint_version_fields appends to errors and the main() function prints FAIL: for each entry, which causes the linter to exit with code 1 — a hard failure.
This is inconsistent with the design intent. A spec with "specopsCreatedWith": "1.3.0-rc1" (a valid pre-release tag that isn't strictly semver \d+\.\d+\.\d+) would block CI even though the file content is valid and the version string is clearly meaningful.
Consider either:
- Introducing a separate
warningslist that is printed withWARN:but does not contribute to the exit code, or - Updating the task description and acceptance criteria to explicitly say "error" if hard failure is the intended behaviour.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
scripts/lint-spec-artifacts.py (3)
219-230:⚠️ Potential issue | 🟠 MajorNon-string version fields can crash the linter.
If a malformed
spec.jsoncontains non-string values forspecopsCreatedWithorspecopsUpdatedWith(e.g.,123,{},[]),semver_re.match(value)at line 227 will raiseTypeError. TheNoneand"unknown"checks don't filter out other non-string types.Suggested fix to add type check
for field in ["specopsCreatedWith", "specopsUpdatedWith"]: value = spec.get(field) if value is None: # Absent is OK (legacy spec) continue if value == "unknown": # Acceptable fallback continue + if not isinstance(value, str): + errors.append( + f" {spec_name}: {field} must be a string, got {type(value).__name__}" + ) + continue if not semver_re.match(value):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lint-spec-artifacts.py` around lines 219 - 230, The linter crashes when non-string values reach semver_re.match; modify the loop that iterates over fields ["specopsCreatedWith", "specopsUpdatedWith"] to first verify the value is a string (e.g., using isinstance(value, str)) before calling semver_re.match, and if it's not a string (and not None/"unknown") append an appropriate error to errors (using spec_name and field) indicating an invalid non-string version value; keep the existing None and "unknown" handling and only call semver_re.match when value is a string.
35-39:⚠️ Potential issue | 🟡 MinorTask name extraction still uses unsafe
strip("# ").The past review comment about
line.strip("# ")was marked as addressed, but the code at line 36 still uses this pattern.strip("# ")removes all#and space characters from both ends, not just the leading###prefix. If a task name ends with#or spaces, they would be incorrectly removed.Suggested fix using lstrip
if re.match(r"^### Task \d+:", line): if current_task: tasks.append(current_task) current_task = { - "name": line.strip("# ").strip(), + "name": line.lstrip("#").strip(), "status": None, "unchecked": [], }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lint-spec-artifacts.py` around lines 35 - 39, The task name extraction in the current_task creation still uses line.strip("# ") which incorrectly removes any trailing '#' or spaces; update the extraction in the code that builds current_task (the "name" value) to only remove the expected leading prefix (e.g., leading '#' characters and a single space) — for example use lstrip to trim leading '#' and spaces or explicitly remove a leading markdown prefix (or use a regex to strip only the leading '### ' pattern) so trailing characters in the task name are preserved.
159-174:⚠️ Potential issue | 🟡 MinorSame type safety issue in
lint_docs_review.The version parsing at line 166 has the same vulnerability to non-string
specopsCreatedWithvalues. Consider addingisinstance(created_with, str)check for consistency with the suggested fixes in other functions.Suggested fix
# Skip legacy specs (created before this gate existed) created_with = spec.get("specopsCreatedWith", "") if not created_with or created_with == "unknown": continue + if not isinstance(created_with, str): + continue # Parse version — skip if <= 1.3.0 (gate introduced at 1.3.0, specs at🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lint-spec-artifacts.py` around lines 159 - 174, In lint_docs_review, the code assumes specopsCreatedWith is a string before calling created_with.split(), which can raise when it’s non-string; update the guard around created_with (the variable retrieved from spec.get("specopsCreatedWith")) to explicitly check isinstance(created_with, str) (e.g., treat non-strings like missing/unknown and continue) before attempting to split/parse into parts, leaving the existing try/except parsing logic intact.platforms/codex/SKILL.md (1)
44-47:⚠️ Potential issue | 🟠 MajorPhase ordering contradiction remains for new specs.
The workflow creates a circular dependency for new specs:
- Phase 1 step 5.5 (line 46) says to persist the Context Summary "immediately after... implementation.md are created in Phase 2 step 2"
- Phase 2 step 0 (line 66) is numbered to run BEFORE step 2 but says "After creating... implementation.md (step 2 below)"
- Phase 2 step 2 (line 72) creates implementation.md "empty (template headers only)"
If Phase 2 step 0 truly runs before step 2 (as the numbering
0suggests), it tries to read/write a file that doesn't exist yet. If it runs after step 2 (as the text "step 2 below" suggests), the numbering is backwards and confusing.For new specs, the correct sequence should be: Phase 2 step 2 creates the file → Phase 2 step 0 verifies/writes the Context Summary using data captured in Phase 1 step 5.5. Renumber step 0 to 2.5 or 3, or explicitly state "Phase 2 entry gate runs after step 2 directory creation".
Suggested clarification
**Phase 2: Create Specification** -0. **Phase 2 entry gate**: After creating `<specsDir>/<spec-name>/` and `implementation.md` (step 2 below), Read the file at `<specsDir>/<spec-name>/implementation.md` and verify it contains `## Phase 1 Context Summary`. If missing (new spec), write the context summary now using the data captured in Phase 1 step 5.5. If the section still cannot be written, STOP — return to Phase 1 step 5.5. Proceeding without the Context Summary is a protocol breach. 1. Generate a structured spec directory in the configured `specsDir` 2. Create four core files: - `requirements.md` (or `bugfix.md` for bugs, `refactor.md` for refactors) - User stories with EARS acceptance criteria, bug analysis, or refactoring rationale - `design.md` - Technical architecture, sequence diagrams, implementation approach - `tasks.md` - Discrete, trackable implementation tasks with dependencies - `implementation.md` - Living decision journal, updated during Phase 3. Created empty (template headers only) — populated incrementally as implementation decisions arise. +2.5. **Phase 2 entry gate**: Read the file at `<specsDir>/<spec-name>/implementation.md` (created in step 2) and verify it contains `## Phase 1 Context Summary`. If missing (new spec), write the context summary now using the data captured in Phase 1 step 5.5. If the section still cannot be written, STOP — return to Phase 1 step 5.5. Proceeding without the Context Summary is a protocol breach.Also applies to: 2803-2804 (template comment should say "Populated in Phase 1 for existing specs, in Phase 2 for new specs")
Also applies to: 66-66, 72-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/codex/SKILL.md` around lines 44 - 47, The Phase ordering is contradictory: Phase 1 step 5.5 says the Context Summary for new specs is written "immediately after... implementation.md are created in Phase 2 step 2", but Phase 2 step 0 is written as occurring before step 2 while its text references "after creating... implementation.md (step 2 below)"; update the doc to make the sequence unambiguous by either renumbering Phase 2 step 0 to a position after step 2 (suggested 2.5 or 3) or explicitly state that the "Phase 2 entry gate" runs after Phase 2 step 2 creates implementation.md, and apply the same clarification to the related template comment lines (the note on "Populated in Phase 1 for existing specs, in Phase 2 for new specs") and the other occurrences referenced in your comment (the Phase 2 step 0 and step 2 descriptions) so the workflow for new specs reads: Phase 2 step 2 creates implementation.md → Phase 2 entry/check writes/verifies the Phase 1 Context Summary.platforms/copilot/specops.instructions.md (1)
2802-2811:⚠️ Potential issue | 🟡 MinorTemplate should include placeholders for coherence and vocabulary checks.
The Phase 1 Context Summary template lists 7 items (Config through Affected files), but Phase 2 steps 5.5 (line 117) and 5.6 (line 118) instruct users to record coherence check and vocabulary check results in this same section. The template should include placeholder lines for these to guide users on where to append them.
Note: Since this is a generated file, the fix should be applied to the source template in
core/templates/implementation.md, then platforms regenerated. As per coding guidelines, generated platform files should not be edited directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/copilot/specops.instructions.md` around lines 2802 - 2811, Phase 1 Context Summary is missing placeholder lines for the coherence and vocabulary checks referenced by Phase 2 steps 5.5 and 5.6; update the source template core/templates/implementation.md to add two placeholder lines (e.g., "- Coherence check: [results]" and "- Vocabulary check: [results]") under the Phase 1 Context Summary section so generated platform files will include them, then regenerate platform files rather than editing the generated platforms/copilot/specops.instructions.md directly.
🧹 Nitpick comments (6)
core/from-plan.md (1)
27-32: Consider escapingplanFileDirectoryin shell command.The RUN_COMMAND at line 28 interpolates
<planFileDirectory>directly into a shell command. IfplanFileDirectorycontains shell metacharacters (spaces, quotes, etc.), this could cause issues. The path is quoted, but ensure the platform's RUN_COMMAND implementation properly handles this.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/from-plan.md` around lines 27 - 32, The RUN_COMMAND invocation in Branch C that builds the shell command using the raw planFileDirectory (the `ls -t "<planFileDirectory>"/*.md 2>/dev/null | head -5` string) can break or be exploited if planFileDirectory contains spaces or shell metacharacters; fix by ensuring planFileDirectory is either (a) shell-escaped (e.g., use a safe quoting/escaping function) before interpolating into the command, or preferably (b) change the code path that invokes RUN_COMMAND to pass the directory and glob as separate, non-shell-joined arguments or perform the file listing with native APIs instead of a shell pipeline; also add validation/sanitization of planFileDirectory (no absolute paths, no .. components, ensure it’s a directory) before use so the READ_FILE/FILE_EXISTS checks in Branch C operate on a safe, canonicalized path.platforms/codex/SKILL.md (3)
3265-3269: Clarify "immediately re-dispatch" behavior in quality gate.Line 3269 says "If any quality check fails, immediately re-dispatch the same task (do not continue to next ready task)." However, the orchestrator loop structure (lines 3258-3278) doesn't show a short-circuit that skips back to delegation (step 4). Instead, the task is set back to
In Progress(line 3266) and the loop continues through steps 6, 6.5, and 7 before repeating from step 1.Since line 3259 prioritizes
In Progresstasks overPending, the downgraded task WILL be re-selected in the next iteration, but it's not "immediate" in the sense of skipping the rest of the loop.Suggested rewording for clarity:
- - If any quality check fails, immediately re-dispatch the same task (do not continue to next ready task). The orchestrator must re-select this task on the next loop iteration rather than leaving it stranded as `In Progress`. + - If any quality check fails, keep the task in `In Progress` state. The orchestrator's ready-set logic (step 1) prioritizes `In Progress` tasks, ensuring the downgraded task is re-selected and re-dispatched in the next loop iteration before any `Pending` tasks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/codex/SKILL.md` around lines 3265 - 3269, The sentence "If any quality check fails, immediately re-dispatch the same task (do not continue to next ready task)" is ambiguous relative to the orchestrator loop; update the Quality gate wording to explicitly state the chosen behavior: either (A) implement a short-circuit (after setting the task to In Progress, break out of the current loop and return to the Delegation step so the task is re-dispatched immediately) or (B) reword to say the task is set back to In Progress and will be prioritized and re-selected at the start of the next loop iteration (no guarantee of skipping steps 6–7). Modify the "quality gate" paragraph to use the chosen option and reference the orchestrator loop behavior so readers understand whether the system performs an immediate short-circuit or relies on next-iteration prioritization.
118-119: Consider renaming "Phase 1 Context Summary" to reflect Phase 2 additions.The section is named
## Phase 1 Context Summary(template line 2803, written in Phase 1 or Phase 2), but then Phase 2 steps 5.5 and 5.6 append Coherence Verification and Vocabulary Verification results to it (lines 118-119, 2920). This makes the name slightly misleading - it's not purely Phase 1 context; it also accumulates Phase 2 verification results.Options to improve clarity:
- Rename the section to
## Context & Verification Summaryor## Pre-Implementation Summaryto reflect that it contains both Phase 1 context and Phase 2 verification results- Keep the name but update the template comment (line 2804) to:
<!-- Populated with Phase 1 context; Phase 2 verification results appended -->- Split into two sections:
## Phase 1 Context Summaryand## Phase 2 Verification ResultsCurrent behavior is correct; this is a documentation clarity suggestion only.
Also applies to: 2909-2920
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/codex/SKILL.md` around lines 118 - 119, Rename the misleading header "## Phase 1 Context Summary" to a more accurate title (suggested: "## Context & Verification Summary") wherever it appears in SKILL.md (including the occurrences that Phase 2 steps 5.5/5.6 append to), and update any nearby template comment that documents the section to reflect that Phase 2 verification results are appended (e.g., replace the existing comment around the header with "<!-- Populated with Phase 1 context; Phase 2 verification results appended -->"); ensure any references to the old header string are updated so appended entries from the Coherence Verification and Vocabulary Verification steps still write into the renamed section.
3109-3117: Add fallback handling for missing Task Scope anchor in Pivot Check.Pre-Task Anchoring (lines 3109-3117) writes a Task Scope note to
implementation.mdbefore starting each task. The Pivot Check (line 3177) then "compare[s] the actual output against the anchored Task Scope note" to detect divergence.However, the Pivot Check doesn't explicitly handle the case where the Task Scope note is missing (e.g., old specs created before this feature, or a delegate that skipped Pre-Task Anchoring).
Suggested enhancement:
Add fallback for missing anchor
### Pivot Check -Before marking a task `Completed`, compare the actual output against the anchored Task Scope note in `implementation.md` (written during Pre-Task Anchoring) and the planned approach in `design.md` and `requirements.md`. If the implementation diverged from the anchored scope (different approach, different data format, different API, scope change), update the affected spec artifact **before** closing the task. +Before marking a task `Completed`: +1. Read the file at `implementation.md` and check for a Task Scope note for this task (written during Pre-Task Anchoring). +2. If the Task Scope note exists, compare the actual output against the anchored scope and the planned approach in `design.md` and `requirements.md`. +3. If the Task Scope note is missing (e.g., old spec, or protocol breach), fall back to comparing only against `design.md` and `requirements.md` planned approach. Print to stdout: "Warning: Task Scope anchor missing for Task N — using design.md/requirements.md as comparison baseline." +4. If the implementation diverged from the scope (different approach, different data format, different API, scope change), update the affected spec artifact **before** closing the task.Also applies to: 3177-3177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/codex/SKILL.md` around lines 3109 - 3117, The Pivot Check logic that "compares the actual output against the anchored Task Scope note" must handle a missing Task Scope anchor in implementation.md: update the Pivot Check to detect a missing or empty Task Scope note when reading implementation.md and fallback by (a) extracting a brief scope summary from the task's Acceptance Criteria / Tests Required in tasks.md (or from requirements.md/bugfix.md/refactor.md + design.md) if available, or (b) use a clear default placeholder (e.g., "No anchored scope; using acceptance criteria summary") and emit a non-fatal warning in the log, then continue with comparison; ensure the code path that reads the anchor (the Pivot Check comparator) treats null/undefined/empty strings safely and documents this fallback behavior where Pre-Task Anchoring writes the Task Scope note.platforms/copilot/specops.instructions.md (2)
500-501: Workflow Impact description is slightly imprecise.The Workflow Impact text states "compute a complexity score from pending tasks (effort weights + file count)" but the actual algorithm (lines 3229-3235) computes
score = sum(effort_weights) + floor(distinct_files / 5). The file count is divided by 5 and floored, and files are deduplicated (distinct paths).While "effort weights + file count" is reasonable shorthand, it could be more precise: "compute a weighted complexity score (sum of effort weights + distinct files ÷ 5)" to better match the algorithm.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/copilot/specops.instructions.md` around lines 500 - 501, Update the Workflow Impact description for taskDelegation to precisely match the implemented algorithm: state that when `"auto"` the system computes a weighted complexity score as score = sum(effort_weights) + floor(distinct_files / 5) (i.e., sum of effort weights plus the number of distinct file paths divided by 5, floored) and activates delegation when score >= 6; keep `"always"` and `"never"` behaviors unchanged. Reference the taskDelegation behavior and the score formula (score = sum(effort_weights) + floor(distinct_files / 5)) so the prose aligns exactly with the implementation.
2908-2924: Vocabulary Verification may over-replace terms in prose.The Vocabulary Verification step (line 118 and lines 2908-2924) instructs scanning generated spec files for prohibited default terms and replacing them with vertical-specific vocabulary. However, the instruction is automated ("scan ... replace any found terms") without distinguishing between structural template headers (e.g., "## User Stories") and incidental mentions in prose (e.g., "This feature is different from typical user stories").
Over-replacement could create awkward prose. For example, replacing "user stories" in "This feature is different from typical user stories" with "infrastructure requirements" yields "This feature is different from typical infrastructure requirements" (awkward in an infrastructure vertical).
Consider refining the instruction to target structural/template usage (section headers, table headers) rather than all occurrences, or add a note that replacement should use judgment to avoid awkward prose.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/copilot/specops.instructions.md` around lines 2908 - 2924, The Vocabulary Verification step currently replaces prohibited default terms everywhere; change it to only target structural elements—specifically Markdown headings and table headers—in generated files (requirements.md, bugfix.md, refactor.md, design.md, tasks.md) by detecting lines that start with Markdown header markers (e.g., "#", "##", "###") or table header rows and replacing those occurrences with the vertical-specific terms, leaving inline/prose text untouched; ensure the implementation.md Phase 1 Context Summary still records the outcome as `- Vocabulary check: [pass / N term(s) replaced]` and add a short note in the step that replacements apply only to headers/table headers and not to body prose.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/claude/SKILL.md`:
- Around line 3532-3537: The quality gate logic that re-dispatches failed tasks
has no retry limit and can loop forever; add a retry counter to task metadata
(e.g., an Attempt field in tasks.md) and increment it each time the
post-delegation checks (File existence for "Files to Modify", checkbox
consistency, and Session Log presence via Read on implementation.md) cause an
immediate re-dispatch; if Attempt exceeds a configurable threshold (2–3), mark
the task as Blocked (or set a blocker reason/escalate to manual intervention)
instead of re-dispatching and stop auto-reselecting it on the next loop
iteration so it won’t be immediately retried.
- Around line 2137-2142: Branch C currently uses the planFileDirectory config
value without validation; add the same containment checks applied to specsDir
when the platform config is loaded so planFileDirectory is rejected if it is an
absolute path (starts with '/'), contains path traversal sequences ('../'), or
resolves outside the project root, and ensure subsequent logic that runs the
Bash ls and the FILE_EXISTS/Read tool only executes if planFileDirectory passes
these checks; update the loader/validator that handles platform config (the code
path that reads planFileDirectory) to perform these validations and fail fast
with a clear error when invalid.
In `@platforms/copilot/specops.instructions.md`:
- Line 3176: The doc is ambiguous about handling post-delegation
deviations—update the orchestrator loop (step 5b) to include an explicit
reconciliation step that runs after all delegation completes and before Phase 4:
read the Deviations table in implementation.md, map each deviation to affected
spec artifacts (implementation.md, design.md, requirements.md), and either (A)
automatically apply the documented, reviewable spec edits or create locked,
clearly labelled review tasks for a human to approve and merge the spec changes
(since delegates cannot modify specs), or (B) if you choose not to automate,
explicitly state that deviations are only flagged and that the user MUST update
the spec artifacts manually before closing the task; adjust the Pivot Check
wording to require this orchestrator reconciliation or manual update, and update
the generator template so regenerated docs include the new orchestrator step and
unambiguous guidance about who may modify spec artifacts.
In `@scripts/lint-spec-artifacts.py`:
- Around line 289-301: The code reads specopsCreatedWith into created_with and
calls created_with.split("."), which will raise AttributeError for non-string
types; add a type check mirroring lint_version_fields by verifying created_with
is a str before doing string operations (e.g., after created_with =
spec.get("specopsCreatedWith", "") add an isinstance(created_with, str) guard
and continue for non-strings) so the subsequent parts = [int(x) ...] and the
try/except block only run for string values.
---
Duplicate comments:
In `@platforms/codex/SKILL.md`:
- Around line 44-47: The Phase ordering is contradictory: Phase 1 step 5.5 says
the Context Summary for new specs is written "immediately after...
implementation.md are created in Phase 2 step 2", but Phase 2 step 0 is written
as occurring before step 2 while its text references "after creating...
implementation.md (step 2 below)"; update the doc to make the sequence
unambiguous by either renumbering Phase 2 step 0 to a position after step 2
(suggested 2.5 or 3) or explicitly state that the "Phase 2 entry gate" runs
after Phase 2 step 2 creates implementation.md, and apply the same clarification
to the related template comment lines (the note on "Populated in Phase 1 for
existing specs, in Phase 2 for new specs") and the other occurrences referenced
in your comment (the Phase 2 step 0 and step 2 descriptions) so the workflow for
new specs reads: Phase 2 step 2 creates implementation.md → Phase 2 entry/check
writes/verifies the Phase 1 Context Summary.
In `@platforms/copilot/specops.instructions.md`:
- Around line 2802-2811: Phase 1 Context Summary is missing placeholder lines
for the coherence and vocabulary checks referenced by Phase 2 steps 5.5 and 5.6;
update the source template core/templates/implementation.md to add two
placeholder lines (e.g., "- Coherence check: [results]" and "- Vocabulary check:
[results]") under the Phase 1 Context Summary section so generated platform
files will include them, then regenerate platform files rather than editing the
generated platforms/copilot/specops.instructions.md directly.
In `@scripts/lint-spec-artifacts.py`:
- Around line 219-230: The linter crashes when non-string values reach
semver_re.match; modify the loop that iterates over fields
["specopsCreatedWith", "specopsUpdatedWith"] to first verify the value is a
string (e.g., using isinstance(value, str)) before calling semver_re.match, and
if it's not a string (and not None/"unknown") append an appropriate error to
errors (using spec_name and field) indicating an invalid non-string version
value; keep the existing None and "unknown" handling and only call
semver_re.match when value is a string.
- Around line 35-39: The task name extraction in the current_task creation still
uses line.strip("# ") which incorrectly removes any trailing '#' or spaces;
update the extraction in the code that builds current_task (the "name" value) to
only remove the expected leading prefix (e.g., leading '#' characters and a
single space) — for example use lstrip to trim leading '#' and spaces or
explicitly remove a leading markdown prefix (or use a regex to strip only the
leading '### ' pattern) so trailing characters in the task name are preserved.
- Around line 159-174: In lint_docs_review, the code assumes specopsCreatedWith
is a string before calling created_with.split(), which can raise when it’s
non-string; update the guard around created_with (the variable retrieved from
spec.get("specopsCreatedWith")) to explicitly check isinstance(created_with,
str) (e.g., treat non-strings like missing/unknown and continue) before
attempting to split/parse into parts, leaving the existing try/except parsing
logic intact.
---
Nitpick comments:
In `@core/from-plan.md`:
- Around line 27-32: The RUN_COMMAND invocation in Branch C that builds the
shell command using the raw planFileDirectory (the `ls -t
"<planFileDirectory>"/*.md 2>/dev/null | head -5` string) can break or be
exploited if planFileDirectory contains spaces or shell metacharacters; fix by
ensuring planFileDirectory is either (a) shell-escaped (e.g., use a safe
quoting/escaping function) before interpolating into the command, or preferably
(b) change the code path that invokes RUN_COMMAND to pass the directory and glob
as separate, non-shell-joined arguments or perform the file listing with native
APIs instead of a shell pipeline; also add validation/sanitization of
planFileDirectory (no absolute paths, no .. components, ensure it’s a directory)
before use so the READ_FILE/FILE_EXISTS checks in Branch C operate on a safe,
canonicalized path.
In `@platforms/codex/SKILL.md`:
- Around line 3265-3269: The sentence "If any quality check fails, immediately
re-dispatch the same task (do not continue to next ready task)" is ambiguous
relative to the orchestrator loop; update the Quality gate wording to explicitly
state the chosen behavior: either (A) implement a short-circuit (after setting
the task to In Progress, break out of the current loop and return to the
Delegation step so the task is re-dispatched immediately) or (B) reword to say
the task is set back to In Progress and will be prioritized and re-selected at
the start of the next loop iteration (no guarantee of skipping steps 6–7).
Modify the "quality gate" paragraph to use the chosen option and reference the
orchestrator loop behavior so readers understand whether the system performs an
immediate short-circuit or relies on next-iteration prioritization.
- Around line 118-119: Rename the misleading header "## Phase 1 Context Summary"
to a more accurate title (suggested: "## Context & Verification Summary")
wherever it appears in SKILL.md (including the occurrences that Phase 2 steps
5.5/5.6 append to), and update any nearby template comment that documents the
section to reflect that Phase 2 verification results are appended (e.g., replace
the existing comment around the header with "<!-- Populated with Phase 1
context; Phase 2 verification results appended -->"); ensure any references to
the old header string are updated so appended entries from the Coherence
Verification and Vocabulary Verification steps still write into the renamed
section.
- Around line 3109-3117: The Pivot Check logic that "compares the actual output
against the anchored Task Scope note" must handle a missing Task Scope anchor in
implementation.md: update the Pivot Check to detect a missing or empty Task
Scope note when reading implementation.md and fallback by (a) extracting a brief
scope summary from the task's Acceptance Criteria / Tests Required in tasks.md
(or from requirements.md/bugfix.md/refactor.md + design.md) if available, or (b)
use a clear default placeholder (e.g., "No anchored scope; using acceptance
criteria summary") and emit a non-fatal warning in the log, then continue with
comparison; ensure the code path that reads the anchor (the Pivot Check
comparator) treats null/undefined/empty strings safely and documents this
fallback behavior where Pre-Task Anchoring writes the Task Scope note.
In `@platforms/copilot/specops.instructions.md`:
- Around line 500-501: Update the Workflow Impact description for taskDelegation
to precisely match the implemented algorithm: state that when `"auto"` the
system computes a weighted complexity score as score = sum(effort_weights) +
floor(distinct_files / 5) (i.e., sum of effort weights plus the number of
distinct file paths divided by 5, floored) and activates delegation when score
>= 6; keep `"always"` and `"never"` behaviors unchanged. Reference the
taskDelegation behavior and the score formula (score = sum(effort_weights) +
floor(distinct_files / 5)) so the prose aligns exactly with the implementation.
- Around line 2908-2924: The Vocabulary Verification step currently replaces
prohibited default terms everywhere; change it to only target structural
elements—specifically Markdown headings and table headers—in generated files
(requirements.md, bugfix.md, refactor.md, design.md, tasks.md) by detecting
lines that start with Markdown header markers (e.g., "#", "##", "###") or table
header rows and replacing those occurrences with the vertical-specific terms,
leaving inline/prose text untouched; ensure the implementation.md Phase 1
Context Summary still records the outcome as `- Vocabulary check: [pass / N
term(s) replaced]` and add a short note in the step that replacements apply only
to headers/table headers and not to body prose.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6f465dc0-8250-4db3-aba2-29f7928cc643
📒 Files selected for processing (14)
.claude/commands/resume-plan.mdCHECKSUMS.sha256core/config-handling.mdcore/from-plan.mdcore/task-delegation.mdcore/workflow.mdgenerator/validate.pyplatforms/claude/SKILL.mdplatforms/codex/SKILL.mdplatforms/copilot/specops.instructions.mdplatforms/cursor/specops.mdcscripts/lint-spec-artifacts.pyscripts/run-tests.shskills/specops/SKILL.md
✅ Files skipped from review due to trivial changes (1)
- CHECKSUMS.sha256
🚧 Files skipped from review as they are similar to previous changes (4)
- scripts/run-tests.sh
- generator/validate.py
- core/task-delegation.md
- platforms/cursor/specops.mdc
Comment on lines
+2137
to
+2142
| **Branch C — Platform auto-discovery**: If no content and no path were provided, and the platform configuration includes a `planFileDirectory` field: | ||
| - Use the Bash tool to run(`ls -t "<planFileDirectory>"/*.md 2>/dev/null | head -5`) to find the 5 most recently modified plan files. | ||
| - If no files found, fall through to Branch D. | ||
| - If `canAskInteractive`: present the file list to the user with modification dates and Use the AskUserQuestion tool: "Which plan would you like to convert? Enter a number, or paste a plan below." | ||
| - If `canAskInteractive` is false: Display a message to the user with the list of discovered plan files and stop ("From Plan mode found these recent plans but requires interactive input to select one."). | ||
| - Once the user selects a file, validate the path (must remain within `<planFileDirectory>`, no absolute path, no `../`, must be `.md`, FILE_EXISTS check) and Use the Read tool to read it. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for planFileDirectory loading and validation
rg -n 'planFileDirectory' --type=json platforms/claude/platform.json
rg -n -C5 'planFileDirectory' core/from-plan.md core/config-handling.mdRepository: sanmak/specops
Length of output: 2233
🏁 Script executed:
# Search for how planFileDirectory and specsDir are used in implementation files
rg -n 'planFileDirectory' --type=md --type=py --type=sh -A 3 -B 3Repository: sanmak/specops
Length of output: 11308
🏁 Script executed:
# Search for specsDir to understand what validation pattern should be applied
rg -n 'specsDir' --type=md -A 3 -B 3 | head -100Repository: sanmak/specops
Length of output: 14172
🏁 Script executed:
# Check lines 2665-2670 in SKILL.md to see the Path Containment safety rules referenced
sed -n '2660,2675p' platforms/claude/SKILL.mdRepository: sanmak/specops
Length of output: 1615
🏁 Script executed:
# Search for path validation patterns in core files
rg -n 'path.*validat|validat.*path|containment|absolute.*path|\.\./|projectRoot' core/ --type=md -A 2Repository: sanmak/specops
Length of output: 5533
🏁 Script executed:
# Find where platform.json is loaded and how planFileDirectory is processed
rg -n 'platform\.json|planFileDirectory' --type=py --type=js --type=sh -B 3 -A 3 | head -150Repository: sanmak/specops
Length of output: 8549
🏁 Script executed:
# Check if there's a config loading module that validates paths
fd -t f -e py -e js | xargs rg -l 'platform.json|config.*load|load.*config' | head -10Repository: sanmak/specops
Length of output: 184
🏁 Script executed:
# Look for where specsDir validation actually happens in code
rg -n 'specsDir.*validat|validat.*specsDir|absolute.*path|path.*traversal' --type=py --type=js -A 2 -B 2Repository: sanmak/specops
Length of output: 1740
🏁 Script executed:
# Look at actual platform.json files to see planFileDirectory usage
for file in platforms/*/platform.json; do
echo "=== $file ==="
cat "$file" | jq . 2>/dev/null | head -30
doneRepository: sanmak/specops
Length of output: 4187
🏁 Script executed:
# Check if there's a JSON schema for platform.json that validates fields
fd -t f -e json -e schema schema* | xargs cat 2>/dev/null | grep -C 5 'planFileDirectory' || echo "Not found in schema files"Repository: sanmak/specops
Length of output: 83
🏁 Script executed:
# Search for where planFileDirectory config value is actually used/loaded at runtime
rg -n 'planFileDirectory' -A 5 -B 5 generator/ --type=pyRepository: sanmak/specops
Length of output: 573
🏁 Script executed:
# Check if platform.json schema defines validation rules for path fields
cat schema.json | jq '.properties | keys' 2>/dev/null || echo "Check schema.json structure"Repository: sanmak/specops
Length of output: 218
🏁 Script executed:
# Check if platform.json is meant to be edited by users or if it's platform-provided
rg -n 'platform\.json' README.md docs/ core/*.md --type=md -B 2 -A 2 | head -100Repository: sanmak/specops
Length of output: 2619
🏁 Script executed:
# Check if there's any load-time validation of planFileDirectory anywhere in the codebase
rg -n 'planFileDirectory' --type=py --type=js --type=sh -B 10 -A 10Repository: sanmak/specops
Length of output: 1090
🏁 Script executed:
# Verify: does the generator inject planFileDirectory into the skill output from platform.json?
head -100 generator/generate.py | grep -A 50 'def generate'Repository: sanmak/specops
Length of output: 40
🏁 Script executed:
# Check the template to see if platform config values are substituted
fd -t f -name '*.j2' | head -5 | xargs cat 2>/dev/null | grep -C 5 'planFileDirectory' || echo "Check template structure"Repository: sanmak/specops
Length of output: 330
🏁 Script executed:
# Find and examine template files
find generator/templates -name '*.j2' -type fRepository: sanmak/specops
Length of output: 177
🏁 Script executed:
# Check if platform config is passed to templates and how planFileDirectory is used
rg -n 'planFileDirectory' generator/templates/ --type=jinja2 2>/dev/null || rg -n 'planFileDirectory' generator/templates/ 2>/dev/nullRepository: sanmak/specops
Length of output: 40
🏁 Script executed:
# Check the generate.py file to see how platform config values are injected into output
sed -n '1,150p' generator/generate.pyRepository: sanmak/specops
Length of output: 5024
🏁 Script executed:
# Look for validation logic in generate.py for path values
rg -n 'def.*validat|path.*check|absolute|traversal' generator/generate.py -A 3Repository: sanmak/specops
Length of output: 40
🏁 Script executed:
# Check if planFileDirectory value in platform.json is user-configurable or just a default
rg -n 'planFileDirectory' docs/ README.md --type=md -B 2 -A 2Repository: sanmak/specops
Length of output: 40
🏁 Script executed:
# Verify if platform.json files are part of the repository or user-created/editable
git ls-files platforms/*/platform.json 2>/dev/null || find platforms -name "platform.json"Repository: sanmak/specops
Length of output: 181
🏁 Script executed:
# Check the actual usage: does the generated code read planFileDirectory from config at runtime?
rg -n 'planFileDirectory' platforms/claude/SKILL.md -B 3 -A 3 | head -50Repository: sanmak/specops
Length of output: 1846
🏁 Script executed:
# Look at core/from-plan.md to understand where planFileDirectory is sourced and if it needs validation
sed -n '1,50p' core/from-plan.mdRepository: sanmak/specops
Length of output: 4852
🏁 Script executed:
# Search for any validation that happens during generation or at platform load time
rg -n 'load_platform_config|platform.json' generator/generate.py -A 10Repository: sanmak/specops
Length of output: 1460
🏁 Script executed:
# Check if there's any documentation about platform.json as a user-editable vs platform-maintainer config
rg -n 'platform.json.*user|editable.*platform|maintain.*platform' README.md docs/ --type=mdRepository: sanmak/specops
Length of output: 40
🏁 Script executed:
# Verify what the actual threat is: at what point does planFileDirectory get used in the generated output?
rg -n 'planFileDirectory' platforms/claude/SKILL.md -B 5 -A 5 | head -80Repository: sanmak/specops
Length of output: 2007
🏁 Script executed:
# Check if platform.json files are user-editable or platform maintainer files
head -20 platforms/claude/README.md | grep -C 10 'platform.json\|edit\|config' || echo "Check README"Repository: sanmak/specops
Length of output: 70
🏁 Script executed:
# Verify: is platform.json part of platform-provided config or user-modifiable?
cat platforms/claude/README.md | head -50Repository: sanmak/specops
Length of output: 1153
🏁 Script executed:
# Final check: does core/from-plan.md document that planFileDirectory should be validated?
grep -C 10 'planFileDirectory' core/from-plan.md | head -50Repository: sanmak/specops
Length of output: 2718
Add path containment validation for planFileDirectory config value.
Branch C uses planFileDirectory from platform.json without validating it at load time. The ls command and path checks assume planFileDirectory is safe, but like specsDir, it should be validated when loaded: reject absolute paths (starting with /), path traversal sequences (../), and paths outside the project root. Apply the same containment rules documented for specsDir in core/safety.md.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/claude/SKILL.md` around lines 2137 - 2142, Branch C currently uses
the planFileDirectory config value without validation; add the same containment
checks applied to specsDir when the platform config is loaded so
planFileDirectory is rejected if it is an absolute path (starts with '/'),
contains path traversal sequences ('../'), or resolves outside the project root,
and ensure subsequent logic that runs the Bash ls and the FILE_EXISTS/Read tool
only executes if planFileDirectory passes these checks; update the
loader/validator that handles platform config (the code path that reads
planFileDirectory) to perform these validations and fail fast with a clear error
when invalid.
Comment on lines
+3532
to
3537
| a.5. **Quality gate** (if status is `Completed`): Check for degradation signals before accepting the result: | ||
| - **File existence**: For each path in the task's "Files to Modify", FILE_EXISTS the path. If any file was supposed to be created but does not exist, Display a message to the user with warning and set the task back to `In Progress` for re-evaluation. | ||
| - **Checkbox consistency**: Verify all Acceptance Criteria and Tests Required checkboxes are checked (`[x]`) for the Completed task. If any are unchecked, Display a message to the user with warning and keep the task as `In Progress`. | ||
| - **Session Log presence**: Use the Read tool to read `implementation.md`, verify a Session Log entry exists for this task. If missing, Use the Edit tool to modify `implementation.md` to append a fallback entry: `Task N: completed by delegate (no session log written — quality gate backfill)`. | ||
| - If any quality check fails, immediately re-dispatch the same task (do not continue to next ready task). The orchestrator must re-select this task on the next loop iteration rather than leaving it stranded as `In Progress`. | ||
| b. Use the Read tool to read `implementation.md` — check for new Decision Log or Deviation entries |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for retry/attempt counting in delegation and quality gate logic
rg -n -C3 'retry|attempt|max.*tries|failure.*count' --type=md core/task-delegation.md core/workflow.mdRepository: sanmak/specops
Length of output: 1020
🏁 Script executed:
#!/bin/bash
# Search for orchestrator loop structure and re-dispatch logic in SKILL.md
rg -n 'orchestrator.*loop|max.*iteration|while.*true|loop.*bound|re-dispatch|re-select' platforms/claude/SKILL.md | head -30Repository: sanmak/specops
Length of output: 961
🏁 Script executed:
#!/bin/bash
# Check for task state tracking (attempt counts, last modified, etc.) around lines 3500-3550
sed -n '3500,3550p' platforms/claude/SKILL.md | cat -nRepository: sanmak/specops
Length of output: 6162
🏁 Script executed:
#!/bin/bash
# Search for any escape hatches or failure limits in task processing
rg -n 'escape|bail|abort|max.*fail|give.*up|stranded|orphan' platforms/claude/SKILL.mdRepository: sanmak/specops
Length of output: 809
🏁 Script executed:
#!/bin/bash
# Check the full orchestrator loop structure and any loop control
sed -n '3490,3550p' platforms/claude/SKILL.md | cat -nRepository: sanmak/specops
Length of output: 6867
🏁 Script executed:
#!/bin/bash
# Check if tasks.md task entry has attempt/retry counter fields
rg -n 'attempt|retry|fail.*count|delegat.*count' platforms/claude/SKILL.md | head -20Repository: sanmak/specops
Length of output: 1042
🏁 Script executed:
#!/bin/bash
# Check Phase 4 start condition to see if there's a loop exit
sed -n '3545,3560p' platforms/claude/SKILL.md | cat -nRepository: sanmak/specops
Length of output: 1534
🏁 Script executed:
#!/bin/bash
# Verify if there's any manual intervention mechanism for stuck In Progress tasks
# or if the loop has any documented escape hatch
rg -n 'stuck|stranded|manual.*interven|pause.*delegat|Give up|escape' platforms/claude/SKILL.md | grep -i 'in progress\|quality.*gate\|re-dispatch'Repository: sanmak/specops
Length of output: 961
🏁 Script executed:
#!/bin/bash
# Check if there's a maximum attempts or failure tracking elsewhere in the workflow
rg -n 'max.*attempt|attempt.*limit|retry.*limit|3.*retries|failure.*threshold' platforms/claude/SKILL.mdRepository: sanmak/specops
Length of output: 40
Add retry limit to quality gate re-dispatch to prevent infinite loops.
The post-delegation quality gate (lines 3532-3537) re-dispatches failed tasks immediately with no retry limit. If a delegate consistently fails a quality check (e.g., Files to Modify contain incorrect paths, or acceptance criteria are not properly marked), the orchestrator will loop indefinitely re-dispatching the same task. The loop only exits when "no Pending tasks remain" (step 8), but a stuck In Progress task counts as neither and will be re-prioritized and re-dispatched on every iteration.
Add a retry counter (e.g., **Attempt:** field in tasks.md) and escalate to manual intervention after N failures (suggest 2-3 retries), or set the task to Blocked with a blocker reason indicating quality gate exhaustion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/claude/SKILL.md` around lines 3532 - 3537, The quality gate logic
that re-dispatches failed tasks has no retry limit and can loop forever; add a
retry counter to task metadata (e.g., an Attempt field in tasks.md) and
increment it each time the post-delegation checks (File existence for "Files to
Modify", checkbox consistency, and Session Log presence via Read on
implementation.md) cause an immediate re-dispatch; if Attempt exceeds a
configurable threshold (2–3), mark the task as Blocked (or set a blocker
reason/escalate to manual intervention) instead of re-dispatching and stop
auto-reselecting it on the next loop iteration so it won’t be immediately
retried.
| ### Pivot Check | ||
|
|
||
| Before marking a task `Completed`, compare the actual output against what was planned in `design.md` and `requirements.md`. If the implementation diverged from the plan (different approach, different data format, different API, scope change), update the affected spec artifact **before** closing the task. Spec artifacts that still describe the old approach after a pivot are a recurring drift class — Phase 4 checkbox verification cannot catch it because the outdated spec text has no checkboxes to fail. | ||
| Before marking a task `Completed`, compare the actual output against the anchored Task Scope note in `implementation.md` (written during Pre-Task Anchoring) and the planned approach in `design.md` and `requirements.md`. If the implementation diverged from the anchored scope (different approach, different data format, different API, scope change), update the affected spec artifact **before** closing the task. Spec artifacts that still describe the old approach after a pivot are a recurring drift class — Phase 4 checkbox verification cannot catch it because the outdated spec text has no checkboxes to fail. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for orchestrator steps that handle deviations after delegation.
# Search for "deviation" in the context of orchestrator/delegation
rg -n -C3 'orchestrator.*deviation|delegation.*complete.*deviation|Phase 4.*deviation' platforms/copilot/specops.instructions.mdRepository: sanmak/specops
Length of output: 713
🏁 Script executed:
# Read the orchestrator loop section (lines 3256-3277)
sed -n '3250,3280p' platforms/copilot/specops.instructions.mdRepository: sanmak/specops
Length of output: 4039
🏁 Script executed:
# Read around line 3298 for fuller context on "orchestrator flags deviations"
sed -n '3290,3310p' platforms/copilot/specops.instructions.mdRepository: sanmak/specops
Length of output: 1331
🏁 Script executed:
# Check Phase 4 context around line 3176 and beyond
sed -n '3170,3200p' platforms/copilot/specops.instructions.mdRepository: sanmak/specops
Length of output: 2721
🏁 Script executed:
# Search for explicit post-delegation steps or deviation handling after delegation
rg -n -C5 'delegation completes|after delegation|post-delegation' platforms/copilot/specops.instructions.mdRepository: sanmak/specops
Length of output: 1771
Clarify post-delegation deviation handling: spec artifact updates after delegation.
The orchestrator loop (step 5b) reads deviations from implementation.md, but does not explicitly update spec artifacts based on them before Phase 4. The phrase "the orchestrator flags deviations for user review after delegation completes" (line 3298) is ambiguous—it's unclear whether deviations trigger automatic spec artifact updates or are deferred to manual user review.
This creates tension with the Pivot Check section (line 3176), which states that spec artifacts must be updated before closing a task. The delegate constraint (line 3298) prevents delegates from updating spec artifacts, implying the orchestrator should handle this—but no such orchestrator step is documented.
Recommend either:
- Add an explicit orchestrator step (after all delegation completes, before Phase 4) that reviews the Deviations table in implementation.md and updates the affected spec artifacts
- Clarify that deviations are flagged for user review and deferred to manual spec artifact updates outside the orchestrator loop
Note: This file is generated; the fix should be applied to the source template in generator/templates/, then regenerated with python3 generator/generate.py --all.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/copilot/specops.instructions.md` at line 3176, The doc is ambiguous
about handling post-delegation deviations—update the orchestrator loop (step 5b)
to include an explicit reconciliation step that runs after all delegation
completes and before Phase 4: read the Deviations table in implementation.md,
map each deviation to affected spec artifacts (implementation.md, design.md,
requirements.md), and either (A) automatically apply the documented, reviewable
spec edits or create locked, clearly labelled review tasks for a human to
approve and merge the spec changes (since delegates cannot modify specs), or (B)
if you choose not to automate, explicitly state that deviations are only flagged
and that the user MUST update the spec artifacts manually before closing the
task; adjust the Pivot Check wording to require this orchestrator reconciliation
or manual update, and update the generator template so regenerated docs include
the new orchestrator step and unambiguous guidance about who may modify spec
artifacts.
Comment on lines
+289
to
+301
| created_with = spec.get("specopsCreatedWith", "") | ||
| if not created_with or created_with == "unknown": | ||
| continue | ||
| try: | ||
| parts = [int(x) for x in created_with.split(".")] | ||
| if len(parts) == 3 and ( | ||
| parts[0] < 1 | ||
| or (parts[0] == 1 and parts[1] < 3) | ||
| or (parts[0] == 1 and parts[1] == 3 and parts[2] == 0) | ||
| ): | ||
| continue | ||
| except (ValueError, IndexError): | ||
| continue |
There was a problem hiding this comment.
Add type check for created_with before string operations.
Similar to lint_version_fields, if specopsCreatedWith is a non-string type, created_with.split(".") at line 293 will raise AttributeError. The truthiness check at line 290 doesn't filter non-string types.
Suggested fix
# Skip legacy specs (created at or before 1.3.0, when task tracking
# enforcement was introduced but specs at that version predate it)
created_with = spec.get("specopsCreatedWith", "")
if not created_with or created_with == "unknown":
continue
+ if not isinstance(created_with, str):
+ continue
try:
parts = [int(x) for x in created_with.split(".")]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/lint-spec-artifacts.py` around lines 289 - 301, The code reads
specopsCreatedWith into created_with and calls created_with.split("."), which
will raise AttributeError for non-string types; add a type check mirroring
lint_version_fields by verifying created_with is a str before doing string
operations (e.g., after created_with = spec.get("specopsCreatedWith", "") add an
isinstance(created_with, str) guard and continue for non-strings) so the
subsequent parts = [int(x) ...] and the try/except block only run for string
values.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scripts/lint-spec-artifacts.py): validates checkbox staleness in completed tasks, mandatory Documentation Review section in completed specs, and semver validation for version metadata fieldsimplementation.mdrecording config, steering, memory, repo map, and vertical status before Phase 2 can startimplementation.mdlisting each doc file checked and its status before spec completion### Workflow Impactsubsections added tocore/config-handling.mdfor all behavioral config values (specReview, taskTracking, codeReview, linting, formatting, testing, autoCommit, createPR, taskDelegation, integrations)generator/validate.pyfor cross-section coherence verification in Phase 2core/task-tracking.md: scope anchor written before each task starts, referenced by strengthened Pivot Checkcore/verticals.md: post-generation scan for prohibited default terms when non-default verticals are usedChanges
scripts/lint-spec-artifacts.py— New: 3-check spec artifact linter (checkbox staleness, docs review, version validation)scripts/run-tests.sh— Added conditional spec artifact lint stepcore/workflow.md— Phase 1 step 5.5 (Context Summary), Phase 2 steps 0/5.5/5.6 (entry gate, coherence, vocabulary)core/templates/implementation.md— Added Phase 1 Context Summary and Documentation Review template sectionscore/config-handling.md— Added Workflow Impact annotations for 8 config value groupscore/task-tracking.md— Added Pre-Task Anchoring section, strengthened Pivot Checkcore/verticals.md— Added Vocabulary Verification subsection with prohibited terms tablegenerator/validate.py— Added COHERENCE_MARKERS (definition, per-platform, cross-platform)CLAUDE.md— Added lint-spec-artifacts.py to Key Commands.specops/enforcement-roadmap/— Complete spec artifacts (requirements, design, tasks, implementation, spec.json).specops/memory/— Updated context.md and patterns.json with enforcement-roadmap completionplatforms/*/,skills/,.claude-plugin/— Regenerated from updated core modulesCHECKSUMS.sha256— RegeneratedTest Plan
python3 generator/validate.py— all platforms pass including new COHERENCE_MARKERSshasum -a 256 -c CHECKSUMS.sha256— all checksums verifiedpython3 generator/generate.py --all && git diff --exit-code platforms/ skills/ .claude-plugin/— generated files freshbash scripts/run-tests.sh— all 8 tests pass (7 existing + 1 new spec artifact lint)python3 scripts/lint-spec-artifacts.py .specops— all completed specs pass cleanly (legacy specs gracefully skipped)Summary by CodeRabbit
New Features
Behavior Changes / Workflow
Tests
Greptile Summary
This PR converts advisory behavioral mechanisms in SpecOps into deterministic enforcement gates by introducing a spec artifact linter, Phase 1/4 workflow gates, COHERENCE_MARKERS for the platform validator, pre-task anchoring, vertical vocabulary verification, and a new
/resume-plancommand.Key changes:
scripts/lint-spec-artifacts.py— New 4-check linter (checkbox staleness, docs review, task IssueID, version fields);lint_task_trackingdoes not filter by per-task status, risking false positives for non-completed tasks within a completed specgenerator/validate.py—COHERENCE_MARKERSadded for cross-platform consistency; markers are now reasonably specific phrases.claude/commands/resume-plan.md— New/resume-plancommand; the merge step (Step 5) does not handle the dirty-worktree pre-merge failure scenario when the user accepted uncommitted changes; also passesPLAN_CONTENTinline to/specops from-planinstead of the more efficientPLAN_PATHfile referencecore/workflow.md— Phase 1 Context Summary gate, Phase 2 entry gate, Coherence Verification, and Vocabulary Verification steps added cleanlycore/task-delegation.md— Updated auto-delegation with complexity-based threshold (score ≥ 6) and post-delegation quality gatecore/config-handling.md,core/task-tracking.md,core/verticals.md— Clean enforcement annotations with no issues foundConfidence Score: 3/5
lint_task_trackinghas a false-positive gap due to missing per-task status filtering. The/resume-planmerge step can produce a confusing raw git error when a dirty worktree prevents the merge. These are real but non-critical issues — they affect correctness of a new enforcement gate and developer experience, not existing functionality.scripts/lint-spec-artifacts.py(task status filter inlint_task_tracking) and.claude/commands/resume-plan.md(merge failure handling in Step 5)Important Files Changed
lint_task_trackingdoesn't filter by per-task status (can produce false positives), and version-field failures are hard errors despite the design intent of "warnings"..specopsdirectory without readingspecsDirfrom.specops.jsonconfig (covered in prior threads).COHERENCE_MARKERSfor cross-platform consistency validation; markers are now reasonably specific phrases ("cross-check for contradictions","contradictions between spec sections") though generic-term fragility was noted in prior threads./resume-plancommand; merge step (Step 5) doesn't handle the dirty-worktree pre-merge failure when user accepted uncommitted changes; Step 8 passesPLAN_CONTENTinline instead of the more efficientPLAN_PATHfile reference.Sequence Diagram
sequenceDiagram participant Agent participant workflow.md participant implementation.md participant lint_spec_artifacts.py participant validate.py Note over Agent,validate.py: Phase 1 → Phase 2 enforcement flow Agent->>workflow.md: Phase 1 steps 1–5 (config, steering, repo map, memory) Agent->>implementation.md: WRITE ## Phase 1 Context Summary (step 5.5) workflow.md-->>Agent: Gate check: Context Summary present? Agent->>workflow.md: Phase 2 entry gate (step 0) — READ implementation.md workflow.md-->>Agent: Missing? STOP (protocol breach) Note over Agent,validate.py: Phase 2 spec generation Agent->>implementation.md: Generate requirements/design/tasks files Agent->>workflow.md: Coherence Verification (step 5.5) — cross-check NFRs vs requirements Agent->>workflow.md: Vocabulary Verification (step 5.6) — scan for prohibited vertical terms Agent->>implementation.md: WRITE coherence + vocabulary results Note over Agent,validate.py: Phase 4 completion gate Agent->>implementation.md: WRITE ## Documentation Review section lint_spec_artifacts.py->>implementation.md: Check 2 — ## Documentation Review present? lint_spec_artifacts.py->>tasks.md: Check 1 — stale checkboxes in completed tasks? lint_spec_artifacts.py->>tasks.md: Check 3 — valid IssueIDs (if taskTracking ≠ none)? lint_spec_artifacts.py->>spec.json: Check 4 — valid semver version fields? lint_spec_artifacts.py-->>Agent: PASS / FAIL (exit 0 / 1) Note over validate.py: CI gate validate.py->>platforms/: Check COHERENCE_MARKERS present in all 4 platform outputs validate.py-->>Agent: Cross-platform consistency PASS / FAILLast reviewed commit: "fix: address PR revi..."