fix(bindings): classify pack content by store existence, not blind substitution - #117
Merged
Merged
Conversation
…bstitution
The rewrite substituted every {project-root}/_bmad/ reference with the
absolute user-install path, assuming everything under the shim dir is pack
content. It is not. Pack content is what the installer put in the store;
everything else under _bmad/ is project state that upstream creates and
writes at runtime. Blind substitution redirected that state into immutable
shared storage, inside instructions an agent then follows.
Controlled traversal of the synced surface (2,205 files, 173 skill dirs)
found 30 distinct pack references, 14 of them absent from the store,
carried by 32 files. All 14 originate as {project-root}/_bmad/... in the
pack source, so the rewrite created every one. They include the config
write targets already filed, and two classes not previously recorded:
runtime state (the agent-builder memory/{skillName} sanctum, planning
output) and the custom/ surface bmad-customize exists to write.
The store is installed read-only (FreezeTree, aae-orc-dihj), so a path
absent from it can never come into being there. That collapses the
read-versus-write question into an existence test, with none of the
write-verb, flag-name, or redirect heuristics the audit measured as
unreliable. A reference is rewritten only if it exists in the store;
otherwise it stays literal and the fallback footer resolves it
cwd-relative, which is correct for project state.
Existence cannot decide one case: {project-root}/_bmad/custom/ exists in
the store but pack.yaml custom_bridge declares it repo-writable territory.
That is settled by reading the declaration, so the bridge path is
preserved alongside _bmad-custom/ and _bmad-output/.
verify() is the post-condition, run independently of the rewrite so a
reference that arrived absolute in the pack source is caught too. It is
structural, so it gates: a violation fails the binding and skips stale
reconcile, per the loud-failure path from #108/#113.
End-to-end sync of bmad 6.10.0 into a sandboxed HOME: 119 artifacts, zero
dangling references, down from 14 across 32 files, with the 16 legitimate
reads still resolving to the store.
Closes aae-orc-c8v8
Refs aae-orc-dihj, aae-orc-mkpo, aae-orc-3mci
arcaven
added a commit
that referenced
this pull request
Aug 8, 2026
…use (#118) The cache comment claimed safety on the grounds that the store is frozen during a sync and that bindings sync sequentially. Both clauses are true and the claim still rested on an invariant nothing enforced: runSync iterates sequentially today, nothing stops that changing, and the corruption would be silent. A parallel test in #117 shared one packRefRules across subtests, which raced on the map and failed CI on main after the PR checks had passed. No alpha was cut, which stalled the fleet remediation waiting on it. Guard the map with a mutex so the sequential-sync invariant is no longer load-bearing. The production path never raced; the detector found the assertion rather than a bug, and the assertion was the fragile part. Verified with go test ./... -count=1 -race, which is what CI runs and what ax test now runs too. Refs aae-orc-c8v8, finding-119
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.
Sideshow was shipping instructions that tell an agent to write into the frozen pack store. The path rewrite substituted every
{project-root}/_bmad/reference with the absolute user-install path, on the assumption that everything under the shim dir is pack content. It is not: pack content is what the installer put in the store, and everything else under_bmad/is project state that upstream creates and writes at runtime.What is actually broken
Controlled traversal of the synced surface (2,205 files, 173 skill directories) finds 30 distinct pack references, 14 of them absent from the store, carried by 32 files. Every one of the 14 originates as
{project-root}/_bmad/...in the pack source, so the rewrite created all of them.Two classes were not previously recorded:
memory/{skillName}/, the agent-builder sanctum, described in-file as "the place it reloads on every waking" (14 hits). Alsoplanning/prd.md.custom/*.toml, the exact surfacebmad-customizeexists to write.The store is frozen 0555/0444, so these fail loudly rather than corrupting anything. The artifact is still wrong, and an agent following it gets an EACCES traceback instead of a diagnostic.
The fix
Freezing the store (
aae-orc-dihj) makes a path absent from it unreachable by construction: a read fails, a write fails with EACCES. That collapses the read-versus-write question into an existence test.No write-verb, flag-name, or shell-redirect heuristics, which is what makes it reliable; the audit behind the ticket measured those as wrong in both directions. Left-literal references are not orphaned, since the fallback footer already resolves cwd-relative, which is the correct resolution for project state.
Existence cannot decide one case.
{project-root}/_bmad/custom/exists in the store butpack.yaml custom_bridgedeclares it repo-writable territory. That is settled by reading the declaration, so the bridge path is preserved alongside_bmad-custom/and_bmad-output/. The two rules are complementary: existence covers what the store lacks, declaration covers what the store has but does not own.verify()is the post-condition, run independently of the rewrite so a reference that arrived absolute in the pack source is caught too. It is structural rather than a health metric, so it gates per.claude/rules/diagnostic-not-gate.md: a violation fails the binding and skips stale reconcile, per the loud-failure path from #108/#113.Verification
End-to-end sync of bmad 6.10.0 into a sandboxed HOME: 119 artifacts, zero dangling references, down from 14 across 32 files, with the 16 legitimate reads such as
scripts/resolve_customization.pystill resolving to the store.Unit coverage includes the fixture the ticket names (
bmad-bmb-setupline 48), thebmad-customizebare-custom/case, placeholder tails, and a reference that arrived absolute.Cost of merge
Behavior changes for 14 references across 32 synced files; each stops being rewritten and reverts to vanilla bmad semantics. Existing binding tests needed realistic fixtures, since existence is now the classifier.
Two follow-ups worth noting and not in scope here: the user's live
~/.claude/skillsstill carries the old output until the next sync, and sideshow doctor has no layer that scans the synced surface for this class.Closes aae-orc-c8v8
Refs aae-orc-dihj, aae-orc-mkpo, aae-orc-3mci