docs(handoff): re-scope the SPARK Danger Room lane — scope holds, but the cautionary tale got worse - #2584
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2cb6d8d028
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| ```bash | ||
| git submodule status | grep '^-' && echo "UNPOPULATED" || echo "populated" | ||
| make -C pmoves bind-sources-check |
There was a problem hiding this comment.
In the reviewed tree, bind-sources-check is not defined in pmoves/Makefile, any included file under pmoves/mk/, or elsewhere under pmoves; running this documented acceptance command ends with No rule to make target 'bind-sources-check'. Because the handoff presents the target as newly merged and required for acceptance, SPARK cannot execute the prescribed verification until the target is actually landed or the document points to an existing check.
Useful? React with 👍 / 👎.
| So acceptance 1 should be stated as two commands rather than an inspection: | ||
|
|
||
| ```bash | ||
| git submodule status | grep '^-' && echo "UNPOPULATED" || echo "populated" |
There was a problem hiding this comment.
When any submodule is unpopulated, grep succeeds and the command runs echo "UNPOPULATED", which itself exits 0; therefore shells and automation still treat this acceptance command as passing and continue to the next check. If acceptance is evaluated by command status rather than a human noticing the label, the empty-submodule case this criterion is meant to reject can be accepted, so the negative match should produce a nonzero status (and should include --recursive if nested submodules are part of the contract).
Useful? React with 👍 / 👎.
Operator asked for a refresh against current main rather than a cold assignment of an 8-day-old handoff. Re-measured against origin/main @ 017de53. Append-only -- z890's and 4090's original text is untouched. The scope holds. Three claims re-verified rather than assumed: E2B gitlink still drifted -- working copy 7a38b33b, origin/main records 78f7c5d8. The pre-req is unresolved and is the first task, not a footnote. Still 31 skills. Still zero skills declaring the verification contract. One thing is newly available. Acceptance criterion 1 ("sandbox has populated submodules") was an eyeball check when written; it can now be two commands, because SUBMODULE_BUILD_AND_MOUNT_GAP.md merged (#2485) and bind-sources-check landed (#2581). A sandbox passing both cannot silently be the empty-submodule case, which is the case that yields confident false negatives. One thing got sharper, and it changes the artifact contract. Piece 5 cites archon.crawl.* as "a subject registered against an operation nobody built". That was too kind. It HAS a handler: _process_crawl echoes the request's own metadata back as extracted_text and fragments, stamped "status": "completed", and nothing fetches the URL (documented 2026-08-16, #2582). An unimplemented subject times out and you notice. That one reports success. For skill.verified.v1 the same failure is a receipt reading PASS because the harness echoed what the request handed it -- so the artifact must carry the skill content hash AND something only an actual run could produce: exit codes, wall time, emitted paths. A receipt derivable from the request alone is archon.crawl.result.v1 under a different name. That promotes acceptance (5) -- a deliberately broken skill must produce a RED artifact -- from "the one I would cut last" to the one that defines the lane. A Danger Room that cannot emit RED is a publish-and-echo circuit. Not claimed by 4090. SPARK remains the named owner; this refreshes the lane, it does not take it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2cb6d8d to
05cfbe5
Compare
1 participant
What
Refresh of the SPARK Danger Room handoff against current main before it gets assigned, rather than cold-assigning an 8-day-old document. Append-only — the original text is untouched.
The scope holds. Three claims re-verified rather than carried forward:
7a38b33b,origin/mainrecords78f7c5d8Newly available: acceptance 1 becomes a command
Criterion 1 ("sandbox has populated submodules") was an eyeball check when written. Since then
SUBMODULE_BUILD_AND_MOUNT_GAP.mdmerged (#2485) andbind-sources-checklanded (#2581), so it can now be:A sandbox passing both cannot silently be the empty-submodule case — which is precisely the case that yields confident false negatives.
Sharper: the
archon.crawl.*cautionary tale is worse than the handoff saysPiece 5 cites
archon.crawl.*as the reason to registerskill.verified.v1before first publish — "a subject registered against an operation nobody built." That was too kind, and the real version changes the artifact contract.It has a handler.
_process_crawltakesmetadatafrom the request message and republishes it unchanged asextracted_text/fragments, stamped"status": "completed". Nothing fetches the URL. Its tests assert dispatch routing and never that a crawl occurred. (Documented 2026-08-16 in #2582.)For
skill.verified.v1that is the exact failure to design against: a receipt reading PASS because the harness echoed back what the request handed it. So the artifact must carry the skill content hash and something only a real run could produce — exit codes, wall time, emitted paths. A receipt derivable from the request alone isarchon.crawl.result.v1wearing a different name.That promotes acceptance (5) — a deliberately broken skill must produce a RED artifact — from "the one I would cut last" to the one that defines the lane.
Ownership
Not claimed by 4090. SPARK remains the named owner. This refreshes the lane; it does not take it.
🤖 Generated with Claude Code