From 3858703ac9a8ebf19925ee49a660d34b60bec589 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 26 Oct 2026 14:16:54 +0000 Subject: [PATCH] fix: gstack-doctor finds the Codex model patch in the hoisted shared preamble (ship Step 11, run 4) The doctor's patch row grepped only the Codex review SKILL.md for the "Model-Specific Behavioral Patch (" heading. The hoist moves that section into sections/preamble.md, so the required row (for ship, review, the plan reviews, office-hours, investigate and more) would fail on every correct post-hoist Codex install and block install.sh's "gstack: ok". It now checks the SKILL.md and then its shared preamble and names the file it found. A new doctor test covers a hoisted render. preamble-hoist's variantSubsections also returns null when no subsection differs, instead of emitting a replacing heading with no subsections. Every host's render is unchanged. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 1 + bin/gstack-doctor-check.sh | 6 +++++- scripts/resolvers/preamble-hoist.ts | 1 + test/gstack-doctor.test.ts | 8 ++++++++ 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a88497ef..bfe51d4cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - **`scripts/resolvers/preamble-hoist.ts`** (new) renders `{{PREAMBLE}}` for tier 2+ skills: a section moves to the shared file only when this skill's render of it is byte-identical to the shared render, so no skill's instructions change. Sections that name the skill (the start command, Question Tuning, Repo Ownership, Search Before Building, Telemetry), text without its own heading, and skill-specific variants stay inline; a variant that keeps the shared subsection headings carries only its differing `###` subsections. Tier 1 skills keep their short preamble inline. Across all 41 hoisted skills, the SKILL.md with its shared file applied holds exactly the paragraphs the pre-hoist SKILL.md held. - **`scripts/gen-skill-docs.ts`** emits `sections/preamble.md` per tier 2+ skill and host as a new `preamble` artifact kind, with the same host rewrites, install-root rewrite and runtime preludes as SKILL.md. `setup` already installs every skill's `sections/`, and gstack-built overlay and per-install renders carry the file the same way. - **`setup --status`**: a bare `sections/` pointer counts as carved only when the skill installs a section other than the shared `preamble.md`, so the new file does not turn on checks for skills that inline their sections on that host. +- **`gstack-doctor --check`**: the `patch` row finds the Codex render's model patch in the review skill's `sections/preamble.md` too, where the hoist moves it; without this the required row failed on every hoisted Codex install. - **Loaded-skill skip lists** (`/autoplan`'s section skip list and the `{{INVOKE_SKILL}}` loader's defaults in `scripts/resolvers/composition.ts`) skip `Shared preamble (load first)`, so a loaded plan-review or /office-hours skill no longer Reads its own shared file and re-imports the sections the parent already handles. `Completeness Principle — Boil the Ocean` and `Completion Status Protocol` leave both lists: no loaded skill carries them inline any more, only in the shared file. Each list is 40 characters shorter, so 30 size pins go down (plan-eng-review's by 26, after main's +14 pointer growth). - **PR paid planner** (`scripts/lib/paid-select.ts`): a generated `/sections/preamble.md` takes its skill's `SKILL.md.tmpl` identity, as that skill's SKILL.md already does, so a skill whose only paid coverage is periodic defers it instead of failing `screen-summary` with "requires full validation". diff --git a/bin/gstack-doctor-check.sh b/bin/gstack-doctor-check.sh index 8cc9e2e34..1bd8b06cd 100644 --- a/bin/gstack-doctor-check.sh +++ b/bin/gstack-doctor-check.sh @@ -138,7 +138,11 @@ _dc_check_patch() { _dc_fail_or_skip patch "no review skill in the Codex render at $_dc_hs_dest" "$fix" COMPONENT_MISSING "not needed; no Codex review render" return fi - model=$(grep -F 'Model-Specific Behavioral Patch (' "$file" 2>/dev/null | head -1 | sed 's/.*Model-Specific Behavioral Patch (\([^)]*\)).*/\1/' | clamp 'A-Za-z0-9._:/-' 60) + # A tier 2+ render keeps the patch in the shared sections/preamble.md its SKILL.md points to. + for f in "$file" "${file%/SKILL.md}/sections/preamble.md"; do + model=$(grep -F 'Model-Specific Behavioral Patch (' "$f" 2>/dev/null | head -1 | sed 's/.*Model-Specific Behavioral Patch (\([^)]*\)).*/\1/' | clamp 'A-Za-z0-9._:/-' 60) + [ -n "$model" ] && { file="$f"; break; } + done if [ -n "$model" ]; then _dc_row PASS patch "$model (${file#"$_dc_hs_dest"/})" else diff --git a/scripts/resolvers/preamble-hoist.ts b/scripts/resolvers/preamble-hoist.ts index 7f3728329..603ded7d2 100644 --- a/scripts/resolvers/preamble-hoist.ts +++ b/scripts/resolvers/preamble-hoist.ts @@ -73,6 +73,7 @@ function variantSubsections(own: string, shared: string): string | null { if (ownParts.length < 2 || ownParts.length !== sharedParts.length || ownParts[0] !== sharedParts[0]) return null; if (ownParts.some((part, i) => headingOf(part) !== headingOf(sharedParts[i]!))) return null; const changed = ownParts.slice(1).filter((part, i) => part !== sharedParts[i + 1]); + if (changed.length === 0) return null; return [`${headingOf(own)}\n\nOnly these subsections replace the shared ones; the rest of this section is shared.`, ...changed].join('\n\n'); } diff --git a/test/gstack-doctor.test.ts b/test/gstack-doctor.test.ts index ca7d3ae73..798d6e021 100644 --- a/test/gstack-doctor.test.ts +++ b/test/gstack-doctor.test.ts @@ -528,6 +528,14 @@ describe('gstack-doctor --check', () => { expect(noPatch.trailer).toBe('gstack: fail patch'); }); + test('--for autoplan: a hoisted Codex render carries the patch in its shared sections/preamble.md', () => { + const f = makeFixture(); + const dest = installCodex(f, false); + write(path.join(dest, 'gstack-review/sections/preamble.md'), '# gstack shared preamble 0123456789ab\n\n## Model-Specific Behavioral Patch (gpt-6-astra)\n'); + const r = row(f, ['--for', 'autoplan']); + expect(r.rows.patch, r.out).toEqual({ status: 'PASS', rest: 'gpt-6-astra (gstack-review/sections/preamble.md)' }); + }); + test('an empty installed-host set is never ok: claude is required and FAILs', () => { const f = makeFixture(); write(path.join(f.state, 'installs.tsv'), '');