Repository navigation
docs: post-PR4 doc-sync follow-ups (BACKLOG B11 + 3 SKILL/template refs) - #5
Merged
Merged
Conversation
The 2026-05-04 split of defect-scan into mechanical and semantic phases (via pipeline-full-with-deep-audit.yaml) covers B11's broad→deep use case from Thaumaturge, but with a different mechanism than originally proposed: phase split rather than within-phase pass-N append. Note the divergence and leave the original "smallest viable form" intact in case the within-phase append pattern is still wanted later. Found via doc-sync skill scan after PR #4 merged.
…line Post-merge follow-ups missed by PR #4. None changes behavior of the existing pipelines; all three are docstring/comment fixes that bring the SKILL/template surface area in line with the new default. - findings/porting/SKILL.md:8 — porting agents in the new default pipeline were instructed to read findings/defect-scan/defect-report.md (which doesn't exist in deep-audit runs). Now mentions both shapes: the legacy single report AND the mechanical+semantic pair. - templates/behavioral-contracts.md:121 — the comment listing which pipelines the default 6-criterion validation table matches now includes full-with-deep-audit (whose contracts phase has the same 6 criteria as the legacy ones). - findings/defect-scan/SKILL.md:62 — the action-set parenthetical ("Pre-porting pipelines") now lists both full-with-audit and full-with-deep-audit, since the new SKILLs point users back here for the action classification scheme. Found via doc-sync skill scan after PR #4 merged.
3 of 5 tasks
TheAmericanMaker
added a commit
that referenced
this pull request
Sep 28, 2026
…mtime, read HEAD through a worktree's gitdir The second review of #445 found three medium findings and one gap the pilot would hit on day one; each is reproduced by its probe before the fix and pinned by a test that kills the corresponding mutant. Re-read AGAIN before minting (review #4). The gate and the tree re-read ran once, before the request; nothing re-checked after the person answered, so a tree edited while the form was open still minted an approval bound to a candidate the tree no longer matched. requestAcceptance now runs args.reread again after elicitationDecision says accepted and checks it with checkCandidateFreshness; a mismatch or a read failure is `refused` with a reason starting `proof-stale:`, the stored request is kept, nothing is minted, and the marker is released. This is the contract's "before issuing AND AGAIN BEFORE MINTING". Unparseable in-flight marker (review #5). JSON.parse on an empty or truncated marker threw into the outer catch, which set stale=false, so the mtime fallback the comment promised was unreachable and the attempt was wedged forever. The parse is now its own try; an unparseable marker (or one without a finite expires_at) is judged by lstat().mtimeMs against the contract's maximum request TTL. Only a regular file is ever cleared: a directory or a symlink at the marker's path is refused by name with invalid-state and never removed (no rm -r). Worktree layout (review, recommended). A `.git` FILE (`gitdir: <path>`) failed closed with "no HEAD could be read". readWorkingTree now resolves the gitdir line (absolute or relative to the worktree root), reads HEAD from that git dir, and resolves a symbolic ref through the per-worktree dir first and then the `commondir` (relative to the gitdir) for shared loose refs and packed-refs. Anything malformed still refuses with the same reason; nothing outside what gitdir:/commondir name is followed; no git process is spawned (the module imports no child_process). Guards for the five mutants that slipped past (review #8): core-level calls with reread undefined and with a failing reread both stop at needs-human-acceptance with nothing presented; real-file tests pin the executable bit, symlinks recorded as links and never followed, the candidate's own exclusions, and unreadable files reported as uncovered. Refs #409.
TheAmericanMaker
added a commit
that referenced
this pull request
Sep 28, 2026
…on) (#445) * feat: MCP acceptance adapter — request_acceptance asks a person through elicitation Adds core/engineering/acceptance.ts and a `request_acceptance` action on codecarto_change. HostCapabilities are derived from the session (client capabilities + clientInfo at initialize) and host config, never from request fields; acceptanceChannelSupported decides whether the client may be asked (registry still empty, so every host returns needs-human-acceptance today); the decision comes only from the client's elicitation response via elicitationDecision; an approval is minted only for `accepted`, checked with evaluateApprovalReceipt against the stored request before persisting, and returned with classifyAcceptance's reading (cooperative on this host, D3). The SUPPORTED path is tested through an @internal buildServer option that only tests/helpers/acceptance-server.mjs passes; bin.mjs never does. Refs #409 * fix: run the gate on a fresh tree re-read before asking, and mint at most one approval per candidate requestAcceptance never ran the acceptance gate and never re-read the tree, so it asked a person about — and minted approvals for — candidates the gate refuses (no review at all) and bytes that had moved since capture. It also minted a second accepted approval for one candidate, sequentially or concurrently. Review of #445 (Refs #409). - core/engineering/acceptance.ts: before any request is built, the caller's `reread` of the working tree is passed to evaluateAcceptanceGate as candidate_reread; unless the gate says may-accept, no request is issued and nobody is asked (`blocked` with the blockers via describeGateOutcome; `needs-human-acceptance` when the tree cannot be re-read). An accepted approval already bound to this candidate's id+digest refuses with invalid-state/already-accepted. Request→present→mint is serialized per attempt by an O_EXCL marker `requests/.in-flight` carrying the request's expires_at; a marker past it is cleared. The store's change lock is not held across the elicitation wait. - mcp-server/working-tree.ts (new): re-reads cwd in the candidate's scope (same exclusions, `.git/` skipped, HEAD read from .git/HEAD; no git process). Both `gate` and `request_acceptance` supply it. - mcp-server/server.ts: buildServer() is the only public overload; the options overload is @internal, so the published .d.ts no longer names a stripped interface (TS2304 under skipLibCheck: false). - tests: gate-refused, edited-tree, gate-action-on-edited-tree, sequential and concurrent duplicate, stale marker, forged elicitation content (M6), and a consumer compile of dist with the repo's tsc and skipLibCheck off. acceptance moves to layer 5 in the layer map (it now imports gates.ts). - docs: registry entry shape after a live check; the assurance wording wart. * test: declaration check resolves dist on Windows (#445) The new "published declarations compile" test failed on test-windows with TS2307. It imported dist through pathToFileURL(...).pathname, which on Windows is "/D:/a/...", a path tsc cannot resolve. The consumer now lives in a scratch directory under node_modules/.cache and imports dist by a relative specifier. A relative path from the OS temp dir does not work either, because the runner's temp dir is on C: and the checkout is on D:. The scratch directory is removed afterwards. Checked that it still catches the bug it exists for: putting back the dangling BuildServerOptions reference in dist fails the test with TS2304. * fix: re-check freshness before minting, clear unparseable markers by mtime, read HEAD through a worktree's gitdir The second review of #445 found three medium findings and one gap the pilot would hit on day one; each is reproduced by its probe before the fix and pinned by a test that kills the corresponding mutant. Re-read AGAIN before minting (review #4). The gate and the tree re-read ran once, before the request; nothing re-checked after the person answered, so a tree edited while the form was open still minted an approval bound to a candidate the tree no longer matched. requestAcceptance now runs args.reread again after elicitationDecision says accepted and checks it with checkCandidateFreshness; a mismatch or a read failure is `refused` with a reason starting `proof-stale:`, the stored request is kept, nothing is minted, and the marker is released. This is the contract's "before issuing AND AGAIN BEFORE MINTING". Unparseable in-flight marker (review #5). JSON.parse on an empty or truncated marker threw into the outer catch, which set stale=false, so the mtime fallback the comment promised was unreachable and the attempt was wedged forever. The parse is now its own try; an unparseable marker (or one without a finite expires_at) is judged by lstat().mtimeMs against the contract's maximum request TTL. Only a regular file is ever cleared: a directory or a symlink at the marker's path is refused by name with invalid-state and never removed (no rm -r). Worktree layout (review, recommended). A `.git` FILE (`gitdir: <path>`) failed closed with "no HEAD could be read". readWorkingTree now resolves the gitdir line (absolute or relative to the worktree root), reads HEAD from that git dir, and resolves a symbolic ref through the per-worktree dir first and then the `commondir` (relative to the gitdir) for shared loose refs and packed-refs. Anything malformed still refuses with the same reason; nothing outside what gitdir:/commondir name is followed; no git process is spawned (the module imports no child_process). Guards for the five mutants that slipped past (review #8): core-level calls with reread undefined and with a failing reread both stop at needs-human-acceptance with nothing presented; real-file tests pin the executable bit, symlinks recorded as links and never followed, the candidate's own exclusions, and unreadable files reported as uncovered. Refs #409. * test: the worktree fixture is isolated from the runner's git config (#445) test (t) failed only on test-windows. After switching the worktree back to `feature` it expected FRESH and got STALE. The fixture passed GIT_CONFIG_GLOBAL=/dev/null and GIT_CONFIG_NOSYSTEM to the main-checkout calls only. The two checkouts inside the worktree ran under the runner's own config. On Windows that includes system core.autocrlf=true, so a.txt was rewritten with CRLF. The byte-exact re-read rightly called the tree STALE. /dev/null is not a path on Windows either. Reproduced on Linux with GIT_CONFIG_SYSTEM pointing at a file setting core.autocrlf=true: the original test fails at the same line (902) and the fixed test passes. The file now imports tests/helpers/git-environment-isolation.mjs first, as every other test that builds a git fixture does. It is added to that helper's guarded list (seven files, now eight), so the "imported first" check covers it. Every git call in the fixture uses one environment. This change is to the test only. readWorkingTree and its CRLF handling are unchanged, and byte-exactness is intended. npm test 1357/1357 twice. * test: pin the worktree HEAD reader's refusals (#445) The third review left four guard branches in mcp-server/working-tree.ts that no test covered. A mutant that removed any one of them still passed 33/33: - gitdir must be a real directory - the sha must be exactly 40 lowercase hex - '..', leading '/' and backslashes are refused in a ref - empty segments are refused in a ref Test (u) builds each malformed HEAD, ref or gitdir. Each refused case also plants a file the reader could resolve to a valid sha if its guard were missing, and each has a well-formed twin that is accepted. All four mutants now fail the test, as does a mutant that drops only the backslash check. The test is POSIX only (symlinks and backslash file names). npm test 1358/1358 twice. * test: replace, not overwrite, the worktree .git file in (t) (#445) On test-windows (runs for 14a8887 and 9438a73), the malformed-.git loop in (t) fails with EPERM when it opens <tmp>/wt/.git. Git for Windows marks a worktree's .git file hidden (core.hideDotFiles defaults to dotGitOnly), and overwriting a hidden file in place is EPERM on Windows. The loop now removes the file before writing it. The autocrlf fix from 14a8887 held: the failure moved past the FRESH assertion at line 902. Only the test changes; readWorkingTree is untouched.
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
2 participants
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
Doc-sync follow-ups from PR #4. Two commits, all docs/comments — no behavior change to any pipeline.
Commits
de48213backlog: mark B11 as partially blessed by deep-audit pipelined8e1afedocs: catch three SKILL/template references up to the deep-audit pipelineWhat changed
.codecarto/BACKLOG.md—B11. Defect-pass-N append disciplinewas an open backlog item asking for multi-pass support in defect-scan. PR #4 partially addressed it (phase split rather than the originally-proposed within-phase pass-N append). Updated theWhy deferredline to record the partial blessing while preserving the original "smallest viable form" so the within-phase append pattern can be revisited if a project still needs it..codecarto/findings/porting/SKILL.md:8— Functional fix. The porting agent in the new default pipeline was being instructed to readfindings/defect-scan/defect-report.md(which doesn't exist in deep-audit runs). Now mentions both report shapes:findings/defect-scan/defect-report.md(full-with-audit)findings/defect-scan-mechanical/mechanical-defects.md+findings/defect-scan-semantic/semantic-defects.md(full-with-deep-audit).codecarto/templates/behavioral-contracts.md:121— Validation-row comment listing which pipelines match the default 6-criterion table. Addedfull-with-deep-audit(the contracts phase in the new pipeline copies the same 6 criteria from the legacy one)..codecarto/findings/defect-scan/SKILL.md:62— Action-set parenthetical ("Pre-porting pipelines") now lists bothfull-with-auditandfull-with-deep-audit, since the new mechanical/semantic SKILLs point users back to this file for the action classification scheme.Test plan
full-with-auditreferences remain that should also mention the new variant — all remaining mentions are intentional naming of the legacy pipeline as a documented alternative.required_readschain still points at a non-existent report path./codecarto-init(default), proceed through architecture → mechanical → contracts → protocols → semantic, and confirm the porting phase's SKILL correctly directs the agent at bothmechanical-defects.mdandsemantic-defects.mdrather than the legacydefect-report.mdpath.Generated by Claude Code