Skip to content

chore(shipped-layer): drop review-round narrative from the installer comments - #1359

Merged
artyhoo merged 1 commit into
stagingfrom
chore/shipped-layer-drop-round-narrative
Aug 10, 2026
Merged

chore(shipped-layer): drop review-round narrative from the installer comments#1359
artyhoo merged 1 commit into
stagingfrom
chore/shipped-layer-drop-round-narrative

Conversation

@artyhoo

@artyhoo artyhoo commented Aug 10, 2026

Copy link
Copy Markdown
Owner

Summary

Sibling sweep of the S1b round-3 MINOR 2 finding closed in #1356. That fix removed one audit-round retelling from _py_json_rules; the finding named a class, and the class turned out to have nine members, not one. This PR removes the remaining eight from files that install into consumer projects.

Consumers read these seams to understand their own install. MAJOR B (W-7), S2 rework round 1, R2, round-1 rework and a path into .superpowers/sdd/ are our process artefacts: the wave numbers index nothing a consumer can open, and the round numbers describe a review conversation they were never part of.

The sweep that found them is wider than the grep used when the observation was first surfaced (that one reported four sites; it missed the MAJOR A/B (W-n) family entirely).

Changes

Every substantive rationale is preserved verbatim — only round/wave/severity markers and one dead internal report path are removed.

File Removed Kept
10-skills.sh:113 beta-delivery-ux S2 / R3, S5 kickoff §2 binding #3, at S2 rework round 1 the design-SSOT citation carrying the actual reason (beta-program-design.md:211)
10-skills.sh:325 beta-delivery-ux S2 rework round 1 / R7 «consolidate to ONE ship-point» as the reason + the whole call-chain argument
85-worktree-scripts.sh:46 R2 (S2 rework round 1) the ship-alongside rationale (spec A9)
lib.sh:82 (round-1 rework) the concrete leak example — that is the useful part
46-cargo.sh:252, 47-go.sh:219 MAJOR B (W-7) / §3a option B / §6 fork 2 design pointers
45-python.sh:245 (S2-T2 review; .superpowers/sdd/s2-task-2-report.md §Fix round 1) the probe-proven silent-unenforcement finding
45-python.sh:648 MAJOR B (W-7): fragment-per-rule rationale
45-python.sh:670 MAJOR A (W-8): the manifest-path rationale, restated to explain the CURRENT design instead of narrating the defect that preceded it

Deliberately out of scope

Design pointers into internal kickoff/spec sections (§3a, §6 fork 2, spec A8/A9) are a neighbouring class — also unreachable for a consumer, but they carry design intent rather than review history. Removing them would rewrite meaning, not strip narrative, so they are left alone.

One round-2 audit retelling remains at 45-python.sh:520. That is the exact line #1356 fixes; this branch is cut from staging, so the fix is not visible here. Not duplicated on purpose — it lands with that PR.

No gate backs this

There is no mechanical check that keeps review narrative out of shipped files, so this class will recur (attention-is-not-a-mechanism.md §1 — a convention whose only enforcement is «someone notices in review» is not a mechanism). Not built here: that would be a capability commit and a separate concern from the cleanup. Flagged for the maintainer's call.

Prior-art consult

  • The commit carries the escape-hatch Prior-art: trailer. Not a capability commit: no new dependency, no new file, nothing under packages/ — the diff is comment text in six setup.d/ files.
  • No new capability area surfaced.
  • No existing SSOT entries matched (no capability area to match).
  • No context7 queries required — no new capability area.

Test plan

  • SNAPSHOT_MODE=compare bash tests/install-sh/snapshot.sh — 15 pass / 0 fail, byte-identical (no delivered bytes move; these are installer-layer comments with no shipped twin)
  • bash -n clean on all six edited files
  • bash scripts/run-local-ci-sweep.sh — selected shellcheck for this diff, PASS
  • make self-audit — 28/28, 274 passed + 4 skipped, composition 46/46, principles 353 passed + 1 skipped, always-on 31297B within the 54000B ceiling
  • Post-edit re-sweep confirms zero remaining round/wave markers under setup.d/** + packages/core/templates/**, except the 45-python.sh:520 line fix(getff-freshness-widening): S1b — close the two round-3 MINORs (emitted proof corpus · shipped-layer narrative) #1356 owns

Not re-run here: the full npm test --workspace=@rules-as-tests/core --run suite, which is red on this host independently of any change (documented in #1356). This diff is comment-only and moves no delivered byte, so the install snapshot is the load-bearing check.

CI went red once on a flake — recorded rather than quietly re-run

The first CI run failed Principles as meta-tests (Phase 2) on hooks/getff-work.test.ts > CC-DEFERRAL, Test timed out in 5000ms. That test name sits uncomfortably close to this diff — setup.d/85-worktree-scripts.sh is the file that ships getff-work.sh — so it was checked rather than assumed:

  1. The diff cannot reach it. Every changed line in all six files is a comment (git diff -U0 filtered to non-comment lines returns empty), and scripts/getff-work.sh itself is not touched by this PR at all.
  2. It is already red on clean staging. Run at c1848e88fe in a fresh provisioned worktree with none of these commits: 4 failed / 3 passed, including the same CC-DEFERRAL — all four failing on timeout (9550ms, 7800ms, 6966ms, 6245ms against a 5000ms limit), not on an assertion.
  3. It is marginal, not broken. In CI only 1 of those 4 failed; the re-run went green (75 pass).

So the gate was re-run on evidence of non-causation, not on hope. The underlying issue — a spawn-heavy test wired to a 5000ms timeout, red locally and marginal in CI — is pre-existing and out of scope here.

Provenance

n/a

Review findings

n/a

Fidelity verdict

FIDELITY: skipped — non-stage follow-up sweep of a class named by a MINOR finding in #1356; comment-only, no kickoff substrate governs it and no behaviour changes.

Parked questions

n/a

§1.7 Self-discipline check (REQUIRED if PR touches discipline-bearing files)

§1.7 Skipped: mechanical maintenance — comment text removed from installer files; no discipline rule, principle, or skill is introduced or extended, and no changed path matches the discipline trigger.

…comments

Sibling sweep of the S1b round-3 MINOR 2 finding (#1356), which removed one such
retelling from `_py_json_rules`. That fix named a class, and the class had nine members,
not one — a wider grep over `setup.d/**` and `packages/core/templates/**` surfaced eight
more sites where our internal review history is narrated inside files that install into
consumer projects.

Consumers read these seams to understand their own install. «MAJOR B (W-7)», «S2 rework
round 1», «R2», «round-1 rework» and a path into `.superpowers/sdd/` are our process
artefacts: the wave numbers index nothing a consumer can open, and the round numbers
describe a review conversation they were never part of.

Every substantive rationale is preserved verbatim — only the round/wave/severity markers
and the one dead internal report path are removed:

- `10-skills.sh`  — env+ contour surface; the design-SSOT citation that carries the actual
  reason (spec:211 «pipeline presets, status, …») stays.
- `10-skills.sh`  — the §1j relocation note; «consolidate to ONE ship-point» stays as the
  reason, and the call-chain argument below it is untouched.
- `85-worktree-scripts.sh` — getff-work.sh ship-alongside rationale (spec A9) stays.
- `lib.sh` — the concrete leak example stays (it is the useful part); «(round-1 rework)»
  becomes plain observation.
- `46-cargo.sh` / `47-go.sh` — «§3a option B / §6 fork 2» design pointers stay; the
  «MAJOR B (W-7)» prefix goes.
- `45-python.sh` ×3 — probe-proven silent-unenforcement finding stays (the dead
  `.superpowers/sdd/s2-task-2-report.md` pointer goes with the round marker); the
  fragment-per-rule and manifest-path rationales stay, the latter restated so it explains
  the CURRENT design rather than narrating the defect that preceded it.

Design pointers into internal kickoff/spec sections (`§3a`, `§6 fork 2`, `spec A8/A9`) are
a NEIGHBOURING class — also unreachable for a consumer, but they carry design intent
rather than review history. Left alone deliberately; removing them would rewrite meaning,
not strip narrative.

Verified on host: install snapshot 15/15 byte-identical (no delivered bytes move — these
are installer-layer comments with no shipped twin), `bash -n` clean on all six edited
files, local CI sweep green, make self-audit green.

Prior-art: skipped — comment-only sweep removing review narrative from shipped files, no behaviour change, no new capability and no new dependency.
@artyhoo
artyhoo merged commit c52f50e into staging Aug 10, 2026
114 of 116 checks passed
@artyhoo
artyhoo deleted the chore/shipped-layer-drop-round-narrative branch August 10, 2026 08:39
artyhoo added a commit that referenced this pull request Aug 10, 2026
…ually do (#1363)

`packages/core/hooks/getff-work.test.ts` was flaky/red on the vitest 5000ms
default: every case spawns a real `bash scripts/getff-work.sh`, which does
`git worktree add` + node_modules wiring + a package-manager install inside the
new worktree. On clean staging c1848e8 it ran 4 failed / 3 passed, all four
`Test timed out in 5000ms`. In CI it was marginal — CC-DEFERRAL failed on PR
#1359's first run and went green on re-run, having already blocked one unrelated
comment-only PR.

Timing the wrapper directly on a cold worktree measured 30.8s total (dominant leg:
the step-2 `npm ci` inside the new worktree), so the 30_000 SLOW_SHELL_MS value the
sibling suites use is itself too tight here. getff-work gets 60_000 — ~2x the
measured cold cost, with precedent in principles/20-bundle-classification
.paired-negative.test.ts and the root vitest.config.ts, and well under that file's
documented 120_000 ceiling. Its inner spawnSync guard is named SPAWN_GUARD_MS so
the "test timeout must outlive the spawn guard" inequality is visible.

A full-suite sweep (`vitest run hooks/ skills/`) showed the same defect class
across the siblings: 70 failed, 69 of them `timed out in 5000ms`. Fixed at
30_000 (the existing SLOW_SHELL_MS convention):

  hooks/create-worktree.test.ts, hooks/worktree-setup.test.ts,
  hooks/dup-detect-empty-arg.test.ts, hooks/priority-score-branch-matcher.test.ts,
  hooks/priority-score-reconstruct-stub.test.ts, skills/dup-detect.test.ts,
  skills/dup-detect-skip-closed.test.ts

plus two CI-gated files that carried only PARTIAL per-test coverage — the untimed
cases (C1/C2 tier-resolution; the fake-SHA paired-negative) were the ones going
red, and `test:hooks` does not go green under load without them:

  hooks/validate-prompt.test.ts, hooks/check-worker-dispatch-channel.test.ts

Verification (clean tree, macOS, 2026-08-10):
  - getff-work.test.ts alone: 7 passed (7), slowest 32.5s
  - `vitest run hooks/ skills/`: hooks/ failures 15 -> 0; total 70 -> 38
  - `tsc --noEmit`: clean

Timeout configuration only — no assertion, test-body, or `it()` line changed;
`git diff` contains zero touched expect/toBe/toMatch/toContain lines.

Known residue, deliberately NOT in this diff (no drive-by per CLAUDE.md
«PR strategy»): the 38 remaining timeouts live in four skills/ files outside the
audited set — planner-discovery (12), plan-currency-check (11), -digest (6),
-kickoff (5), run-helper (4). They are the same defect class, but skills/ is gated
by no workflow (CI runs test:hooks / test:principles / ir / backends / composition
/ live-generation / render), so they block no PR.

Prior-art: skipped — test timeout configuration only, no new capability, no new
dependency and no new module; the values reuse the SLOW_SHELL_MS convention that
already exists in this package.

Co-authored-by: Test <test@example.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant