fix(install): gate every depth-profile refresh arm + one SSOT for skill slugs (#1312, #1334) - #1412
Merged
Merged
Conversation
…ll slugs (#1312, #1334) do_refresh had drifted from the install arm in BOTH directions. #1312: `arch` reached no refresh loop at all, `claude-glm-executor-handoff` only the install one, and `rule-tests` was ANNOUNCED by the refresh header then dropped by the payload-root guard (its dir lives under .claude/skills/, not skills/). #1334: the worktree-scripts arm carried no profile check, so any --refresh on a `core` project delivered four env+ artefacts — the inverse defect, and the one that makes «what does core contain» unanswerable from --profile alone. One resolution for both: every depth-gated refresh arm now uses the delivery site's own profile predicate OR presence-on-disk (presence = prior opt-in, the brownfield upgrade path #1312 is about), and the per-tier slug lists collapse into shared GETFF_SKILLS_{CORE,ENV,FACTORY} constants both arms read — #1312's own suggested direction, since its three "drifts" were three copies of one list. tier-home moves from presence-ONLY onto the same gate (named in #1334's arm table): without it `--refresh --profile env` would deepen some arms and not others — the «each arm decides for itself» state INSTALL-FOR-AI.md had to describe in a paragraph. The vendored runtime-bridge payload still has no refresh arm; that is a pre-existing gap of the same class, recorded in the doc as install-only rather than silently widened into this fix. Gates, paired-negative both: tests/install-sh/consumer-upgrade-path.test.sh TESTs 8-11 are behavioural over real installs (a core refresh delivers nothing deeper; a bare refresh keeps an opted-in factory payload fresh; the header cannot announce what the run skips; an explicit --profile env DOES deliver, which is TEST 8's paired negative — same paths, same tree). refresh-covers-full-delivery.test.sh extends the #1327 parity gate rather than adding a parallel one: both arms must read every tier constant, and no literal-slug loop may reintroduce a copy. Measured: consumer-upgrade-path 37/44 → 44/44, refresh-covers 9/11 → 13/13, snapshot compare 15/15 byte-identical, install-sh battery green. Closes #1312 Closes #1334 Prior-art: skipped — bug fix inside existing install/refresh arms plus one shared constant, no new capability area
Merged
11 tasks
artyhoo
added a commit
that referenced
this pull request
Aug 17, 2026
…yload (#1418) ## Summary `setup.d/55-runtime-bridge-vendor.sh` delivers both halves of the factory-depth bridge — the `.claude/vendor/runtime-bridge/` payload and the `.claude/hooks/runtime-bridge-dispatch.sh` dispatch hook — and `do_refresh` had no arm for either, so a factory consumer could never receive a vendor fix non-destructively (the hook is frozen forever by `copy_safe`'s skip-if-exists; the payload only moves when the whole installer re-runs, which is the destructive path `--refresh` exists to avoid). Same refresh-drift class as #869 / #1312 / #1334, deliberately left out of #1412's scope and recorded as install-only in `INSTALL-FOR-AI.md:168`. Closing it first required closing two defects in the gate that covers that class, both of which made the fix untestable and both of which are why the gap was never reported. ## Changes - **`install.sh`** — new `do_refresh` arm for the vendored runtime-bridge, on the uniform #1412 shape: the delivery site's own profile predicate (`factory | WITH_AIF_SUITE`, `setup.d/55-runtime-bridge-vendor.sh:65`) OR presence on disk (prior opt-in). The vendor directory is the presence probe — layer 55 writes both halves in one gated block. `refresh_safe` replaces a directory payload rather than nesting into it (#873), so the dir ships as-is. - **`tests/install-sh/refresh-covers-full-delivery.test.sh` — gate defect 1, variable-indirected destinations.** `FULL` was built by grepping literal `$PROJECT_ROOT` tokens off `copy_safe` lines, so a delivery written `copy_safe "$HOOK_SRC" "$HOOK_DST"` contributed **nothing** and the artefact escaped the set entirely — invisible, not flagged. Live blast radius when measured: all of layer 55, plus all eight `.claude/hooks/` deliveries in `setup.d/10-skills.sh`. Now resolves simple `VAR="$PROJECT_ROOT/<literal>"` assignments per layer file before extraction, with a named allowlist of the remaining unreadable dst forms so a **new** one fails the gate instead of silently escaping. - **same file — gate defect 2, presence probes counted as writes.** Every depth-gated arm #1312/#1334 introduced ends with `|| [ -e "$PROJECT_ROOT/<artefact>" ]`, which reads the path and never writes it, yet `refresh_writes()` counted it — the same false-GREEN shape the existing `chmod_safe` exclusion already guarded against. Test-expression groups are now stripped (not their lines dropped: guarded one-liners like `[ -f "$src" ] && refresh_safe "$src" "$dst"` carry a genuine write on the same line). - **same file** — dedicated layer-55 parity check for the `cp -r` **directory** payload, which no verb scan can reach, with a paired negative. - **`tests/install-sh/consumer-upgrade-path.test.sh`** — both destinations added to `FACTORY_ONLY` (TESTs 8/11: a core-depth `--refresh` must not create them) and to `REFRESH_TARGETS9` (TEST 9: a factory consumer's stale copies are refreshed by a bare `--refresh`). - **`INSTALL-FOR-AI.md:168`** — the paragraph stated the missing arm as a fact; rewritten, and the factory-only list in the depth-boundary bullet extended. ## Prior-art consult - [x] Commit carries `Prior-art: skipped — refresh-parity bug fix plus two gate-extraction fixes in an existing test; no new capability, no new dependency, no new module.` - [x] No new capability area surfaced (no new dependency; no new file ≥80 LOC under `packages/`; no new subdirectory of `packages/core/`). - [x] n/a — no existing SSOT entry matched (nothing consulted, nothing to re-date). - [x] n/a — context7 is scoped to new capability areas; none here. ## Test plan - [x] `tests/install-sh/refresh-covers-full-delivery.test.sh` → **17 pass / 0 fail** - [x] `tests/install-sh/consumer-upgrade-path.test.sh` → **46 pass / 0 fail** - [x] `SNAPSHOT_MODE=compare bash tests/install-sh/snapshot.sh` → **15 pass / 0 fail** (install path untouched, no baseline regen) - [x] Refresh-adjacent suite green: `refresh-safe-dir-payload` (6), `byte-identical` (2), `with-aif-suite-flag` (59), `bridge-guided` (11), `layer-units` (33), `refresh-regenerates-barrel` (10), `refresh-reconciles-skill-rename` (10), `refresh-different-stack-prunes-barrel` (9), `refresh-offers-lintstaged-migration` (11), `snapshot-exclusion-guard` (7), `lib-helpers` (20), `meta-all-wired` (2) - [x] `shellcheck --exclude=SC2034,SC2016,SC2317 setup.d/*.sh install.sh` green (CI gate's exact form); the two touched test files green under the same flags - [x] `npm run format:check` green; pre-push gates green on push - [x] **Non-vacuity, proven live by removing each real fix and watching the gate flip** — not asserted, measured: - neutralise the vendor-dir `refresh_safe` → only `gh-1312 pos: .claude/vendor/runtime-bridge/README.md still stale` reddens (45/1) - neutralise the hook `refresh_safe` → only `gh-1312 pos: .claude/hooks/runtime-bridge-dispatch.sh still stale` reddens (45/1) - drop the whole arm → static gate flags `.claude/hooks/runtime-bridge-dispatch.sh` as OMITTED (16/1) - drop only the dir line → the new layer-55 parity check reddens **while Check 1 stays green**, which is exactly why that check exists (16/1) - **defect 2 regression proof:** before the fix, deleting the vendor payload's real `refresh_safe` line left the gate GREEN — and so did deleting `tier-home`'s. Both now redden. ## Provenance n/a — not a stage PR. ## Review findings n/a — not a stage PR. Two findings surfaced *during* implementation and are folded into this PR because the fix was untestable without them (gate defects 1 and 2 above). One observation is deliberately **not** actioned here, per the no-drive-by-PR rule: `setup.d/46-cargo.sh` and `setup.d/47-go.sh` sit in the npm-lane layer set but deliver through `$tpl` / `$dst` / `$wf_dst` locals that no refresh-parity assertion covers — the python lane got its own Check 4, these two lanes have nothing equivalent. Worth a separate task if wanted. ## Fidelity verdict FIDELITY: skipped — non-stage bug-fix PR against an install/test surface; no kickoff or stage spec applies to it. ## Parked questions n/a ## §1.7 Self-discipline check (REQUIRED if PR touches discipline-bearing files) ### §1.7 Skipped: mechanical maintenance — a refresh-parity bug fix plus two extraction fixes inside an existing gate; no discipline rule is introduced, extended, or re-scoped by this PR.
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
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
do_refreshhad drifted from the install arm in both directions, and the two directions are the two issues this PR closes. #1312: three skill slugs shipped on install but reached no refresh loop —arch(in none at all),claude-glm-executor-handoff(factory install loop only), andrule-tests, which the refresh header announced and then skipped because its payload lives under.claude/skills/, notskills/. #1334: the worktree-scripts arm carried no profile check at all, so any--refreshon acoreproject delivered fourenv+artefacts — the inverse defect, and one that makes «what doescorecontain» unanswerable from--profilealone.Both are the same seam, so both get the same resolution: every depth-gated refresh arm now uses the delivery site's own profile predicate OR presence-on-disk (presence = prior opt-in, the brownfield upgrade path), and the per-tier slug lists collapse into one shared constant each so the two arms cannot drift again.
Changes
setup.d/lib.sh:58-60—GETFF_SKILLS_CORE/_ENV/_FACTORY: the slug lists as an SSOT read by both arms. Issue install.sh --refresh does not ship arch / rule-tests / claude-glm-executor-handoff — install↔refresh skill-slug parity drift (rule-tests is announced then skipped) #1312's own suggested direction feat: audit fixes Phase 1+2+3 (R2/R11 drift, manifest SSOT, depcruise integration) #1; the three drifts it measured were three copies of one list, not three bugs.setup.d/10-skills.sh:113/127/133— the install arm reads the constants; its▶ Contour surface/▶ AIF operator suiteannouncements render from them too.install.sh:684-701— the refresh arm reads the same constants, in three loops with three gates: core unconditional, env+ (env|factory|WITH_AIF_SUITEOR presence), factory (factory|WITH_AIF_SUITEOR presence).arch,pipelineandclaude-glm-executor-handoffare refreshed for the first time;rule-testsmoves here from the plain-copy loop, where its source path could never resolve.install.sh:643— the plain-copy header renders from_PLAIN_SKILLS, so it can no longer announce a slug the loop does not carry (install.sh --refresh does not ship arch / rule-tests / claude-glm-executor-handoff — install↔refresh skill-slug parity drift (rule-tests is announced then skipped) #1312's honest-signals half).install.sh:1030— the worktree-scripts arm gains the gate it never had (install.sh --refresh delivers env+ worktree scripts onto a core project — ungated refresh arm breaks the profile depth boundary (inverse of #1312) #1334). Presence probe isscripts/create-worktree.sh: the cluster's load-bearing entry point, present in every version of it, and the four ship together by construction — so a consumer who opted in beforegetff-work.shjoined the cluster still receives it.install.sh:1128— tier-home moves from presence-ONLY to the same uniform gate. Without it,--refresh --profile envwould deepen some arms and not others: the «each arm decides for itself» stateINSTALL-FOR-AI.mdpreviously had to describe in a paragraph. Named in install.sh --refresh delivers env+ worktree scripts onto a core project — ungated refresh arm breaks the profile depth boundary (inverse of #1312) #1334's own arm table as part of this decision.tests/install-sh/consumer-upgrade-path.test.sh:447-612— TESTs 8-11, the behavioural gates: core refresh delivers no deeper artefact (install.sh --refresh delivers env+ worktree scripts onto a core project — ungated refresh arm breaks the profile depth boundary (inverse of #1312) #1334), a bare--refreshkeeps an already-opted-in factory payload fresh (install.sh --refresh does not ship arch / rule-tests / claude-glm-executor-handoff — install↔refresh skill-slug parity drift (rule-tests is announced then skipped) #1312), the▶ Skillsheader cannot announce what the run does not touch, and--refresh --profile envDOES deliver — which is the paired-negative for TEST 8's absence claim (same paths, same tree, present as soon as the profile says so).tests/install-sh/refresh-covers-full-delivery.test.sh:296-345— extends the existing parity gate (added by feat(beta-delivery-ux): S2 — pipeline presets + status + workspace one-command, with the refresh-drift gate closed #1327 for the same worktree cluster) rather than adding a parallel one: both arms must READ every tier constant, and neither may carry a literal-slugfor _skill inloop (a fourth copy). Two paired-negative arms.INSTALL-FOR-AI.md:152/160-166/444— re-measured. The old text documented both defects as consumer-observable behaviour; leaving it would have made the shipped doc lie in the opposite direction.Prior-art consult
packages/. Commit carries the escape-hatch trailer with rationale.install.sh:621agents arm, F7 skills arm) and the parity-test precedent from feat(beta-delivery-ux): S2 — pipeline presets + status + workspace one-command, with the refresh-drift gate closed #1327.Test plan
bash tests/install-sh/consumer-upgrade-path.test.sh— RED baseline before the fix:PASS=37 FAIL=7(the four worktree scripts leaking onto a core refresh ×2 arms;arch,rule-tests,claude-glm-executor-handoffstale after a bare refresh; the announced-then-skippedrule-tests;arch/pipeline/tier-homemissing under--refresh --profile env). After:PASS=44 FAIL=0.bash tests/install-sh/refresh-covers-full-delivery.test.sh— before:PASS=9 FAIL=2; after:PASS=13 FAIL=0(both new checks plus their negatives).SNAPSHOT_MODE=compare bash tests/install-sh/snapshot.sh— exit 0, baselines unchanged (the install arm ships the same bytes; only its source of names moved).tests/install-sh/*.test.shbattery — see the run summary in the review findings section.shellcheck --exclude=SC2034,SC2016,SC2317 setup.d/*.sh install.sh— exit 0 (CI's exact invocation,.github/workflows/audit-self.yml:804).--profile coreinstall → bare--refresh→--refresh --profile core→--refresh --profile env, asserting the depth boundary at each step (this is TESTs 8/11 run end-to-end against real installs, not a mocked harness).Provenance
n/a — issue-driven fix, not a stage PR. Base:
staging@049b35e851. In-session (no aif substrate).Review findings
n/a — see Test plan for the RED→GREEN evidence.
Fidelity verdict
FIDELITY: skipped — issue-driven bug fix with no kickoff or spec stage behind it; the acceptance contract is the two issue bodies (#1312, #1334) plus the paired-negative gates listed in the Test plan.
Parked questions
n/a
§1.7 Forward-check applied
Checked against the disciplines this diff touches.
dual-implementation-discipline.md §7(single source of truth): the two hand-maintained slug lists collapse tosetup.d/lib.sh:58-60, read bysetup.d/10-skills.sh:113andinstall.sh:684— the duplication that drifted three times is gone rather than re-asserted.attention-is-not-a-mechanism.md §1: every claim in this PR lands on a deterministic channel — the depth boundary ontests/install-sh/consumer-upgrade-path.test.sh:447, the announce↔deliver honesty on:554, the SSOT-consumption invariant ontests/install-sh/refresh-covers-full-delivery.test.sh:296; none of them is «a reviewer will read the diff».no-paid-llm-in-ci.md: the new gates are bash + real installs, zero API calls.effort-worthiness.md §1:build-and-verifycontour — a reversible installer change verified live against real consumer trees, no research-grade round.evidence-regeneration.md:INSTALL-FOR-AI.md:160was a dated measurement of the old behaviour, so it was re-measured (2026-08-17) instead of silently inherited.§1.7 Backward-check applied
Swept every profile-gated install layer for its refresh counterpart, not just the two the issues named:
setup.d/20-agents.sh:39(suite agents) → gated arm exists atinstall.sh:621;setup.d/20-agents.sh:70(skill-context) → gated arm exists atinstall.sh:1146;setup.d/30-templates.sh:108(tier-home) → was presence-only, unified here;setup.d/85-worktree-scripts.sh:34(worktree cluster) → the #1334 defect, fixed here;setup.d/55-runtime-bridge-vendor.sh:65(factory vendor payload) → has NO refresh arm at all — a pre-existing #1312-class gap, outside both issues' scope, so it is recorded inINSTALL-FOR-AI.md:166as install-only rather than silently fixed in this PR. Also swept for stale claims about the changed behaviour:grep -rln "create-worktree\|getff-work.sh" tests/→ only the file updated here;grep -rn 'for _skill in' setup.d install.sh→ no literal-slug loop survives (now asserted);INSTALL-FOR-AI.md:152/160-166/444were the only prose stating the old semantics, and #1329's weakened claim (repaired in #1332) still holds under the new behaviour.Closes #1312
Closes #1334