Repository navigation
feat: scaffold into Codex and opencode, sharpen the review agents, and add scc update - #6
Merged
Merged
Conversation
…nly Claude Code `scc init` takes --claude (the default), --codex, or --opencode, and asks with a numbered picker when a person is at the terminal and named no flag. Unattended callers keep getting Claude Code silently, so an agent or a CI job still drives init exactly as it did. One prose source, three harnesses. The rules, skills, and review agents are written once and re-addressed per harness: paths come from a paths.Harness profile, and the header each loader parses is synthesized at render time — YAML frontmatter for Claude Code and opencode, a TOML agent role file for Codex. Shipping a copy of the template set per harness would drift, and the drift would be silent. The review agents change with it: both now run five named gates (the ticked boxes are true, the code matches the tasks, the tests, the lint, the practices no linter has an opinion about) and report in a fixed shape the orchestrator can branch on. security-review works from a method — surface, trust boundaries, reachability, deliberate attack — with the known attack classes demoted to prompts rather than the definition of done. Effort is pinned high wherever the harness expresses one; the model tier only where there is a stable alias for it. The manifest moves into the harness directory and records which harness it is: a template renders differently per harness, so an upgrade that re-renders the recorded version to reconstruct a merge base has to render it for the same one. workspace.Find accepts any harness marker, and the skills validator checks every harness directory, because the Agent Skills format is one standard. Template version 5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yCM51izVEvKywpKDgYvNM
…ting Every managed file is hashed against what this build renders and against what the manifest says scc last wrote. That splits it six ways — current, create, update, conflict, delete, orphan — plus owned for the two files that exist to be edited, and the plan is printed grouped by outcome before anything is touched. The confirmation is the feature, not decoration: update is the one command that can destroy work, and a summary the user agrees to is the difference between a tool and a surprise. --yes is that agreement given up front and is required when stdin is not a terminal, --dry-run reports and stops, and --json emits one shape whatever the run decided to do. A conflict is kept and named, never merged and never clobbered; --force is the separate decision. The kept file stays recorded at the version scc last wrote, which is precisely the base revision the three-way merge will need when it lands. That merge — plan.md phase 11.1, internal/merge — is still unbuilt: this is the honest subset of it, and reporting a conflict is strictly safer than resolving one badly. Without a flag it updates every harness the workspace was initialized for, since a repo worked on from two tools has two managed trees and leaving one behind would let them drift apart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yCM51izVEvKywpKDgYvNM
…shipped orchestration.md §6 gains the layout table and the two things that fall out of it: Codex ships no slash commands, because its custom prompts are user-global and deprecated in favor of skills, and a repo initialized for both Codex and opencode has one AGENTS.md written by whichever ran first. §12 keeps the three-way merge as the destination and states plainly what shipped instead — replace-or-keep, with the plan confirmed first — and why the conflicted file's recorded version is preserved rather than stamped forward. The "No AGENTS.md" decision is gone with its reason: it argued against buying portability to tools this product isn't for, and the product is now for them. plan.md's phase 11 note is rewritten from "deliberately not started" to what is built and what is not, and CLAUDE.md and the README follow the code: the marker is per harness, paths are methods on paths.Harness, and adding a harness is one value in paths.Harnesses(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yCM51izVEvKywpKDgYvNM
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe change adds Claude Code, Codex, and opencode support. It introduces harness-specific paths, manifests, rendering, initialization, validation, and replace-or-keep updates through the ChangesMulti-harness scaffolding and updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant CLI
participant Workspace
participant Assets
participant Manifest
Operator->>CLI: run init or update
CLI->>Workspace: discover selected or initialized harnesses
CLI->>Assets: render harness-specific workspace
Assets-->>CLI: return files and paths
CLI->>Manifest: load or save harness manifest
CLI-->>Operator: report scaffold or update results
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
internal/assets/templates/rules/routing.md (1)
18-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a language tag to the fenced example blocks.
Both fenced code blocks open with a bare ``` fence. markdownlint-cli2 flags this as MD040 (fenced-code-language) in both files. Add a language identifier, for example
text, to each fence.
internal/assets/templates/rules/routing.md#L18-L21: change the opening fence fromtotextfor thescc spec new/scc plan newexample.internal/assets/templates/rules/specs.md#L70-L74: change the opening fence fromtotextfor the requirement-delta example.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/assets/templates/rules/routing.md` around lines 18 - 21, Add the text language identifier to the opening fenced code blocks in internal/assets/templates/rules/routing.md lines 18-21 and internal/assets/templates/rules/specs.md lines 70-74, covering the examples associated with the routing commands and requirement delta respectively; leave the example contents and closing fences unchanged.Source: Linters/SAST tools
internal/assets/assets.go (1)
399-403: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPass the template name into
splitMetaso a bad command template is identifiable.
renderCommandcallssplitMeta("command", raw). Every command template therefore fails with the same error text,template "command": ....renderAgentpassesf.Nameand names the failing file. Passfthrough for the same result here.♻️ Proposed change
-func renderCommand(h paths.Harness, raw string) (string, error) { - m, err := splitMeta("command", raw) +func renderCommand(h paths.Harness, f File, raw string) (string, error) { + m, err := splitMeta(f.Name, raw) if err != nil { return "", err }And at the call site in
Render:case Command: - return renderCommand(h, expanded) + return renderCommand(h, f, expanded)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/assets/assets.go` around lines 399 - 403, Update renderCommand to accept the command template identifier f and pass it to splitMeta instead of the constant "command"; update the corresponding Render call site to provide f, matching renderAgent so errors identify the failing template.internal/scaffold/update.go (1)
174-194: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Writescan reportfalsewhileApplyUpdatestill rewrites the manifest.The comment states that
Writesreports whether applying the plan would change anything "on disk or in the manifest". One case does not hold. If every file isUpCurrentbut the manifest records an olderVersion,Writes(false)returnsfalse, whileApplyUpdatestampsassets.Versionat Line 279 andmanifestUnchangedthen reports a difference.No data is lost either way. If the CLI skips
ApplyUpdatewhenWritesisfalse, the version stamp stays stale and each laterscc updaterepeats the same no-op. Either narrow the comment to on-disk changes, or makeWritesalso account for a stale recorded version.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/scaffold/update.go` around lines 174 - 194, Update UpdatePlan.Writes to return true when all file actions are UpCurrent but the manifest’s recorded version differs from the version ApplyUpdate will stamp, using the existing version fields or comparison logic. Preserve the current handling for creates, updates, deletes, orphans, and forced conflicts, and keep the comment aligned with the method’s manifest-change behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 9-13: Update the governed-workspace overview in CLAUDE.md at lines
9-13 to include plans/ alongside specs/ and docs/ in both the artifact list and
uncommitted-tree note. Update design/orchestration.md at lines 14-18 to include
plans/ in the scaffolded artifact list, or explicitly state that the list is
intentionally partial.
In `@design/orchestration.md`:
- Around line 414-418: Update the scaffold.Apply handling of the shared
AGENTS.md entry file so a second harness initialization refreshes its contents
using the harness-specific paths generated by assets.Render, rather than
skipping the existing file. Preserve the shared filename while ensuring each
harness can discover its own rules.
In `@internal/assets/templates/agents/code-review.md`:
- Line 86: Update the report-format opening fences in
internal/assets/templates/agents/code-review.md at lines 86-86 and
internal/assets/templates/agents/security-review.md at lines 90-90 to specify
the text language tag, leaving the fenced content unchanged.
In `@internal/cli/update.go`:
- Around line 112-121: Update the update command’s pre-confirmation flow around
the !*yes branch to reject --json when applying changes without --yes, before
calling interactive or confirm. Preserve --json --dry-run by applying this
validation only when the command would write changes, and return the command’s
existing error outcome for the invalid combination.
In `@internal/scaffold/update.go`:
- Around line 100-115: Validate every loaded Entry.Path as a safe local relative
path before the update planning loop can hash or delete it, rejecting absolute
paths and paths that escape root through traversal. Apply this validation in the
manifest-loading/update flow, including paths handled by UpDelete, and add a
regression test proving an escaping path is rejected without affecting files
outside the workspace.
In `@internal/workspace/workspace.go`:
- Around line 78-80: Update the documentation for workspace.Find to state that
it returns the absolute form of the start directory when no workspace marker is
found, matching the existing abs fallback behavior; keep the marker-search
behavior unchanged.
- Around line 117-133: The isFile check currently follows symlinks, allowing
symlinked workspace markers to be accepted. Update isFile to use os.Lstat and
require the marker path itself to be a regular file, then add a test covering a
symlinked marker and confirming Harnesses or IsWorkspace rejects it.
---
Nitpick comments:
In `@internal/assets/assets.go`:
- Around line 399-403: Update renderCommand to accept the command template
identifier f and pass it to splitMeta instead of the constant "command"; update
the corresponding Render call site to provide f, matching renderAgent so errors
identify the failing template.
In `@internal/assets/templates/rules/routing.md`:
- Around line 18-21: Add the text language identifier to the opening fenced code
blocks in internal/assets/templates/rules/routing.md lines 18-21 and
internal/assets/templates/rules/specs.md lines 70-74, covering the examples
associated with the routing commands and requirement delta respectively; leave
the example contents and closing fences unchanged.
In `@internal/scaffold/update.go`:
- Around line 174-194: Update UpdatePlan.Writes to return true when all file
actions are UpCurrent but the manifest’s recorded version differs from the
version ApplyUpdate will stamp, using the existing version fields or comparison
logic. Preserve the current handling for creates, updates, deletes, orphans, and
forced conflicts, and keep the comment aligned with the method’s manifest-change
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3724d4d5-37a7-4b35-863b-6abef6cad885
📒 Files selected for processing (51)
CLAUDE.mdREADME.mddesign/orchestration.mddesign/plan.mdinternal/assets/assets.gointernal/assets/assets_test.gointernal/assets/templates/agents/code-review.mdinternal/assets/templates/agents/security-review.mdinternal/assets/templates/claude/agents/code-review.mdinternal/assets/templates/claude/agents/security-review.mdinternal/assets/templates/commands/scc-adr.mdinternal/assets/templates/commands/scc-codewiki.mdinternal/assets/templates/commands/scc-glossary.mdinternal/assets/templates/commands/scc-prd.mdinternal/assets/templates/commands/scc-stack.mdinternal/assets/templates/commands/scc-wiki.mdinternal/assets/templates/entry.mdinternal/assets/templates/rules/autonomy.mdinternal/assets/templates/rules/delivery.mdinternal/assets/templates/rules/knowledge-base.mdinternal/assets/templates/rules/methodology.mdinternal/assets/templates/rules/project.mdinternal/assets/templates/rules/routing.mdinternal/assets/templates/rules/specs.mdinternal/assets/templates/rules/tasks.mdinternal/assets/templates/rules/verification.mdinternal/assets/templates/skills/adr/SKILL.mdinternal/assets/templates/skills/codewiki/SKILL.mdinternal/assets/templates/skills/glossary/SKILL.mdinternal/assets/templates/skills/prd/SKILL.mdinternal/assets/templates/skills/stack/SKILL.mdinternal/assets/templates/skills/wiki/SKILL.mdinternal/cli/cli.gointernal/cli/harness.gointernal/cli/harness_test.gointernal/cli/init.gointernal/cli/init_test.gointernal/cli/update.gointernal/cli/update_test.gointernal/cli/validate_test.gointernal/manifest/manifest.gointernal/manifest/manifest_test.gointernal/paths/paths.gointernal/scaffold/scaffold.gointernal/scaffold/scaffold_test.gointernal/scaffold/update.gointernal/scaffold/update_test.gointernal/validate/skill.gointernal/validate/skill_test.gointernal/workspace/workspace.gointernal/workspace/workspace_test.go
💤 Files with no reviewable changes (2)
- internal/assets/templates/claude/agents/security-review.md
- internal/assets/templates/claude/agents/code-review.md
Comment on lines
+414
to
+418
| **No entry file is shared across harness *layouts*.** Codex and opencode both read | ||
| `AGENTS.md`, so a repo initialized for both has one entry file, written by whichever | ||
| ran first and never overwritten — it points at that harness's rules, which are the | ||
| same bytes as the other's. That is a rough edge, not a bug: the alternative is | ||
| inventing a neutral rules directory neither tool knows, to save one duplicated tree. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 \
'entry\.md|EntryFile|AGENTS\.md|RulesSeg|rules/' \
internal/assets internal/paths internal/scaffoldRepository: protonspy/spec-claude-code
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- entry template references ---'
rg -n -C 6 'rules/|\.claude|\.codex|\.opencode|EntryFile|Render\\(|Render\(' \
internal/assets internal/scaffold design/orchestration.md \
-g '*.go' -g '*.md' | head -n 500
printf '%s\n' '--- relevant asset/rendering code ---'
ast-grep outline internal/assets/assets.go
sed -n '100,190p' internal/assets/assets.go
sed -n '1,120p' internal/assets/templates/entry.md
printf '%s\n' '--- initialization and shared-entry handling ---'
rg -n -C 10 'Skipped|Owned|Apply|Workspace\\(|Entry\\(|shared entry|SecondHarness|harness' \
internal/scaffold internal/assets internal/manifest internal/workspace \
-g '*.go' | head -n 800Repository: protonspy/spec-claude-code
Length of output: 321
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate entry templates ---'
fd -i 'entry.*|.*entry.*' internal/assets
printf '%s\n' '--- entry references and rendering calls ---'
rg -n -C 5 'rules/|\.claude|\.codex|\.opencode|EntryFile|Render\(' \
internal/assets internal/scaffold design/orchestration.md \
-g '*.go' -g '*.md' | head -n 700
printf '%s\n' '--- asset definitions and rendering ---'
sed -n '100,190p' internal/assets/assets.go
sed -n '1,180p' internal/assets/assets.go
printf '%s\n' '--- entry template content ---'
entry=$(fd -i -t f 'entry.*' internal/assets | head -n 1)
if [ -n "$entry" ]; then
cat -n "$entry"
fi
printf '%s\n' '--- initialization and shared-entry handling ---'
rg -n -C 10 'Skipped|Owned|Apply|Workspace\(|Entry\(|shared entry|SecondHarness|harness' \
internal/scaffold internal/assets internal/manifest internal/workspace \
-g '*.go' | head -n 900Repository: protonspy/spec-claude-code
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- entry template ---'
cat -n internal/assets/templates/entry.md
printf '%s\n' '--- layout and expansion ---'
sed -n '235,340p' internal/assets/assets.go
printf '%s\n' '--- scaffold write decisions ---'
sed -n '1,145p' internal/scaffold/scaffold.go
sed -n '145,235p' internal/scaffold/scaffold.go
printf '%s\n' '--- shared-entry test and assertions ---'
sed -n '390,450p' internal/scaffold/scaffold_test.go
sed -n '245,270p' internal/assets/assets_test.goRepository: protonspy/spec-claude-code
Length of output: 16919
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
template = Path("internal/assets/templates/entry.md").read_text()
layouts = {
"codex": {
"Entry": "AGENTS.md",
"Rules": ".codex/rules",
"Skills": ".codex/skills",
"Agents": ".codex/agents",
"Commands": "",
},
"opencode": {
"Entry": "AGENTS.md",
"Rules": ".opencode/rules",
"Skills": ".opencode/skills",
"Agents": ".opencode/agent",
"Commands": ".opencode/command",
},
}
def render(text, values):
for key, value in values.items():
text = text.replace("{{." + key + "}}", value)
text = re.sub(r"\{\{- if \.HasCommands\}\}(.*?)\{\{- end\}\}", "", text, flags=re.S)
return text
codex = render(template, layouts["codex"])
opencode = render(template, layouts["opencode"])
print("entry bytes equal:", codex == opencode)
for line in codex.splitlines():
if ".codex/" in line:
print("codex entry:", line)
for line in opencode.splitlines():
if ".opencode/" in line:
print("opencode entry:", line)
links = re.findall(r"\]\(([^)]+)\)", codex)
existing_after_both = {
".codex/rules/" + name
for name in (
"project.md", "routing.md", "autonomy.md", "methodology.md",
"tasks.md", "verification.md", "delivery.md", "specs.md",
"knowledge-base.md",
)
} | {
".opencode/rules/" + name
for name in (
"project.md", "routing.md", "autonomy.md", "methodology.md",
"tasks.md", "verification.md", "delivery.md", "specs.md",
"knowledge-base.md",
)
}
print("codex entry links missing after codex+opencode init:",
[link for link in links if link.startswith(".") and link not in existing_after_both])
PYRepository: protonspy/spec-claude-code
Length of output: 2888
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
template = Path("internal/assets/templates/entry.md").read_text()
layouts = {
"codex": {
"Entry": "AGENTS.md",
"Rules": ".codex/rules",
"Skills": ".codex/skills",
"Agents": ".codex/agents",
},
"opencode": {
"Entry": "AGENTS.md",
"Rules": ".opencode/rules",
"Skills": ".opencode/skills",
"Agents": ".opencode/agent",
},
}
def render(text, values):
for key, value in values.items():
text = text.replace("{{." + key + "}}", value)
return re.sub(
r"\{\{- if \.HasCommands\}\}.*?\{\{- end\}\}",
"",
text,
flags=re.S,
)
rendered = {name: render(template, layout) for name, layout in layouts.items()}
print("Codex and opencode entry bytes equal:", rendered["codex"] == rendered["opencode"])
for name, text in rendered.items():
links = re.findall(r"\]\(([^)]+)\)", text)
print(f"{name} rule links:", [link for link in links if "/rules/" in link])
rule_names = (
"project.md", "routing.md", "autonomy.md", "methodology.md",
"tasks.md", "verification.md", "delivery.md", "specs.md",
"knowledge-base.md",
)
for first, second in (("codex", "opencode"), ("opencode", "codex")):
available = {f".{first}/rules/{name}" for name in rule_names}
available |= {f".{second}/rules/{name}" for name in rule_names}
links = re.findall(r"\]\(([^)]+)\)", rendered[first])
missing = [link for link in links if link.startswith(".") and link not in available]
print(f"{first} first, {second} second; missing links:", missing)
PYRepository: protonspy/spec-claude-code
Length of output: 886
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
template = Path("internal/assets/templates/entry.md").read_text()
layouts = {
"codex": (".codex/rules", ".codex/skills", ".codex/agents"),
"opencode": (".opencode/rules", ".opencode/skills", ".opencode/agent"),
}
def render(rules, skills, agents):
text = template.replace("{{.Entry}}", "AGENTS.md")
text = text.replace("{{.Rules}}", rules)
text = text.replace("{{.Skills}}", skills)
text = text.replace("{{.Agents}}", agents)
return re.sub(r"\{\{- if \.HasCommands\}\}.*?\{\{- end\}\}", "", text, flags=re.S)
entries = {
name: render(*values)
for name, values in layouts.items()
}
for first, second in (("codex", "opencode"), ("opencode", "codex")):
links = [link for link in re.findall(r"\]\(([^)]+)\)", entries[first])
if "/rules/" in link]
expected_prefix = layouts[second][0] + "/"
wrong = [link for link in links if not link.startswith(expected_prefix)]
print(f"{first} initialized first, {second} second")
print(" shared AGENTS.md links:", links)
print(" links to second harness rules:", not wrong)
print(" wrong links:", wrong)
PYRepository: protonspy/spec-claude-code
Length of output: 1492
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
template = Path("internal/assets/templates/entry.md").read_text()
layouts = {
"codex": (".codex/rules", ".codex/skills", ".codex/agents"),
"opencode": (".opencode/rules", ".opencode/skills", ".opencode/agent"),
}
def render(rules, skills, agents):
text = template.replace("{{.Entry}}", "AGENTS.md")
text = text.replace("{{.Rules}}", rules)
text = text.replace("{{.Skills}}", skills)
text = text.replace("{{.Agents}}", agents)
return re.sub(r"\{\{- if \.HasCommands\}\}.*?\{\{- end\}\}", "", text, flags=re.S)
entries = {name: render(*values) for name, values in layouts.items()}
for first, second in (("codex", "opencode"), ("opencode", "codex")):
links = [
link for link in re.findall(r"\]\(([^)]+)\)", entries[first])
if "/rules/" in link
]
expected_prefix = layouts[second][0] + "/"
wrong = [link for link in links if not link.startswith(expected_prefix)]
print(f"{first} initialized first, {second} second")
print(" links to second harness rules:", not wrong)
print(" wrong links:", wrong)
PYRepository: protonspy/spec-claude-code
Length of output: 897
Fix the shared AGENTS.md contract. assets.Render generates harness-specific rule paths, but scaffold.Apply skips the existing entry file during the second initialization. The second harness therefore reads links to the first harness's rules, which makes its rules undiscoverable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@design/orchestration.md` around lines 414 - 418, Update the scaffold.Apply
handling of the shared AGENTS.md entry file so a second harness initialization
refreshes its contents using the harness-specific paths generated by
assets.Render, rather than skipping the existing file. Preserve the shared
filename while ensuring each harness can discover its own rules.
Comment on lines
+112
to
+121
| if !*yes { | ||
| if !interactive() { | ||
| render.Err("stdin is not a terminal: re-run with --yes to apply, or --dry-run to see the plan") | ||
| return ExitError | ||
| } | ||
| if !confirm(promptIn, "Apply these changes?") { | ||
| render.Info("nothing was written") | ||
| return ExitOK | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether render.Ask writes to stdout or stderr.
set -euo pipefail
rg -n -A5 'func Ask\(' internal/render/render.goRepository: protonspy/spec-claude-code
Length of output: 396
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- internal/cli/update.go ---'
sed -n '1,165p' internal/cli/update.go
printf '%s\n' '--- internal/render/render.go ---'
sed -n '1,70p' internal/render/render.go
printf '%s\n' '--- JSON flag/output wiring ---'
rg -n -A4 -B4 'addJSON|emitJSON|jsonOut|runUpdate' internal/cliRepository: protonspy/spec-claude-code
Length of output: 26708
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,165p' internal/cli/update.go
sed -n '1,70p' internal/render/render.go
rg -n -A4 -B4 'addJSON|emitJSON|jsonOut|runUpdate' internal/cliRepository: protonspy/spec-claude-code
Length of output: 26611
Require --yes for applying update --json.
When --json is set without --yes, confirm writes its prompt through render.Ask to stdout. A declined confirmation also writes render.Info("nothing was written") to stdout, so the stream is not valid JSON. Reject this combination before prompting, while allowing --json --dry-run.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/cli/update.go` around lines 112 - 121, Update the update command’s
pre-confirmation flow around the !*yes branch to reject --json when applying
changes without --yes, before calling interactive or confirm. Preserve --json
--dry-run by applying this validation only when the command would write changes,
and return the command’s existing error outcome for the invalid combination.
Source: Path instructions
A manifest arrives with a clone, and `scc update` joins each recorded path onto the root and then calls os.Remove on the ones this version no longer ships. An entry reading "../../.ssh/authorized_keys" therefore deletes outside the workspace — the exact class this project's SafeName rule exists for, reached through the one file scc reads back rather than through a CLI argument. Load refuses the whole file instead of skipping the bad entry: a manifest scc did not write is not a manifest, and silently dropping part of it would leave a workspace whose state nobody can account for. Two related hardenings from the same review: `update --json` without --yes is refused rather than prompting. confirm writes through render.Ask to stdout, so asking would emit a question into the document the caller is piping into jq; --dry-run and --yes both still work. The workspace marker is checked with Lstat. Its whole claim is "scc wrote this here", and a symlink pointing at somebody else's manifest satisfies Stat while making that claim false. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yCM51izVEvKywpKDgYvNM
…ENTS.md note CodeRabbit found both: the governed-artifact lists said specs/ and docs/ but not plans/, and §6's claim that the two AGENTS.md harnesses have byte-identical rules was not quite true — eight of nine are, and the ninth differs in a glossary example naming a manifest path. The conclusion is unchanged; the reason it holds is now stated accurately. The report-format fences in both review agents get a language tag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yCM51izVEvKywpKDgYvNM
protonspy
added a commit
that referenced
this pull request
Sep 17, 2026
…ook reaches (#52) * fix(attribution): the shapes a footer collapses to, and the half no hook reaches A session told not to sign its work does not stop signing it — it rephrases. Three escapes were measured walking past the validator, and the pull request comment was never read at all. A vendor product link is now a signature wherever it sits. The old gate asked whether the URL stood alone on its line, and a harness that kept the link and moved it into a sentence was read as prose. A product page is nobody's citation, so its position says nothing. Documentation keeps the old gate — code.claude.com and docs.anthropic.com are sources, and an argument that cannot cite the vendor's own documentation is one nobody can check. Crediting a tool in one's own words is the same claim as the footer: "thanks to", "with the help of", "powered by", "pair-programmed with", and the instrumental voice, "used Claude Code to draft the tests". The last needs the stricter test — the name has to be the verb's own object and not possessive — because the passive reading is ordinary prose about a product this repository supports. `scc validate --pr` now reads the comments and the review bodies. A commit message passes through commit-msg on the machine that wrote it; a comment is posted straight into the forge, by a tool as readily as by a person, and nothing ran when it was. The query degrades on its own, so a forge that answers about the pull request and not about its comments still gets the title and body checked. Two things the review found, both in code this change did not write but did newly expose to live traffic: `signed` accepted any unicode symbol, and `>` and `|` are Sm — so every blockquoted line and every table row opened a signature, and the rule reported "> Claude Code, Codex, and opencode", the very sentence its own paragraph promises never to report. Measured over this repository's 147 pull request bodies and comments: 56 hits before, of which two were false — a review bot's quoted file list and a table row from #6 — and 54 after, every one a real signature. The symbol now has to be non-ASCII, which is what a signature's decoration actually is. And a file is not a name: CLAUDE.md is a path the vocabulary matches on, so a line naming it read as somebody signing the work. `clip` also escapes now. It formats the offending line into a finding that finding.Set.Report prints to a real terminal on every push, and under --pr that line is written by anybody who can comment. A raw ESC surviving the trip is a control sequence the reader's emulator obeys — enough to scroll a finding off the screen and paint a clean report over it, which is the one lie this validator exists to prevent. Verified: go build, golangci-lint run, golangci-lint fmt --diff, and scc validate --checks --pr — 12 validators, 0 findings; 1001 tests at 86.2%. The corpus measurement above is the evidence for the false-positive claim. * fix(attribution): quoting a shape is mentioning it, not wearing it The package comment names this failure at the top of the file: a pattern that could match mid-sentence fires on a commit message that merely quotes one of these lines, which is exactly what a fix for this finding looks like. The other rules are anchored against it — a trailer at the start of a line, a badge or a link or a mention alone on one. The footer family had nothing, and the instrumental voice added in the previous commit had nothing either. Measured on this branch's own record. The commit explaining the widened rule quotes the phrasings it now catches, and the pull request body quotes them again, in a list. Both were reported by the rule they were explaining, and `scc validate --pr` came back exit 2 on the change that had just been pushed. So an occurrence behind an odd number of straight quotes or backticks, or inside a typographic quotation still open, is skipped — per occurrence rather than per line, so an unquoted credit beside a quotation is still caught. The apostrophe is deliberately not counted: it is a possessive far more often than it opens anything, and the instrumental rule already reads one three words along to tell a credit from a description. What this gives up is a footer written inside quotation marks, which is a shape nothing writes and a person would read as a mention anyway. What it buys is the one property this package trades everything for: one wrong finding teaches the reader to disbelieve the other ten. Verified against this repository's whole record — 149 files, every pull request body, issue comment and review comment, plus this branch's own commit and the body of #52: 54 hits, every one a real footer or session link from the pull requests that predate the rule, and nothing else.
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
1 participant
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.
Three changes, in three commits that each build on their own.
1 · The review agents run gates instead of reading
code-reviewnow runs five named gates in order, and runs all of them even afterone fails — bailing at the first red gate hands the orchestrator one problem when
there were four, and pays for a second review round to learn the rest.
[x]traced to code in the diff, and the reverseA gate it could not run is reported
not-run, never as a pass, and never inferredfrom the author saying it was green.
security-reviewis attack-class agnostic, as asked. The question is not "does thisdiff contain any of the ten bugs I know the names of" but "what can someone make this
code do that it was not built to do". Four passes — surface, trust boundaries,
reachability source-to-effect, deliberate attack — with the known classes (injection,
traversal, authz, secrets, crypto, exhaustion, TOCTOU, new deps, what got loosened)
demoted to prompts that jog the passes and explicitly not the definition of done.
Both end in a fixed report shape: verdict, a table of what was actually checked,
findings with severities that map to actions, and a separate place for the
unconfirmed suspicion so it is not smuggled in as a finding.
delivery.mdand§9 of the design tell the orchestrator how to branch on it.
Effort is pinned
highwherever the harness expresses one; the model tier onlywhere the harness has a stable alias (Claude Code's
sonnet). A pinnedgpt-5.6oranthropic/claude-…would be a guess about a name that churns and a provider theuser may not have configured.
2 · Three harnesses, one template set
scc init --claude | --codex | --opencode, defaulting to Claude Code, and a numberedpicker when a person is at the terminal and named no flag. Unattended callers still
get Claude Code silently — an agent or a CI job drives
initexactly as before.CLAUDE.mdAGENTS.mdAGENTS.md.claude/rules/.codex/rules/.opencode/rules/.claude/agents/*.md.codex/agents/*.toml.opencode/agent/*.md.claude/skills/.codex/skills/.opencode/skills/.claude/commands/.opencode/command/.claude/scc-manifest.json.codex/scc-manifest.json.opencode/scc-manifest.jsonThe template tree lost its per-harness directory rather than gaining two more: the
rules, skills, and reviewers are written once, paths come from a
paths.Harnessprofile, and the header each loader parses is synthesized at render time — YAML
frontmatter for two of them, a TOML agent role file for Codex. Three copies of the
prose would drift, and the drift would be silent.
Two consequences worth naming:
and now deprecated in favor of skills, so there is nothing project-scoped to write
and scc does not write into anyone's home directory.
harness and an upgrade that re-renders the recorded version to reconstruct a merge
base has to render it for the same one.
The data-free rule survives, restated: the harness profile is the only value a
workspace template may interpolate, and it is data scc records — so a
(version, harness)pair still renders byte-identically everywhere.Conventions verified against each tool's own source rather than its docs, which
disagree in two places: opencode's loader globs
{agent,agents}and{command,commands}under
.opencode/, and a Codex agent role file is flat TOML withdeveloper_instructions, discovered in<config>/agents/*.toml.3 ·
scc updateHashes every managed file against what this build renders and against what the
manifest says scc last wrote, which splits it six ways —
current,create,update,conflict,delete,orphan— plusownedfor the two files that existto be edited. Prints the plan grouped by outcome, asks, then applies.
An edited file is kept and named, never merged and never clobbered;
--forceis theseparate decision, and the kept file stays recorded at the version scc last wrote —
which is the base revision the three-way merge will need when it lands.
--yesisthe confirmation given up front and is required without a terminal,
--dry-runreports and stops,
--jsonemits one shape whatever the run decided.This is less than
design/plan.mdphase 11 promised, on purpose:internal/mergeisstill unbuilt, reporting a conflict is strictly safer than resolving one badly, and
this is the part users need first. The deviation is recorded in the design rather
than left for someone to discover.
Checks
gofmt,go vet, andgo test ./...are green, plus a manual run ofinit/update/validateagainst scaffolded Codex and opencode workspaces.-racedidnot run locally — no cgo toolchain on the machine — so CI is the first race run.
New tests worth knowing about: every harness's fresh workspace passes every
validator; no rendered file for one harness may name another's directory; the
reviewer prose is safe to embed in TOML; and the prompts are driven through an
injected stdin so they are testable rather than dependent on whether the runner
attached a console.
🤖 Generated with Claude Code
https://claude.ai/code/session_019yCM51izVEvKywpKDgYvNM
Summary by CodeRabbit
New Features
updatecommand with previews, dry runs, confirmations, JSON output, and conflict-safe updates.Documentation