fix: resolve terse rendering conflict

This commit is contained in:
maxpetrusenkoagent 2026-06-14 14:13:50 -04:00
parent 7b7e2662e5
commit 50e5c736d6
3 changed files with 166 additions and 7 deletions

View File

@ -153,6 +153,39 @@ const EXPLAIN_LEVEL: 'default' | 'terse' = (() => {
return val;
})();
// ─── Out-dir (dev workspace render isolation) ───────────────
// --out-dir <abs-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/<skill>/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

24
setup
View File

@ -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

View File

@ -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');
});
});