Repository navigation
feat: implement scc through validate — scaffolding, artifacts, and eight validators - #1
Conversation
…ght validators
Lands the spec-driven development binary described in design/plan.md phases 1
through 10: a single stdlib-only Go binary with one surface, a headless CLI.
- init scaffolds .claude/rules/, the review agents, and the layout, recording
a content hash per managed file in .claude/scc-manifest.json — which
doubles as the workspace marker, so the upward walk stats a regular
file and never resolves to ~/.claude
- spec/plan create the two artifact vehicles; artifact templates take data and the
user owns the result, workspace templates stay data-free so a
three-way merge remains possible
- validate the eight validators over EARS grammar, requirement numbering and
traceability, plan one-source-of-truth, skill conformance, and the
wiki/ADR/glossary/stack/codewiki knowledge base
Exit codes are the contract: 0 ok, 1 usage or runtime error, 2 findings — so CI
and agents can branch on a lint result without conflating it with a failure of
the tool. Every command emits --json to stdout with diagnostics on stderr.
scc update (phase 11) is deliberately absent: a three-way merge is untestable
until a second template version exists, so the upgrade path is re-running init,
which is additive and never overwrites an edited file.
Ships with the CI gate (gofmt, vet, test -race on Linux/macOS/Windows,
golangci-lint, govulncheck), the manual release workflow, and the npm packages.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013XH8WYThtTSGJxS7tXRvfp
📝 WalkthroughWalkthroughThe PR adds the ChangesSCC platform
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SCCCLI
participant Workspace
participant Validators
User->>SCCCLI: Run scc validate
SCCCLI->>Workspace: Resolve root and check manifest
SCCCLI->>Validators: Run selected validators
Validators-->>SCCCLI: Return findings
SCCCLI-->>User: Print report and exit code
🚥 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: 20
🧹 Nitpick comments (4)
npm/scripts/build-packages.mjs (1)
135-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSort generated optional dependencies before serialization.
Serialize a lexically sorted map so generated package metadata remains stable independently of
TARGETSdeclaration order.Proposed fix
-rootPkg.optionalDependencies = optionalDependencies; +rootPkg.optionalDependencies = Object.fromEntries( + Object.entries(optionalDependencies).sort(([a], [b]) => + a < b ? -1 : a > b ? 1 : 0 + ) +);As per coding guidelines,
**/*.{yml,mjs,go}requires sorting before serialization.🤖 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 `@npm/scripts/build-packages.mjs` around lines 135 - 140, Update the package metadata generation around rootPkg.optionalDependencies in the build script to assign a lexically sorted dependency map before JSON serialization. Preserve the existing dependency values while ensuring output order is independent of TARGETS declaration order and satisfies the sorting guideline.Source: Coding guidelines
internal/validate/spec_test.go (1)
239-244: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe test checks less than its comment claims.
R1.3 (REMOVED)carries no requirement text, so it is precisely the input that would regress intoears.malformed, but only the orphan rule is asserted against.♻️ Proposed tightening
- for _, unwanted := range []string{"spec.orphan-requirement"} { + for _, unwanted := range []string{"spec.orphan-requirement", "ears.malformed"} { if contains(got, unwanted) { t.Errorf("rules = %v, want no %s: a REMOVED requirement needs no task", got, unwanted) } }🤖 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/validate/spec_test.go` around lines 239 - 244, Strengthen the assertions in the REMOVED requirement test around the unwanted rules list so it verifies that R1.3 does not produce either spec.orphan-requirement or ears.malformed. Keep the existing no-task expectation and ensure the test input still specifically exercises the REMOVED requirement with no requirement text.internal/mdscan/mdscan_test.go (1)
151-172: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider pinning the unterminated-fence behavior.
An unclosed fence makes every remaining line invisible to all eight validators, which is a silent whole-file pass. Worth one test so the behavior is deliberate rather than incidental.
♻️ Suggested test
// An unclosed fence hides the rest of the document from every validator. func TestUnterminatedFenceSwallowsTheRest(t *testing.T) { doc := parse(t, "```\n- [ ] inside\n\n# not seen\n") if len(doc.Checkboxes) != 0 || len(doc.Headings) != 0 { t.Errorf("content after an unclosed fence was scanned: %+v %+v", doc.Checkboxes, doc.Headings) } }🤖 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/mdscan/mdscan_test.go` around lines 151 - 172, Extend the fence behavior tests near TestTildeFences with a case for an unclosed backtick fence, such as TestUnterminatedFenceSwallowsTheRest. Parse content containing a fence, checkbox, and heading without a closing fence, then assert that both Checkboxes and Headings remain empty, preserving the intended behavior that the unterminated fence hides the remainder of the document.internal/validate/skill.go (1)
160-174: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
..in the middle of a target still escapes the skill directory.
isLocalReferenceonly rejects a..prefix, sorefs/../../other/x.mdreachesfilepath.Join(dir, …)and the existence probe lands outside the skill — a finding (or a silent pass) about a file that is not the skill's. Check the cleaned path instead.♻️ Suggested tightening
target = strings.SplitN(target, "#", 2)[0] if target == "" { continue } - if strings.Count(strings.Trim(path.Clean(target), "/"), "/") > 1 { + clean := path.Clean(target) + if clean == ".." || strings.HasPrefix(clean, "../") { + continue // escapes the skill; not scc's to judge + } + if strings.Count(strings.Trim(clean, "/"), "/") > 1 {🤖 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/validate/skill.go` around lines 160 - 174, Update the local-reference validation around isLocalReference and the target path handling to clean the target before resolving it, then reject any cleaned path that escapes the skill directory via parent traversal. Use the cleaned target consistently for the shallow-reference check and filepath.Join existence probe, while preserving fragment removal and empty-target handling.
🤖 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 @.github/workflows/ci.yml:
- Line 31: Update both actions/checkout steps in the CI workflow, including the
step pinned to v5.0.1 and the checkout step near line 72, to set
persist-credentials to false. Keep the existing checkout references and other
configuration unchanged.
In @.github/workflows/release.yml:
- Around line 36-39: Replace the shell glob validation in the release workflow
at .github/workflows/release.yml lines 36-39 with an anchored numeric regex that
accepts only the exact vX.Y.Z format; update the corresponding release-version
check in Makefile lines 64-66 to use the identical rule so CI and local release
targets behave consistently.
- Around line 112-122: Update the release archive and checksum steps around the
tar/zip commands and checksum generation to produce deterministic artifacts.
Derive and export SOURCE_DATE_EPOCH from the target commit, normalize archive
file timestamps and metadata, use zip metadata-free mode, and explicitly sort
checksum input/output. Normalize generated checksum content to LF before
serialization while preserving the existing archive naming and packaging
behavior.
- Around line 152-155: Update the “Upgrade npm” workflow step to install the
exact npm version 11.5.1 instead of the floating npm@11 major-line reference,
preserving the existing release toolchain setup.
In `@CLAUDE.md`:
- Around line 47-59: Update the architecture diagram in CLAUDE.md by changing
its fenced code block language identifier to text, preserving the diagram
content unchanged.
In `@internal/assets/templates/claude/rules/delivery.md`:
- Around line 11-13: Add the bash language specifier to the fenced code block
containing the git worktree add command, changing the opening fence to ```bash
while leaving the command and closing fence unchanged.
In `@internal/assets/templates/claude/rules/knowledge-base.md`:
- Around line 7-15: Label the directory-tree fenced code block in the
knowledge-base documentation with the text language identifier by changing the
opening fence to ```text, while preserving the tree contents and closing fence.
In `@internal/cli/cli.go`:
- Around line 111-114: Update the CLI help text near the shared-flag description
to scope --root to workspace commands and state that only commands supporting
--json emit machine-readable output on stdout. Ensure JSON-capable commands bind
the flag through addJSON and produce output via emitJSON, without implying these
flags apply to version or top-level help.
In `@internal/cli/validate_all.go`:
- Around line 43-50: Update the jsonOut branch in the validation command to pass
set.Document() directly to emitJSON, removing the anonymous wrapper and its
validators field. Preserve per-validator counts only in the human-readable
output and keep the frozen {findings, count} schema consistent with the other
validation commands.
In `@internal/render/render.go`:
- Line 14: Update the color-selection logic in the rendering functions,
including Warn and Err, to evaluate TTY support for the specific output stream
being written rather than the global useColor derived from os.Stdout. Ensure
stderr output disables ANSI color when redirected while preserving
stream-appropriate color for terminal destinations.
In `@internal/validate/codewiki.go`:
- Around line 91-125: Validate the cleaned citation path before resolving or
reading it in checkCitation: reject targets whose cleaned path is absolute or
begins with “..”, report them as unresolved, and return. Only allow paths
contained under root to proceed to isFile and os.ReadFile.
In `@internal/validate/skill.go`:
- Around line 98-101: Update the length checks in the validation logic for both
description and compatibility to count Unicode characters rather than UTF-8
bytes. Import and use unicode/utf8’s rune-counting helper in the checks that
emit skill.description-too-long and skill.compatibility-too-long, while leaving
the existing ASCII-only name validation unchanged.
In `@internal/validate/stack.go`:
- Around line 181-182: Update the majorVersionRe pattern used by stack
validation to recognize every valid Go major-version suffix from v2 onward,
including multi-digit versions such as v10 and v11, while continuing to reject
v0 and v1.
In `@internal/validate/wiki.go`:
- Around line 65-73: Normalize wikilink targets before slug resolution by
trimming whitespace and removing any “#anchor” suffix, so links such as
order-total#totals resolve against the page slug while preserving the original
target in diagnostics. Add a shared normalization helper and use it in both the
wikilink validation loop and checkChangelog, ensuring both paths apply identical
matching behavior.
- Around line 117-120: The changelog read failure path in the validation flow
silently returns without reporting the unreadable file or performing desync
checks. Update the logic around read(root, path) to distinguish a nil or errored
document from a successful read, emit the existing wiki.frontmatter-unreadable
finding for the changelog, and preserve normal validation for successfully read
documents.
In `@internal/workspace/workspace.go`:
- Around line 140-158: The no-clobber paths in SafeWrite and WriteFile must
prevent concurrent creators from replacing a file after the existence check.
Replace their stat-then-AtomicWrite behavior with an atomic create-if-absent
operation while preserving the existing return and refusal-error semantics, and
add a regression test that exercises concurrent creation attempts.
- Around line 54-75: Update the contract for AtomicWrite to avoid promising
atomic replacement across platforms, particularly on Windows where os.Rename may
expose intermediate states. Either document the guarantee as Unix-specific or
implement Windows-specific replacement handling while preserving the existing
temporary-file workflow.
In `@npm/scripts/build-packages.mjs`:
- Around line 21-28: Replace the cpSync import and its uses in the
build-packages script with fs.copyFileSync for these simple file copies,
preserving the existing copy destinations and behavior while keeping
compatibility with the declared Node >=16 engine.
- Around line 95-105: Replace the external "unzip" invocation in the Windows
branch of the package extraction flow with a Node-managed extractor or an
existing bundled repository helper, preserving extraction of the archive at
artifactsDir/base into binDir. Keep the non-Windows tar extraction and chmodSync
behavior unchanged.
In `@README.md`:
- Around line 67-70: Update the managed-artifact list in the README to include
plans/ alongside .claude/, specs/, and docs/. Keep the existing description
unchanged and ensure it reflects that scc plan creates and validates artifacts
there.
---
Nitpick comments:
In `@internal/mdscan/mdscan_test.go`:
- Around line 151-172: Extend the fence behavior tests near TestTildeFences with
a case for an unclosed backtick fence, such as
TestUnterminatedFenceSwallowsTheRest. Parse content containing a fence,
checkbox, and heading without a closing fence, then assert that both Checkboxes
and Headings remain empty, preserving the intended behavior that the
unterminated fence hides the remainder of the document.
In `@internal/validate/skill.go`:
- Around line 160-174: Update the local-reference validation around
isLocalReference and the target path handling to clean the target before
resolving it, then reject any cleaned path that escapes the skill directory via
parent traversal. Use the cleaned target consistently for the shallow-reference
check and filepath.Join existence probe, while preserving fragment removal and
empty-target handling.
In `@internal/validate/spec_test.go`:
- Around line 239-244: Strengthen the assertions in the REMOVED requirement test
around the unwanted rules list so it verifies that R1.3 does not produce either
spec.orphan-requirement or ears.malformed. Keep the existing no-task expectation
and ensure the test input still specifically exercises the REMOVED requirement
with no requirement text.
In `@npm/scripts/build-packages.mjs`:
- Around line 135-140: Update the package metadata generation around
rootPkg.optionalDependencies in the build script to assign a lexically sorted
dependency map before JSON serialization. Preserve the existing dependency
values while ensuring output order is independent of TARGETS declaration order
and satisfies the sorting guideline.
🪄 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: a69e7f7e-8c97-4c31-9ebd-7b918631f973
📒 Files selected for processing (84)
.gitattributes.github/workflows/ci.yml.github/workflows/release.yml.gitignore.golangci.ymlCLAUDE.mdMakefileREADME.mdcmd/scc/main.godesign/orchestration.mddesign/plan.mddesign/research.mdgo.modinternal/assets/assets.gointernal/assets/assets_test.gointernal/assets/templates/CLAUDE.mdinternal/assets/templates/artifacts/design.mdinternal/assets/templates/artifacts/plan.mdinternal/assets/templates/artifacts/requirements.mdinternal/assets/templates/artifacts/tasks.mdinternal/assets/templates/claude/agents/code-review.mdinternal/assets/templates/claude/agents/security-review.mdinternal/assets/templates/claude/rules/autonomy.mdinternal/assets/templates/claude/rules/delivery.mdinternal/assets/templates/claude/rules/knowledge-base.mdinternal/assets/templates/claude/rules/methodology.mdinternal/assets/templates/claude/rules/project.mdinternal/assets/templates/claude/rules/routing.mdinternal/assets/templates/claude/rules/specs.mdinternal/assets/templates/claude/rules/tasks.mdinternal/assets/templates/claude/rules/verification.mdinternal/cli/cli.gointernal/cli/cli_test.gointernal/cli/flags.gointernal/cli/init.gointernal/cli/init_test.gointernal/cli/jsonout.gointernal/cli/plan.gointernal/cli/plan_test.gointernal/cli/skill.gointernal/cli/spec.gointernal/cli/spec_test.gointernal/cli/validate.gointernal/cli/validate_all.gointernal/cli/validate_test.gointernal/ears/ears.gointernal/ears/ears_test.gointernal/finding/finding.gointernal/finding/finding_test.gointernal/manifest/manifest.gointernal/manifest/manifest_test.gointernal/mdscan/frontmatter.gointernal/mdscan/frontmatter_test.gointernal/mdscan/mdscan.gointernal/mdscan/mdscan_test.gointernal/paths/paths.gointernal/render/render.gointernal/scaffold/scaffold.gointernal/scaffold/scaffold_test.gointernal/textutil/textutil.gointernal/textutil/textutil_test.gointernal/validate/adr.gointernal/validate/all.gointernal/validate/codewiki.gointernal/validate/glossary.gointernal/validate/knowledge_test.gointernal/validate/plan.gointernal/validate/plan_test.gointernal/validate/skill.gointernal/validate/skill_test.gointernal/validate/spec.gointernal/validate/spec_test.gointernal/validate/stack.gointernal/validate/tasks.gointernal/validate/validate.gointernal/validate/wiki.gointernal/workspace/workspace.gointernal/workspace/workspace_test.gonpm/.gitignorenpm/README.mdnpm/scc/README.mdnpm/scc/bin/scc.jsnpm/scc/package.jsonnpm/scripts/build-packages.mjs
| os: [ubuntu-latest, windows-latest, macos-latest] | ||
| runs-on: ${{ matrix.os }} | ||
| steps: | ||
| - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
wc -l .github/workflows/ci.yml
cat -n .github/workflows/ci.yml | sed -n '1,220p'Repository: protonspy/spec-claude-code
Length of output: 4730
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
p = Path('.github/workflows/ci.yml')
text = p.read_text()
print("checkout occurrences:", text.count('actions/checkout'))
for m in re.finditer(r'actions/checkout@[^\s#]+(?:\s*#\s*([^\n]+))?', text):
start = text.rfind('\n', 0, m.start()) + 1
end = text.find('\n', m.start())
print(text[start:end])
PYRepository: protonspy/spec-claude-code
Length of output: 350
🌐 Web query:
actions/checkout v5 persist-credentials default token .git/config
💡 Result:
In actions/checkout v5, the persist-credentials input defaults to true [1][2]. When persist-credentials is set to true (the default), actions/checkout configures the GitHub Actions runner to authenticate Git commands by writing credential information to a file [3][4]. Crucially, in v5.0.1 and later versions of the v5 release, this behavior was updated: rather than writing credentials directly into the local .git/config file, the action stores them in a separate configuration file located within the $RUNNER_TEMP directory [4]. The main .git/config is then updated with an include.path directive to reference this temporary file [4]. This change was implemented to improve security by isolating the credentials from the repository's main configuration and preventing accidental leakage [4]. The credentials are removed during the post-job cleanup process [3]. If you wish to prevent this behavior, you can explicitly set persist-credentials: false in your workflow step [3].
Citations:
- 1: https://git.liteyuki.org/actions/checkout/src/tag/v5.0.1/action.yml
- 2: https://git.woni.link/actions/checkout/src/branch/releases/v5/action.yml
- 3: https://git.liteyuki.org/actions/checkout/src/tag/v5/README.md
- 4: https://git.cxtech.cc/actions/checkout/compare/v6.0.0..v5.0.1
Disable persisted checkout credentials on both checkout steps. The default checkout config leaves a repo-scoped credential available to later steps; set persist-credentials: false here and at line 72 so PR code can’t reuse it.
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 31-31: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/ci.yml at line 31, Update both actions/checkout steps in
the CI workflow, including the step pinned to v5.0.1 and the checkout step near
line 72, to set persist-credentials to false. Keep the existing checkout
references and other configuration unchanged.
Source: Linters/SAST tools
| case "$VERSION" in | ||
| v[0-9]*.[0-9]*.[0-9]*) echo "releasing $VERSION" ;; | ||
| *) echo "::error::version must look like vX.Y.Z (got '$VERSION')"; exit 1 ;; | ||
| esac |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use one strict release-version rule. Both checks accept malformed values such as v1.2.3-extra; npm packaging may then fail only after CI/build work has completed. Require the documented exact vX.Y.Z format with an anchored numeric regex.
.github/workflows/release.yml#L36-L39: replace the shell glob with an anchored numeric validation.Makefile#L64-L66: enforce the same anchored numeric validation for local release targets.
📍 Affects 2 files
.github/workflows/release.yml#L36-L39(this comment)Makefile#L64-L66
🤖 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 @.github/workflows/release.yml around lines 36 - 39, Replace the shell glob
validation in the release workflow at .github/workflows/release.yml lines 36-39
with an anchored numeric regex that accepts only the exact vX.Y.Z format; update
the corresponding release-version check in Makefile lines 64-66 to use the
identical rule so CI and local release targets behave consistently.
| mkdir -p dist | ||
| go build -trimpath \ | ||
| -ldflags "-s -w -X github.com/protonspy/spec-claude-code/internal/cli.version=${VERSION}" \ | ||
| -o "dist/${bin}" ./cmd/scc | ||
| name="scc_${VERSION}_${GOOS}_${GOARCH}" | ||
| cd dist | ||
| if [ "$GOOS" = "windows" ]; then | ||
| zip -q "${name}.zip" "${bin}" | ||
| else | ||
| tar -czf "${name}.tar.gz" "${bin}" | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Make release archives and checksums reproducible.
The tar/zip metadata captures build-time timestamps, so rebuilding the same commit/version changes release assets and checksums.txt. Derive SOURCE_DATE_EPOCH from the target commit, normalize file/archive metadata, use zip -X, and explicitly sort checksum output.
As per coding guidelines, “Maintain deterministic generated artifacts by normalizing newlines to LF and sorting before serialization.”
Also applies to: 212-213
🤖 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 @.github/workflows/release.yml around lines 112 - 122, Update the release
archive and checksum steps around the tar/zip commands and checksum generation
to produce deterministic artifacts. Derive and export SOURCE_DATE_EPOCH from the
target commit, normalize archive file timestamps and metadata, use zip
metadata-free mode, and explicitly sort checksum input/output. Normalize
generated checksum content to LF before serialization while preserving the
existing archive naming and packaging behavior.
Source: Coding guidelines
| # Trusted publishing requires npm >= 11.5.1 (newer than what ships with | ||
| # Node). Pin the major line so the release toolchain is reproducible. | ||
| - name: Upgrade npm | ||
| run: npm install -g npm@11 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
npm view npm@11.5.1 version engines dist.integrity --jsonRepository: protonspy/spec-claude-code
Length of output: 676
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '140,170p' .github/workflows/release.ymlRepository: protonspy/spec-claude-code
Length of output: 1347
Pin npm to 11.5.1. npm@11 will keep moving to newer 11.x patches, which breaks release reproducibility and can change the publishing toolchain without review.
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 155-155: ad-hoc installation of packages (adhoc-packages): installs a package outside of a lockfile
(adhoc-packages)
🤖 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 @.github/workflows/release.yml around lines 152 - 155, Update the “Upgrade
npm” workflow step to install the exact npm version 11.5.1 instead of the
floating npm@11 major-line reference, preserving the existing release toolchain
setup.
Source: Linters/SAST tools
| ``` | ||
| cmd/scc/main.go os.Exit(cli.Run(os.Args[1:])) | ||
| | | ||
| internal/cli the whole command surface | ||
| | | ||
| scaffold · validate write / check | ||
| | \ | ||
| assets · manifest ears · mdscan templates, hashes, grammars | ||
| | | ||
| paths · workspace · render · textutil · finding | ||
| | | ||
| plain files on disk: .claude/ · specs/ · plans/ · docs/ · CLAUDE.md | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language to the architecture fence.
Use ````text` for this diagram so Markdown linting does not report MD040.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 47-47: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@CLAUDE.md` around lines 47 - 59, Update the architecture diagram in CLAUDE.md
by changing its fenced code block language identifier to text, preserving the
diagram content unchanged.
Source: Linters/SAST tools
| func AtomicWrite(path string, data []byte, perm os.FileMode) error { | ||
| if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { | ||
| return err | ||
| } | ||
| tmp, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".tmp-*") | ||
| if err != nil { | ||
| return err | ||
| } | ||
| tmpName := tmp.Name() | ||
| defer func() { _ = os.Remove(tmpName) }() // no-op once the rename succeeds | ||
| if _, err := tmp.Write(data); err != nil { | ||
| _ = tmp.Close() | ||
| return err | ||
| } | ||
| if err := tmp.Chmod(perm); err != nil { | ||
| _ = tmp.Close() | ||
| return err | ||
| } | ||
| if err := tmp.Close(); err != nil { | ||
| return err | ||
| } | ||
| return os.Rename(tmpName, path) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== workspace.go ==\n'
sed -n '1,140p' internal/workspace/workspace.go
printf '\n== AtomicWrite usages ==\n'
rg -n "AtomicWrite\\(" -S .
printf '\n== workspace tests ==\n'
fd -a 'workspace*test.go' internal/workspaceRepository: protonspy/spec-claude-code
Length of output: 7746
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Go os.Rename docs excerpt ==\n'
go doc -all os.Rename | sed -n '1,120p'Repository: protonspy/spec-claude-code
Length of output: 708
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== workspace package outline ==\n'
ast-grep outline internal/workspace/workspace.go --view expanded
printf '\n== adjacent files ==\n'
fd -a . internal/workspaceRepository: protonspy/spec-claude-code
Length of output: 1022
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,140p' internal/workspace/workspace.goRepository: protonspy/spec-claude-code
Length of output: 5125
🌐 Web query:
Go os.Rename non-Unix platforms atomicity docs
💡 Result:
The Go standard library documentation for the os.Rename function explicitly states that it is not an atomic operation on non-Unix platforms, even when the operation occurs within the same directory [1][2][3][4]. On Unix-like systems, os.Rename generally provides atomic behavior, meaning that if the destination path already exists, it is replaced atomically, and other processes will always see either the old file or the new file [5][6]. However, because the underlying operating system behavior for renaming files varies significantly, the Go standard library does not guarantee atomic semantics across all platforms [6]. For developers who require atomic file replacement (e.g., to ensure that applications never see a partially written or empty file), the standard library's os.Rename is insufficient on non-Unix platforms [7]. Third-party solutions, such as the google/renameio package, are often used to handle the complexities of atomic file operations, though they may explicitly note that reliable atomic file writing is not possible on Windows due to platform limitations [7].
Citations:
- 1: https://pkg.go.dev/os?utm_source=godoc
- 2: https://pkg.go.dev/os@go1.26.4
- 3: https://github.com/golang/go/blob/go1.26.5/src/os/file.go
- 4: https://go.googlesource.com/go/+/refs/heads/master/src/os/file.go
- 5: os: make Rename atomic on Windows golang/go#8914
- 6: Windows os.Rename bad golang/go#3366
- 7: https://pkg.go.dev/github.com/google/renameio/v2
AtomicWrite overstates its cross-platform guarantee. os.Rename is not atomic on non-Unix platforms, so this helper shouldn’t promise that concurrent readers never observe an intermediate state on Windows; either narrow the contract or add Windows-specific replace handling.
🤖 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/workspace/workspace.go` around lines 54 - 75, Update the contract
for AtomicWrite to avoid promising atomic replacement across platforms,
particularly on Windows where os.Rename may expose intermediate states. Either
document the guarantee as Unix-specific or implement Windows-specific
replacement handling while preserving the existing temporary-file workflow.
Source: Coding guidelines
| func SafeWrite(path, content string) (bool, error) { | ||
| if _, err := os.Stat(path); err == nil { | ||
| return false, nil | ||
| } | ||
| if err := AtomicWrite(path, []byte(content), 0o644); err != nil { | ||
| return false, err | ||
| } | ||
| return true, nil | ||
| } | ||
|
|
||
| // WriteFile writes content unconditionally unless the path exists and overwrite | ||
| // is false. | ||
| func WriteFile(path, content string, overwrite bool) error { | ||
| if !overwrite { | ||
| if _, err := os.Stat(path); err == nil { | ||
| return fmt.Errorf("refusing to overwrite existing file: %s (pass --force)", path) | ||
| } | ||
| } | ||
| return AtomicWrite(path, []byte(content), 0o644) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files internal/workspace
printf '\n--- workspace.go (outline) ---\n'
ast-grep outline internal/workspace/workspace.go --view expanded || true
printf '\n--- workspace.go relevant lines ---\n'
sed -n '1,240p' internal/workspace/workspace.go | cat -n
printf '\n--- tests mentioning SafeWrite/WriteFile/AtomicWrite ---\n'
rg -n "SafeWrite|WriteFile|AtomicWrite|overwrite|no-clobber|concurrent|race" internal/workspace -g '*.go' -A 3 -B 3 || trueRepository: protonspy/spec-claude-code
Length of output: 15553
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "O_EXCL|CreateTemp|Create\(|SafeWrite|WriteFile|concurrent|race" internal/workspace -g '*.go' -A 2 -B 2 || trueRepository: protonspy/spec-claude-code
Length of output: 4029
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "\bSafeWrite\b|\bWriteFile\b" -g '*.go' .
printf '\n--- workspace_test.go excerpt ---\n'
sed -n '170,230p' internal/workspace/workspace_test.go | cat -nRepository: protonspy/spec-claude-code
Length of output: 6108
Make the no-clobber path atomic. SafeWrite and WriteFile(..., false) still do a Stat/Rename sequence, so a file created after the existence check can be replaced. Use an atomic create-if-absent path for the no-force branch and add a concurrent-creator regression test.
🤖 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/workspace/workspace.go` around lines 140 - 158, The no-clobber paths
in SafeWrite and WriteFile must prevent concurrent creators from replacing a
file after the existence check. Replace their stat-then-AtomicWrite behavior
with an atomic create-if-absent operation while preserving the existing return
and refusal-error semantics, and add a regression test that exercises concurrent
creation attempts.
Source: Coding guidelines
| chmodSync, | ||
| cpSync, | ||
| existsSync, | ||
| mkdirSync, | ||
| readFileSync, | ||
| rmSync, | ||
| writeFileSync, | ||
| } from "node:fs"; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
node -e 'const fs = require("node:fs"); if (typeof fs.cpSync !== "function") process.exit(1)'
rg -n -C2 'cpSync|copyFileSync|"node": ">=16"' npmRepository: protonspy/spec-claude-code
Length of output: 1108
🏁 Script executed:
#!/bin/bash
set -eu
sed -n '120,140p' npm/scripts/build-packages.mjsRepository: protonspy/spec-claude-code
Length of output: 1038
🌐 Web query:
Node.js fs.cpSync introduced version 16.7.0 14.18.0 official docs
💡 Result:
The fs.cpSync method was introduced in Node.js version 16.7.0 [1][2][3]. It provides a synchronous way to copy the entire directory structure from a source path to a destination path, including files and subdirectories [2][4]. Originally introduced as an experimental feature, the method was later marked as stable in Node.js 22.3.0 [5][1][2].
Citations:
- 1: Stabilize
fs.cp(),fs.cpSync(),fsPromises.cp()methods nodejs/node#44598 - 2: https://stackoverflow.com/questions/39106516/node-fs-copy-a-folder
- 3: https://stackoverflow.com/questions/13786160/copy-folder-recursively-in-node-js
- 4: https://nodejs.org/api/fs.html
- 5: https://nodejs.org/docs/latest-v22.x/api/fs.html
Avoid fs.cpSync with the current Node floor.
cpSync requires Node 16.7.0, but npm/scc/package.json still allows >=16, so Node 16.0–16.6 will fail to load npm/scripts/build-packages.mjs. These are simple file copies, so copyFileSync fits here unless the engine requirement is raised.
🤖 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 `@npm/scripts/build-packages.mjs` around lines 21 - 28, Replace the cpSync
import and its uses in the build-packages script with fs.copyFileSync for these
simple file copies, preserving the existing copy destinations and behavior while
keeping compatibility with the declared Node >=16 engine.
| const base = `${BIN}_${tag}_${t.goos}_${t.goarch}`; | ||
| if (t.goos === "windows") { | ||
| execFileSync("unzip", ["-o", join(artifactsDir, `${base}.zip`), "-d", binDir], { | ||
| stdio: "inherit", | ||
| }); | ||
| } else { | ||
| execFileSync("tar", ["-xzf", join(artifactsDir, `${base}.tar.gz`), "-C", binDir], { | ||
| stdio: "inherit", | ||
| }); | ||
| chmodSync(join(binDir, binName), 0o755); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
rg -n -C2 'build-packages|npm-build|execFileSync\("unzip"|windows' \
npm Makefile .github/workflowsRepository: protonspy/spec-claude-code
Length of output: 8745
🏁 Script executed:
#!/bin/bash
set -eu
printf '\n## npm/scripts/build-packages.mjs\n'
cat -n npm/scripts/build-packages.mjs | sed -n '1,180p'
printf '\n## npm/README.md\n'
cat -n npm/README.md | sed -n '1,120p'
printf '\n## .github/workflows/release.yml (relevant excerpt)\n'
cat -n .github/workflows/release.yml | sed -n '145,190p'Repository: protonspy/spec-claude-code
Length of output: 10826
🏁 Script executed:
#!/bin/bash
set -eu
printf '\n## package files mentioning unzip/zip\n'
rg -n 'unzip|zip' package.json npm package-lock.json .github/workflows MakefileRepository: protonspy/spec-claude-code
Length of output: 921
🏁 Script executed:
#!/bin/bash
set -eu
printf '\n## npm/scripts/build-packages.mjs\n'
cat -n npm/scripts/build-packages.mjs | sed -n '1,180p'
printf '\n## npm/README.md\n'
cat -n npm/README.md | sed -n '1,120p'
printf '\n## .github/workflows/release.yml (relevant excerpt)\n'
cat -n .github/workflows/release.yml | sed -n '145,190p'
printf '\n## package files mentioning unzip/zip\n'
rg -n 'unzip|zip' package.json npm package-lock.json .github/workflows MakefileRepository: protonspy/spec-claude-code
Length of output: 11583
🏁 Script executed:
#!/bin/bash
set -eu
printf '\n## workflow references to npm-build / build-packages\n'
rg -n -C2 'npm-build|build-packages\.mjs|make dist|make npm-build' .github/workflows Makefile npm
printf '\n## release workflow header and job runners\n'
cat -n .github/workflows/release.yml | sed -n '1,140p'
printf '\n## ci workflow header and job runners\n'
cat -n .github/workflows/ci.yml | sed -n '1,140p'Repository: protonspy/spec-claude-code
Length of output: 16359
🏁 Script executed:
#!/bin/bash
set -eu
printf '\n## npm workspace files\n'
git ls-files npm | sed -n '1,200p'
printf '\n## package manifests\n'
for f in $(git ls-files 'npm/**/package.json'); do
echo
echo "### $f"
cat -n "$f"
done
printf '\n## archive-related dependencies in npm manifests\n'
rg -n 'yauzl|adm-zip|unzip|tar|extract|zip' npm/**/package.jsonRepository: protonspy/spec-claude-code
Length of output: 1781
Avoid the undeclared unzip dependency on Windows npm/scripts/build-packages.mjs:95-100 will fail on Windows hosts that don’t already have unzip installed, and the repo doesn’t bundle or declare one. Use a Node-managed extractor or another bundled helper for the Windows archive path.
🤖 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 `@npm/scripts/build-packages.mjs` around lines 95 - 105, Replace the external
"unzip" invocation in the Windows branch of the package extraction flow with a
Node-managed extractor or an existing bundled repository helper, preserving
extraction of the archive at artifactsDir/base into binDir. Keep the non-Windows
tar extraction and chmodSync behavior unchanged.
Source: Coding guidelines
| The artifacts it governs are plain Markdown and JSON in your repo — `.claude/`, | ||
| `specs/`, `docs/`. No server, no database, no conversion layer: the files *are* the | ||
| API, and they are the ones Claude Code already reads. `scc` adds no directory of its | ||
| own and no config file — it owns a single manifest inside `.claude/`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Include plans/ in the managed-artifact list.
This list omits plans/, although scc plan creates and validates plans. Add it so the storage contract matches the command surface.
🤖 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 `@README.md` around lines 67 - 70, Update the managed-artifact list in the
README to include plans/ alongside .claude/, specs/, and docs/. Keep the
existing description unchanged and ensure it reflects that scc plan creates and
validates artifacts there.
Lands the spec-driven development binary described in
design/plan.mdphases 1–10.sccis a single stdlib-only Go binary with one surface — a headless CLI — so an agent or a CI job drives it exactly as well as a human does.What works
init.claude/rules/, the review agents, and the layout. Idempotent; never overwrites your edits.spec new|list|show|delete|validateplan new|list|delete|validateskill validatevalidateThe eight validators cover EARS grammar across all five patterns, requirement numbering, one methodology annotation per task, traceability in both directions, plan one-source-of-truth, skill conformance, the wiki link/orphan graph, ADR numbering and superseding, glossary vocabulary drift, dependencies missing from
docs/stack.md, and codewiki citations that no longer resolve.Source code is deliberately not checked — scc never parses it, so it cannot tell you the code honors what the artifact says. That stays the orchestrator's accountability; a checker that was confidently incomplete would be worse than none.
Decisions worth reviewing
.claude/scc-manifest.json, never the.claude/directory.~/.claudeis Claude Code's own global config directory and exists on every machine that runs it — an upward walk accepting the directory would resolve the root to$HOMEfor any command run outside a workspace, and every command would then read and write the user's global config..claude/also exists in every repo that merely uses Claude Code.workspace.Findtherefore stats a regular file.0ok,1usage/runtime error,2findings. A finding is a legitimate answer to a lint question, not a failure of the tool, so2is never collapsed into1..claude/rules/, which is Markdown the orchestrator already reads.workspace.SafeNamebeforefilepath.Join— without itscc spec delete .. --forceresolves to the workspace root.Not in scope, deliberately
scc update(phase 11) is absent. A three-way merge is untestable until a second template version exists, so until then the upgrade story is re-runninginit, which is additive and never overwrites a file you edited.Verification
make checkis green —gofmt -lclean,go vet,go test -raceacross all packages. Coverage where it matters:mdscan97.9%,finding95.2%,ears87.0%,manifest87.3%,validate85.0%,cli79.8%.TestFreshArtifactsPassTheirOwnValidatorsininternal/cliis the gate that matters most here: a validator that fires on scc's own scaffolded output would be the worst bug in the product, since one wrong finding teaches the user to disbelieve all eight.golangci-lintandgovulncheckwere not run locally (not installed) — CI runs both, along with the suite on Linux/macOS/Windows.🤖 Generated with Claude Code
https://claude.ai/code/session_013XH8WYThtTSGJxS7tXRvfp
Summary by CodeRabbit
New Features
scccommand-line tool with help, version, JSON output, and stable exit codes.Documentation
Chores