diff --git a/scripts/gen-skill-docs.ts b/scripts/gen-skill-docs.ts index 39c2ed0de..74a9d997f 100644 --- a/scripts/gen-skill-docs.ts +++ b/scripts/gen-skill-docs.ts @@ -153,6 +153,39 @@ const EXPLAIN_LEVEL: 'default' | 'terse' = (() => { return val; })(); +// ─── Out-dir (dev workspace render isolation) ─────────────── +// --out-dir redirects Claude SKILL.md + section output to a separate +// (untracked) directory instead of writing in place, AND rewrites the literal +// section-base path (`~/.claude/skills/gstack//sections/`) inside the +// generated content to point at the out-dir, so section Reads resolve to the +// rendered copy rather than the global install. Used by bin/dev-setup to render +// the gbrain `:user` variant for a Conductor workspace without dirtying tracked +// source. Default (unset) = in-place, behavior unchanged. Claude host only. +const OUT_DIR_ARG = process.argv.find(a => a.startsWith('--out-dir')); +const OUT_DIR: string | null = (() => { + if (!OUT_DIR_ARG) return null; + const val = OUT_DIR_ARG.includes('=') + ? OUT_DIR_ARG.split('=')[1] + : process.argv[process.argv.indexOf(OUT_DIR_ARG) + 1]; + if (!val) throw new Error('--out-dir requires a directory path'); + return path.resolve(val); +})(); + +/** + * When rendering to an out-dir, repoint the literal section-base path at the + * out-dir so section Reads resolve to the rendered copy, not the global install. + * Surgical: ONLY paths containing `/sections/` are rewritten — bin/, browse/, + * docs/ references keep pointing at `~/.claude/skills/gstack` (the global + * install, which still works). No-op when --out-dir is unset. + */ +function rewriteSectionBase(content: string): string { + if (!OUT_DIR) return content; + return content.replace( + /~\/\.claude\/skills\/gstack\/([^\s)`"'*]+\/sections\/)/g, + `${OUT_DIR}/$1`, + ); +} + // HostPaths, HOST_PATHS, and TemplateContext imported from ./resolvers/types (line 7-8) // Design constants (AI_SLOP_BLACKLIST, OPENAI_HARD_REJECTIONS, OPENAI_LITMUS_CHECKS) // live in ./resolvers/constants and are consumed by resolvers directly. @@ -784,6 +817,12 @@ function processTemplate(tmplPath: string, host: Host = 'claude'): { outputPath: // Determine skill directory relative to ROOT const skillDir = path.relative(ROOT, path.dirname(tmplPath)); + // --out-dir (Claude only): mirror the skill tree into the out-dir instead of + // writing in place. External hosts compute their own paths below. + if (OUT_DIR && host === 'claude') { + outputPath = path.join(OUT_DIR, skillDir, path.basename(tmplPath).replace(/\.tmpl$/, '')); + } + // Extract name/description: name drives external skill naming + setup symlinks // (and TemplateContext.skillName via buildContext); description feeds external // host metadata. When frontmatter name: differs from directory name (e.g. @@ -838,6 +877,9 @@ function processTemplate(tmplPath: string, host: Host = 'claude'): { outputPath: } } + // --out-dir: repoint section-base paths to the out-dir (no-op otherwise). + if (host === 'claude') content = rewriteSectionBase(content); + return { outputPath, content, symlinkLoop, catalogParts }; } @@ -876,6 +918,10 @@ function processSectionTemplate( // External hosts: rewrite cross-reference paths/tools (no frontmatter to transform). if (host !== 'claude') { content = applyHostRewrites(content, hostConfig); + } else { + // --out-dir: a section may cross-reference another section by absolute path; + // repoint those to the out-dir too (no-op when --out-dir is unset). + content = rewriteSectionBase(content); } // Plain generated header (no frontmatter to insert after). @@ -884,7 +930,7 @@ function processSectionTemplate( const fileName = path.basename(sectionTmplPath).replace(/\.tmpl$/, ''); let outputPath: string; if (host === 'claude') { - outputPath = path.join(ROOT, skillDir, 'sections', fileName); + outputPath = path.join(OUT_DIR || ROOT, skillDir, 'sections', fileName); } else { const externalName = externalSkillName(skillDir, parentName); outputPath = path.join(ROOT, hostConfig.hostSubdir, 'skills', externalName, 'sections', fileName); @@ -949,7 +995,7 @@ for (const currentHost of hostsToRun) { voice_line: catalogParts.voiceLine, }; } - const relOutput = path.relative(ROOT, outputPath); + const relOutput = path.relative(OUT_DIR || ROOT, outputPath); if (symlinkLoop) { console.log(`SKIPPED (symlink loop): ${relOutput}`); @@ -962,6 +1008,9 @@ for (const currentHost of hostsToRun) { console.log(`FRESH: ${relOutput}`); } } else { + // In-place writes land in existing dirs; --out-dir needs the mirrored + // skill dir created first. + if (OUT_DIR) fs.mkdirSync(path.dirname(outputPath), { recursive: true }); fs.writeFileSync(outputPath, content); console.log(`GENERATED: ${relOutput}`); } @@ -998,7 +1047,7 @@ for (const currentHost of hostsToRun) { currentHostConfig.generation.skipSkills.includes(sec.skillDir)) continue; const { outputPath, content } = processSectionTemplate(path.join(ROOT, sec.tmpl), sec.skillDir, currentHost); - const relOutput = path.relative(ROOT, outputPath); + const relOutput = path.relative(OUT_DIR || ROOT, outputPath); if (DRY_RUN) { const existing = fs.existsSync(outputPath) ? fs.readFileSync(outputPath, 'utf-8') : ''; @@ -1095,7 +1144,9 @@ The orchestrator will persist the plan link to its own memory/knowledge store. // No timestamp field — keeps the file content-deterministic across runs so // CI dry-run freshness checks don't flap on regen. If a per-run timestamp // is ever needed for debugging, write it to a separate `.gen-stamp` file. - if (currentHost === 'claude' && CATALOG_MODE === 'trim' && Object.keys(proactiveAggregate).length > 0 && !DRY_RUN) { + // Skip the global proactive-suggestions.json in --out-dir mode: it lives at + // a repo path (scripts/) and the dev workspace render doesn't need it. + if (currentHost === 'claude' && CATALOG_MODE === 'trim' && Object.keys(proactiveAggregate).length > 0 && !DRY_RUN && !OUT_DIR) { const proactivePath = path.join(ROOT, 'scripts', 'proactive-suggestions.json'); // Sort keys alphabetically so the serialized JSON is identical across // machines regardless of filesystem-iteration order. Without this, CI diff --git a/setup b/setup index b495ffca9..163548e95 100755 --- a/setup +++ b/setup @@ -1341,6 +1341,7 @@ fi # already registered under that tag, the install is a no-op (no prompt). PLAN_TUNE_LOG_HOOK="$SOURCE_GSTACK_DIR/hosts/claude/hooks/question-log-hook" PLAN_TUNE_PREF_HOOK="$SOURCE_GSTACK_DIR/hosts/claude/hooks/question-preference-hook" +AUQ_ERROR_FALLBACK_HOOK="$SOURCE_GSTACK_DIR/hosts/claude/hooks/auq-error-fallback-hook" PLAN_TUNE_INSTALL_MARKER="$HOME/.gstack/.plan-tune-hooks-prompted" if [ "$NO_TEAM_MODE" -ne 1 ] \ @@ -1348,9 +1349,13 @@ if [ "$NO_TEAM_MODE" -ne 1 ] \ && [ -x "$PLAN_TUNE_LOG_HOOK" ] \ && [ -x "$PLAN_TUNE_PREF_HOOK" ]; then - # Already installed? Check the settings.json for our source tag. + # Already installed? Require BOTH the plan-tune source AND the AUQ-error-fallback + # source — so an existing install that predates the fallback hook re-runs the + # install (which is idempotent for the plan-tune hooks) and picks up the new one. ALREADY_INSTALLED=0 - if "$SETTINGS_HOOK" list-sources 2>/dev/null | grep -q "plan-tune-cathedral"; then + _HOOK_SOURCES=$("$SETTINGS_HOOK" list-sources 2>/dev/null || true) + if printf '%s' "$_HOOK_SOURCES" | grep -q "plan-tune-cathedral" \ + && printf '%s' "$_HOOK_SOURCES" | grep -q "auq-error-fallback"; then ALREADY_INSTALLED=1 fi @@ -1388,6 +1393,21 @@ if [ "$NO_TEAM_MODE" -ne 1 ] \ --command "$PLAN_TUNE_PREF_HOOK" \ --source plan-tune-cathedral \ --timeout 5 + # AskUserQuestion-failure prose-fallback reliability hook (OV3:B). Fires only when + # an AskUserQuestion call returns an error/missing result; inert on success and + # inert if the platform doesn't invoke PostToolUse on tool errors. MUST use its + # OWN source tag: gstack-settings-hook dedupes by (event, matcher, source) and + # REPLACES the entry's hooks, so sharing 'plan-tune-cathedral' would overwrite the + # question-log capture hook (same event+matcher). A distinct source = a second + # PostToolUse entry; both run in parallel. + if [ -x "$AUQ_ERROR_FALLBACK_HOOK" ]; then + "$SETTINGS_HOOK" add-event \ + --event PostToolUse \ + --matcher '(AskUserQuestion|mcp__.*__AskUserQuestion)' \ + --command "$AUQ_ERROR_FALLBACK_HOOK" \ + --source auq-error-fallback \ + --timeout 5 + fi } if [ "$ALREADY_INSTALLED" -eq 1 ]; then diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index 2ec9fe4dd..a1c50d55c 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect } from 'bun:test'; +import { describe, test, expect, beforeAll } from 'bun:test'; import { COMMAND_DESCRIPTIONS } from '../browse/src/commands'; import { SNAPSHOT_FLAGS } from '../browse/src/snapshot'; import * as fs from 'fs'; @@ -2157,7 +2157,24 @@ describe('Factory generation (--host factory)', () => { import { ALL_HOST_CONFIGS, getExternalHosts } from '../hosts/index'; +const EXTERNAL_HOSTS_WITH_TRACKED_SIDECARS = new Set(['openclaw']); +function getIgnoredExternalHosts() { + return getExternalHosts().filter(host => !EXTERNAL_HOSTS_WITH_TRACKED_SIDECARS.has(host.name)); +} + describe('Parameterized host smoke tests', () => { + // Regenerate ignored external host outputs up front so the per-host `--dry-run` + // freshness checks are deterministic. Hosts with tracked sidecars are excluded: + // pre-regenerating them would dirty tracked files and mask real stale-output + // failures that dry-run is supposed to catch. + beforeAll(() => { + for (const h of getIgnoredExternalHosts()) { + Bun.spawnSync(['bun', 'run', 'scripts/gen-skill-docs.ts', '--host', h.name], { + cwd: ROOT, stdout: 'pipe', stderr: 'pipe', + }); + } + }); + for (const hostConfig of getExternalHosts()) { describe(`${hostConfig.displayName} (--host ${hostConfig.name})`, () => { const hostDir = path.join(ROOT, hostConfig.hostSubdir, 'skills'); @@ -2165,6 +2182,7 @@ describe('Parameterized host smoke tests', () => { test('generates output that exists on disk', () => { // Generated dir should exist (created by earlier bun run gen:skill-docs --host all) if (!fs.existsSync(hostDir)) { + if (EXTERNAL_HOSTS_WITH_TRACKED_SIDECARS.has(hostConfig.name)) return; // Generate if not already done Bun.spawnSync(['bun', 'run', 'scripts/gen-skill-docs.ts', '--host', hostConfig.name], { cwd: ROOT, stdout: 'pipe', stderr: 'pipe', @@ -2210,6 +2228,7 @@ describe('Parameterized host smoke tests', () => { }); test('generates Claude outside-voice skill for external hosts', () => { + if (!fs.existsSync(hostDir)) return; const skillMd = path.join(hostDir, 'gstack-claude', 'SKILL.md'); expect(fs.existsSync(skillMd)).toBe(true); const content = fs.readFileSync(skillMd, 'utf-8'); @@ -2241,6 +2260,16 @@ describe('Parameterized host smoke tests', () => { // ─── --host all tests ──────────────────────────────────────── describe('--host all', () => { + // Same determinism guard as the parameterized block, but only for ignored host + // outputs. Tracked sidecars must stay untouched so --dry-run can catch staleness. + beforeAll(() => { + for (const h of getIgnoredExternalHosts()) { + Bun.spawnSync(['bun', 'run', 'scripts/gen-skill-docs.ts', '--host', h.name], { + cwd: ROOT, stdout: 'pipe', stderr: 'pipe', + }); + } + }); + test('--host all generates for all registered hosts', () => { const result = Bun.spawnSync(['bun', 'run', 'scripts/gen-skill-docs.ts', '--host', 'all', '--dry-run'], { cwd: ROOT, stdout: 'pipe', stderr: 'pipe', @@ -3261,3 +3290,62 @@ describe('EXIT PLAN MODE GATE placement', () => { expect(codex).toContain('Failing this gate and calling ExitPlanMode anyway is a contract violation'); }); }); + +describe('GSTACK REVIEW REPORT mandatory unresolved-decisions status', () => { + // Report text rides in PLAN_FILE_REVIEW_REPORT → every report consumer gets it. + // devex-review is a report consumer but NOT a gate consumer, so the two target + // sets differ (CP5/CX5). Regression guard: a future token-cut that drops the + // unresolved-status line again fails here. See plan-flag-unresolved-issues. + const REPORT_CONSUMERS = [ + 'plan-ceo-review', + 'plan-eng-review', + 'plan-design-review', + 'plan-devex-review', + 'codex', + 'devex-review', + ]; + // Gate text rides in EXIT_PLAN_MODE_GATE (lives in SKILL.md, not sections). + const GATE_SKILLS = [ + 'plan-ceo-review', + 'plan-eng-review', + 'plan-design-review', + 'plan-devex-review', + 'codex', + ]; + + for (const skill of REPORT_CONSUMERS) { + test(`${skill}: report mandates the unresolved-decisions status as final content`, () => { + const content = readSkillUnion(skill); + expect(content).toContain('NO UNRESOLVED DECISIONS'); + // The "never omit / always final" contract must be present, not just the phrase. + expect(content).toContain('Unresolved-decisions status (MANDATORY'); + expect(content).toMatch(/never omitted/); + // \s+ tolerates prose line-wraps within "final non-whitespace line". + expect(content).toMatch(/final\s+non-whitespace\s+line/); + }); + } + + for (const skill of GATE_SKILLS) { + test(`${skill}: exit gate blocks unless the unresolved status is the final line`, () => { + const md = fs.readFileSync(path.join(ROOT, skill, 'SKILL.md'), 'utf-8'); + // Gate check #4 — present, sentinel named, and explicitly blocking (no escape). + expect(md).toContain('NO UNRESOLVED DECISIONS'); + expect(md).toContain('FINAL non-whitespace line is the unresolved-decisions'); + expect(md).toContain('FAILS the gate'); + }); + } + + test('scripts/resolvers/review.ts source carries the mandatory block + blocking gate', () => { + const src = fs.readFileSync(path.join(ROOT, 'scripts', 'resolvers', 'review.ts'), 'utf-8'); + // Report resolver: mandatory, never-omitted, exact sentinel, anti-double-count algorithm. + expect(src).toContain('Unresolved-decisions status (MANDATORY'); + expect(src).toContain('NO UNRESOLVED DECISIONS'); + expect(src).toContain('avoids double-counting'); + expect(src).toContain('DROP the current skill'); + // Gate resolver: the blocking final-line check with no "if applicable" escape. + expect(src).toContain('FINAL non-whitespace line is the unresolved-decisions'); + expect(src).toContain('FAILS the gate'); + // The old soft wording must be gone from the gate. + expect(src).not.toContain('absorbs CODEX / CROSS-MODEL / UNRESOLVED lines if applicable'); + }); +});