Repository navigation
fix(launch): a marker in prose is not a block, and the dev container comes up - #49
Merged
Merged
Conversation
A run of `scc launch claude` on Windows fell back to the host unsandboxed and warned about a malformed block in this repo's own entry file. Both causes ship to every scaffolded workspace and both fail in the direction that looks like success.
Splice, Block, Version and Remove each found the markers with strings.Index over the document itself, so a file that *documents* the markers was read as a file carrying the block. This repository's own CLAUDE.md is the measured case: it names RTK's opening and closing markers in one sentence, which made rtkBlockPresent true and scc launch skip the RTK wiring it believed was already done — and left scc rtk ready to splice over that sentence — and it names scc's CodeGraph opener alone, which was reported malformed so the block was never written at all. mdscan.Mask blanks fenced blocks and inline code spans to spaces, preserving every byte offset and leaving HTML comments intact, because the markers a caller looks for here are HTML comments. One span helper in mdblock measures over it and slices out of the original, so four methods cannot disagree about where the block is. rtk.ForeignBlock had the same defect for Headroom's marker and takes the same mask. Mask covers fenced blocks and code spans only; a marker in a 4-space-indented block or a blockquote still reads as live. Recorded as n-0010.
…d-only bind postCreateCommand ran `git config --global --add safe.directory`, and ~/.gitconfig is the read-only bind that carries your identity in, so git failed with "Device or resource busy". That failed postCreate, which failed `devcontainer up`, which dropped scc launch back onto the host — a run that reported a sandbox and started the agent outside one. The setting moves into the image, where nothing is read-only and the build runs as root: postCreate's non-root user cannot write /etc/gitconfig either, so neither scope works from there. It is scoped to '/workspaces/*' rather than '*' — git's ownership check is what stops a differently-owned .git from running its own config or hooks (CVE-2022-24765), and this container forwards GH_TOKEN and mounts the agent's credential volume.
… tests All three were at 0%, and all three are the kind of code that fails silently: a segment constant that grows a leading separator turns a workspace-relative path absolute, a status line on the wrong stream corrupts every --json document, and an argument vector that puts the agent's flags ahead of the agent hands them to the CLI instead. The integration tests drive install, lookup and daemon detection against a stand-in binary on a replaced PATH, so no test shells out to the real npm or installs a global package on whoever runs the suite.
…parsers The package was at 8.3%: the diff parser had tests and everything that actually shells out to git had none, which is most of it and all of the reasoning the comments spend their length on. The distinctions now have a test each — merged is ahead 0 and behind > 0 rather than "is an ancestor", a detached head answers "" rather than "HEAD", core.hooksPath wins over .git/hooks, core.commentChar's "auto" comes back verbatim, and Changed reads the working tree while skipping binary and ignored files. Each repository is built in a temp directory with GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM pointed at an empty file, so a developer's own gpgsign, hooksPath or diff.noprefix cannot change what the suite measures — the same class of interference Changed's pinned prefixes exist for. gh is a stand-in binary on a replaced PATH; no test needs a network or an account.
harness.go was at 0%, and it is the half of this package that writes into a file scc does not own. The promises now have a test each: an install preserves every key, every event and every entry it did not write; a remove leaves somebody else's hook and drops the keys it emptied rather than littering empty arrays; a file scc cannot parse is reported and left byte for byte, because reformatting somebody's broken JSON destroys what they were in the middle of fixing. Recognition by the exact command is pinned from both sides — `my-scc hooks run stop` and `scc hooks run stop | tee log` are not scc's, and both were real near-misses for the substring test this replaced. On the git side, Look is covered for the one thing a report must never do, which is write what it was asked to report on.
… the boundary 76.8% to 88.8%, and the parts that had nothing were the ones whose whole value is a distinction: the delivery vocabulary is closed and terminal states are what separate "still open" from "done with"; the seal is tamper-evidence, so an edited approved plan has to report drift and a resealed one has to stop; the schedule is one implementation behind --next, --ready and --blocked, so 1.9 has to sort before 1.10 and Next has to agree with Ready. Within is covered for what it exists for — `..` out of the workspace, and a sibling whose name merely starts with the root's, which a prefix test gets wrong.
…placed PATH Five packages shell out to a third party, and in each one the half that looks up, versions and installs the binary had little or no coverage — rtk was at 32%, because everything tested was the marker splice and nothing was the cargo build. Each now drives those paths against a script the test writes and puts on PATH by itself, so no test installs a global npm package, resolves a Python distribution, or starts a Rust build on whoever runs the suite. The distinctions that get tested are the ones these packages exist to make: a missing cargo is reported as a missing toolchain rather than as a failed build, uv is chosen over pip on a machine with both, and a build whose --help cannot answer means scc passes no options rather than guessing at them. gate's Load and Save are covered for the reason they read and write every manifest: taking the first that has any makes a single write depend on which tool the reader was standing in.
…s in checks.go was at 0%: the eleventh validator's whole design is that its findings are distinct rather than one "the gate failed" — a command that did not run, a report that could not be read, a suite with no tests, and one that came up short are four different things to go and do. Each now has a test, including that an undecided gate says what to write and a declined one says nothing. All() is covered for the ordering it assembles by, which is cost: the two options are behind a flag because they run a compiler and go to the network, checks runs before pr so a broken build reports before a round trip to the forge, and WithoutAttribution drops exactly the one check the Stop hook cannot carry.
Two gaps, and the first is the contract itself: `--help` is 0 on every subcommand, and nothing checked it across the surface — the defect it guards against was 52 handlers routing flag.ErrHelp to ExitError at once. The sweep runs every command's help, every resource's bare `help` positional, and every unknown subcommand, which has to name what was typed because the caller is usually an agent with no manual. `scc check` with no subcommand is deliberately absent from those lists: it runs the gates, which in this repository is the suite running the test. The map reads now have a spec to read from, so trace follows a requirement out of requirements.md and into the design, and every block `map blocks` prints is checked to be an address `map show` actually accepts — an index of addresses that do not resolve is worse than no index.
…es find adds over grep patch's data-loss case is pinned from both ends: changing a task's methodology has to re-emit its flags and its continuation, and an address that does not resolve has to leave the file untouched rather than insert at a guess. notes covers the questions a substring cannot answer — which tags exist, what this project knows about a path, what is new — plus the tombstone, which is what makes a spent number spent rather than reusable.
Four small gaps, each on something whose failure is silent. AtomicWrite is covered for what it promises to leave behind, which is nothing — no temp file, the whole content, the mode asked for. CarryOver is covered for why it exists: `scc init` and `scc update` rebuild the manifest from scratch, so a re-run was throwing away the delivery gate's commands and every key a newer scc wrote. mdblock's Remove now has the tests Splice had, including that a marker written in prose is not a block on that path either, and that CRLF survives both directions. mdscan.Ticked is exported precisely so there is one answer to what a ticked box looks like, and it had none.
A scope is one graph per directory rather than one graph filtered, so --json over several roots is an array of sections — each carrying its root's own document rather than a re-encoding of it, and degrading to a string when a root printed something that is not JSON. rtkInstallOK is one decision for init and launch both, precisely so there is not an untested second copy; it now has a test per way it can refuse.
The v2 plan contract is six sections and no others, and nothing tested that a seventh is a finding — which is the whole mechanism that caps a plan's size. The seal's value is where it is checked: drift is not a validator finding, it is refused before an edit is applied, because a harness that edited by hand and then patched would otherwise have its edit resealed by the command that should have reported it. Both halves are pinned, plus that reseal --force leaves the status alone.
…mary AttributionPR never returns an error, and the reasons are the ones nobody would write a test for: no repository, no gh, no pull request. Each is now pinned, because the failure mode is scc breaking validate in every workspace that is not a git repository over a rule about git. UpdatePlan.Writes had one subtlety and no test: an orphan writes without force, since the file is left alone but its manifest entry goes — and a stale entry sends the next update looking for a template that no longer exists.
…ry on a sealed plan Every map read now has a test for the human form and the machine one, including that the JSON is compact — the reader of that document is an agent paying by the byte, and a third of the indented form was leading whitespace. The container backend had no tests at all: the two reasons a Windows run ends up on the host, and that it says so as the weaker boundary in the document as well as in the line a person reads. Discovery on an approved plan is pinned from both sides, including the case the rollback catches — a ticked task cannot be struck out, because it was either done or it was not.
…it reports stale is the reason the Stop stage no longer re-indexes on every turn, and its asymmetry had no test: it errs toward syncing, because an unnecessary index costs a subprocess while a skipped one leaves the agent an index that answers confidently about code that changed. touchedHere falls back to the whole set when git cannot answer, which is the direction that keeps the stage useful rather than silent, and relAll names the manifest from the workspace root — against the working directory the same file came back under two names.
Both steps exist to end in the agent starting anyway, and the reason is the whole value — silence is how somebody spends a month wondering why the proxy never seemed to help. Driven against the resolvers directly, because --json implies install nothing and the install-shaped branches are unreachable from a plan-only run.
…chors CompareNumbers is a correction rather than a preference — sorted as text 1.10 comes before 1.9, so a loop asking for the next task got the tenth before the ninth the moment a group grew past nine items — and it had no test of its own. Search is pinned on the property that makes it usable: every hit comes back as an address Find resolves, an empty query is no hits rather than the whole corpus, and a malformed regex is an error rather than silence that reads as no matches. Codewiki's slug rules each have a case now, including the heading that yields no anchor at all, and a citation to the last line of a file with a trailing newline resolves rather than reading as one past the end.
… hook gate isLocalReference is the boundary of what the skills validator can check, and everything outside it — an anchor, a URL, a path that escapes the skill — is somebody else's to resolve; reporting one would be scc claiming to know about the world outside the directory it is looking at. The hook gate narrows outside a workspace, which is what lets the hooks be installed in a repository that follows scc's rules without being one of its workspaces — scc's own, for one. Without it every such repository would get findings about a specs/ tree that does not exist.
map find's flags are the whole of what it offers over a grep, and each now has a case — including that a malformed regex is an error rather than zero hits, which would read as nothing matching. The notes rollback had none: a refused first note has to take the log it just seeded with it, or the command reports the note as not written and still changes the workspace.
A spec name becomes a path segment, so it goes through SafeName first — without it walks out of the workspace — and a bare sync reconciles every spec that recorded something rather than every directory under specs/. --dry-run is pinned for the reason it exists: sync writes to every spec in the workspace, and a flag that edited them anyway would be the one nobody expects to change anything doing exactly that.
… word
A scope that matches nothing stops the run rather than falling back to the whole
workspace — indexing everything because a path was misspelled is an expensive way
to find out about a typo — and one tree missing out of two is named and skipped.
touchedHere is pinned against a real branch now, not only on its fallback: a
finding in a file this branch never opened is what the filter exists to drop.
helpWord rewrites a lone GNU bash, version 5.3.9(1)-release (x86_64-pc-cygwin)
These shell commands are defined internally. Type `help' to see this list.
Type `help name' to find out more about the function `name'.
Use `info bash' to find out more about the shell in general.
Use `man -k' or `info' to find out more about commands not in this list.
A star (*) next to a name means that the command is disabled.
! PIPELINE history [-c] [-d offset] [n] or hist>
job_spec [&] if COMMANDS; then COMMANDS; [ elif C>
(( expression )) jobs [-lnprs] [jobspec ...] or jobs >
. [-p path] filename [arguments] kill [-s sigspec | -n signum | -sigs>
: let arg [arg ...]
[ arg... ] local [option] name[=value] ...
[[ expression ]] logout [n]
alias [-p] [name[=value] ... ] mapfile [-d delim] [-n count] [-O or>
bg [job_spec ...] popd [-n] [+N | -N]
bind [-lpsvPSVX] [-m keymap] [-f file> printf [-v var] format [arguments]
break [n] pushd [-n] [+N | -N | dir]
builtin [shell-builtin [arg ...]] pwd [-LPW]
caller [expr] read [-Eers] [-a array] [-d delim] [>
case WORD in [PATTERN [| PATTERN]...)> readarray [-d delim] [-n count] [-O >
cd [-L|[-P [-e]]] [-@] [dir] readonly [-aAf] [name[=value] ...] o>
command [-pVv] command [arg ...] return [n]
compgen [-V varname] [-abcdefgjksuv] > select NAME [in WORDS ... ;] do COMM>
complete [-abcdefgjksuv] [-pr] [-DEI]> set [-abefhkmnptuvxBCEHPT] [-o optio>
compopt [-o|+o option] [-DEI] [name .> shift [n]
continue [n] shopt [-pqsu] [-o] [optname ...]
coproc [NAME] command [redirections] source [-p path] filename [argument>
declare [-aAfFgiIlnrtux] [name[=value> suspend [-f]
dirs [-clpv] [+N] [-N] test [expr]
disown [-h] [-ar] [jobspec ... | pid > time [-p] pipeline
echo [-neE] [arg ...] times
enable [-a] [-dnps] [-f filename] [na> trap [-Plp] [[action] signal_spec ..>
eval [arg ...] true
exec [-cl] [-a name] [command [argume> type [-afptP] name [name ...]
exit [n] typeset [-aAfFgiIlnrtux] name[=value>
export [-fn] [name[=value] ...] or ex> ulimit [-SHabcdefiklmnpqrstuvxPRT] [>
false umask [-p] [-S] [mode]
fc [-e ename] [-lnr] [first] [last] o> unalias [-a] name [name ...]
fg [job_spec] unset [-f] [-v] [-n] [name ...]
for NAME [in WORDS ... ] ; do COMMAND> until COMMANDS; do COMMANDS-2; done
for (( exp1; exp2; exp3 )); do COMMAN> variables - Names and meanings of so>
function name { COMMANDS ; } or name > wait [-fn] [-p var] [id ...]
getopts optstring name [arg ...] while COMMANDS; do COMMANDS-2; done
hash [-lr] [-p pathname] [-dt] [name > { COMMANDS ; }
help [-dms] [pattern ...] and nothing else, because GNU bash, version 5.3.9(1)-release (x86_64-pc-cygwin)
These shell commands are defined internally. Type `help' to see this list.
Type `help name' to find out more about the function `name'.
Use `info bash' to find out more about the shell in general.
Use `man -k' or `info' to find out more about commands not in this list.
A star (*) next to a name means that the command is disabled.
! PIPELINE history [-c] [-d offset] [n] or hist>
job_spec [&] if COMMANDS; then COMMANDS; [ elif C>
(( expression )) jobs [-lnprs] [jobspec ...] or jobs >
. [-p path] filename [arguments] kill [-s sigspec | -n signum | -sigs>
: let arg [arg ...]
[ arg... ] local [option] name[=value] ...
[[ expression ]] logout [n]
alias [-p] [name[=value] ... ] mapfile [-d delim] [-n count] [-O or>
bg [job_spec ...] popd [-n] [+N | -N]
bind [-lpsvPSVX] [-m keymap] [-f file> printf [-v var] format [arguments]
break [n] pushd [-n] [+N | -N | dir]
builtin [shell-builtin [arg ...]] pwd [-LPW]
caller [expr] read [-Eers] [-a array] [-d delim] [>
case WORD in [PATTERN [| PATTERN]...)> readarray [-d delim] [-n count] [-O >
cd [-L|[-P [-e]]] [-@] [dir] readonly [-aAf] [name[=value] ...] o>
command [-pVv] command [arg ...] return [n]
compgen [-V varname] [-abcdefgjksuv] > select NAME [in WORDS ... ;] do COMM>
complete [-abcdefgjksuv] [-pr] [-DEI]> set [-abefhkmnptuvxBCEHPT] [-o optio>
compopt [-o|+o option] [-DEI] [name .> shift [n]
continue [n] shopt [-pqsu] [-o] [optname ...]
coproc [NAME] command [redirections] source [-p path] filename [argument>
declare [-aAfFgiIlnrtux] [name[=value> suspend [-f]
dirs [-clpv] [+N] [-N] test [expr]
disown [-h] [-ar] [jobspec ... | pid > time [-p] pipeline
echo [-neE] [arg ...] times
enable [-a] [-dnps] [-f filename] [na> trap [-Plp] [[action] signal_spec ..>
eval [arg ...] true
exec [-cl] [-a name] [command [argume> type [-afptP] name [name ...]
exit [n] typeset [-aAfFgiIlnrtux] name[=value>
export [-fn] [name[=value] ...] or ex> ulimit [-SHabcdefiklmnpqrstuvxPRT] [>
false umask [-p] [-S] [mode]
fc [-e ename] [-lnr] [first] [last] o> unalias [-a] name [name ...]
fg [job_spec] unset [-f] [-v] [-n] [name ...]
for NAME [in WORDS ... ] ; do COMMAND> until COMMANDS; do COMMANDS-2; done
for (( exp1; exp2; exp3 )); do COMMAN> variables - Names and meanings of so>
function name { COMMANDS ; } or name > wait [-fn] [-p var] [id ...]
getopts optstring name [arg ...] while COMMANDS; do COMMANDS-2; done
hash [-lr] [-p pathname] [-dt] [name > { COMMANDS ; }
help [-dms] [pattern ...] is a perfectly
good plan name, note id or search term.
…e hook report The three acts on a gate are distinct and had no test between them: set records, skip declines — a word rather than an absent key — and clear puts it back to undecided, which the validator reports where a declined gate is silence. scc graph is a hard error without its binary, unlike the launch path, because the whole command is the binary; and update reports a stale hook block without rewriting it, since a yes about template files is not consent to edit .git. This brings the suite to the 86% floor the delivery gate requires.
An early version of the usage sweep ran `scc plan new help` against the real workspace rather than a temp one. The sweep no longer does; the file it left is not an artifact of this work.
…th path The lifecycle test built a repository without turning commit signing off, so on a machine whose global config signs commits every commit in it would fail non-interactively — an environment the test does not mean to assert about. The plan's Paths section named three of the four files this work touched.
Within evaluated symlinks on the root and on the target, but a target that is not on disk yet — the ordinary case for a file about to be written — fails EvalSymlinks and stayed lexical while the root beside it was resolved. The two were then no longer comparable, and a path plainly inside the workspace came back as an escape. CI found it where this machine could not: on macOS, where a temporary directory is /var against a resolved /private/var, and on Windows, where a path handed over with an 8.3 short name resolves to its long form on one side only. Each side now resolves its deepest existing ancestor and re-attaches the rest. The part that does not exist cannot be a symlink — there is nothing there to be one — and the `..` this exists to catch is resolved away by Abs before any of it runs, so the boundary is unchanged in the direction that matters. The regression is pinned three ways, because the condition that produces it is platform-shaped: a root reached through a symlink, a target that does not exist, and resolveExisting on its own, which needs neither.
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.
A run of
scc launch claudeon Windows fell back to the host unsandboxed and warned about a malformed block in this repository's own entry file. Both causes ship to every scaffolded workspace, and both fail in the direction that looks like success.Plan:
plans/launch-defects.md.What was wrong
A marker written in prose was read as a block.
internal/mdblocklocated its marker-delimited region withstrings.Indexover the raw document, so a file that documents the markers was read as a file carrying the block. This repository's ownCLAUDE.mdnames RTK's opening and closing markers in one sentence, which madertkBlockPresenttrue —scc launchskipped the RTK wiring it believed was already done — and leftscc rtkstanding ready to splice over that sentence. It also names scc's CodeGraph opener alone, which was reported malformed, so that block was never written at all.rtk.ForeignBlockhad the same defect for Headroom's marker, sending the reader toheadroom unwrapfor a block nobody wrote.The dev container could not come up.
postCreateCommandrangit config --global --add safe.directory, and~/.gitconfigis the read-only bind that carries your identity in, so git failed withDevice or resource busy. That failed postCreate, which faileddevcontainer up, which dropped the launch back onto the host — a run that reported a sandbox and started the agent outside one.What changed
mdscan.Maskblanks fenced blocks and inline code spans to spaces, preserving every byte offset and leaving HTML comments intact, because the markers a caller looks for here are HTML comments. Onespanhelper inmdblockmeasures over it and slices out of the original, so four methods cannot disagree about where the block is.The dev container's
safe.directorymoves into the image, where nothing is read-only and the build runs as root — postCreate's non-root user cannot write/etc/gitconfigeither, so neither scope works from there. It is scoped to/workspaces/*rather than*.Coverage
scc check's test gate was failing onmainat 73.6% against an 86% floor, which blocked every push. It now passes: 73.6% → 86.0%, entirely by adding tests. Three packages went from 0%;internal/gitfrom 8.3% to 91%;internal/hooks's settings splice — the half that writes into a file scc does not own — from 0% to 87%.No test installs anything or touches the real workspace: stand-in binaries are written into
t.TempDir()with PATH replaced throught.Setenv, and the git tests pointGIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEMat an empty file so a developer's own config cannot change what the suite measures.Verification
go build ./...,go test ./...,golangci-lint run— green, 0 issues.scc validate— no findings.scc check— no findings, 978 tests, 86% covered against the 86% floor.scc launch claude --dry-runin this repository: no malformed-block warning, no false Headroom block, RTK reported as absent — which is the truth.Reviews
scc-security-review: clean, no findings. It flaggedsafe.directory '*'as high on the first round — disabling git's CVE-2022-24765 ownership check for a container that forwardsGH_TOKENand mounts the agent's credential volume — and confirmed the narrowed/workspaces/*on the second: the Dev Containers CLI mounts there by default, and git's glob covers only the immediate level, not a.gitplanted inside a dependency.scc-code-review: clean, one minor finding — a pre-existing lifecycle test built a git repository without turning commit signing off, which would fail non-interactively on a hardened runner. Fixed indfb536e, along with the plan's## Pathssection, which named three of the four files this work touched.