mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
fix(setup,relink): plan_mode_preamble=off survives builds and failed renders; alias follows the router
Review findings on the plan_mode_preamble wiring: - a build renders env hosts without --plan-mode=off, so with the key off setup no longer skips their re-render after a build; - with the key off the gbrain-absent branch leaves the render to the plain re-render, so a failed render keeps the previous one; - gstack-relink refreshes the _gstack-command alias after re-rendering the router (also fixes the same staleness for disabled_skills). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
cff65f7229
commit
42b64c2071
5 files changed
+106
-10
No files matched your search
+5
-2
@@ -7,8 +7,11 @@
|
||||
### Added
|
||||
- **`gen-skill-docs --plan-mode=off`** drops the two sections at generation time. Commands marked "PLAN MODE EXCEPTION — ALWAYS RUN" keep their marker. Hosts whose capabilities declare no plan mode already render the sections empty, so in practice the flag matters for Claude, Copilot and OpenCode. Fixes #3168. From #3213 by @xingzihai, finished with the config wiring.
|
||||
- **The `plan_mode_preamble` config key** (`on` | `off`, default `on`). It is a closed domain like `explain_level`: an unknown value warns and stores `on`. Setting it outside `./setup` says it applies on the next setup run and prints the `./setup --refresh-registered` command that applies it now.
|
||||
- **Every installed render carries the choice:** all 16 `gen:skill-docs` call sites in `setup`, the Claude render helper, `gstack-config gbrain-refresh`, and the router that `gstack-relink` re-renders in place (as it already does for `disabled_skills`).
|
||||
- **The default Claude install gets a render when the key is off.** `gstack_claude_render_action` returns `plain` whatever gbrain detection says, and the activation record (`~/.gstack/render/claude.overlay`) gains `plan_mode=off`. Turning the key back on removes that render on the next setup, including when the gbrain detector fails, which otherwise keeps a render. Records written with the key on are unchanged.
|
||||
- **Every installed render carries the choice:** all 16 `gen:skill-docs` call sites in `setup`, the Claude render helper, `gstack-config gbrain-refresh`, and the router that `gstack-relink` re-renders in place (as it already does for `disabled_skills`). When setup has to build, `scripts/build.sh` renders Factory, OpenCode, Cursor, Copilot and Antigravity without the flag, so with the key off setup renders those hosts again afterwards.
|
||||
- **The default Claude install gets a render when the key is off.** Without a working gbrain (absent, or the detector failed), `gstack_claude_render_action` returns `plain` instead of serving the committed files; with gbrain the brain-aware render already runs and now carries the flag too. The activation record (`~/.gstack/render/claude.overlay`) gains `plan_mode=off`. Turning the key back on removes that render on the next setup when gbrain is absent or its detector fails (a failed detector otherwise keeps a render), and re-renders the brain-aware render with the sections when gbrain is present. A failed render with the key off keeps the previous render. Records written with the key on are unchanged.
|
||||
|
||||
### Fixed
|
||||
- **`gstack-relink` refreshes the `_gstack-command` router alias after it re-renders the router.** It used to copy the router before the re-render, so the alias kept the previous router (with `disabled_skills`, the full skill list) until the next relink.
|
||||
|
||||
### Changed
|
||||
- **Gate integrity:** new tests `test/plan-mode-off-build.test.ts` (from #3213, unchanged), `test/plan-mode-off-generation.test.ts`, `test/plan-mode-preamble-config.test.ts`, `test/plan-mode-preamble-render.test.ts` and `test/plan-mode-preamble-setup.test.ts`. No existing test was edited, and no threshold, trial count, case kind or judge changed.
|
||||
|
||||
@@ -476,6 +476,7 @@ if { [ "$DISABLED" != " " ] || grep -q '^Disabled on this install' "$_ROUTER" 2>
|
||||
if [ -n "$_ROUTER_TMP" ] && command -v bun >/dev/null 2>&1 \
|
||||
&& (cd "$_GEN_SRC" && bun run scripts/gen-skill-docs.ts --host claude --out-dir "$_ROUTER_TMP" ${_DISABLED_CSV:+"--disabled-skills=$_DISABLED_CSV"} ${_PLAN_MODE_ARG:+"$_PLAN_MODE_ARG"} >/dev/null 2>&1) \
|
||||
&& [ -s "$_ROUTER_TMP/SKILL.md" ] && cp "$_ROUTER_TMP/SKILL.md" "$_ROUTER.tmp.$$" && mv "$_ROUTER.tmp.$$" "$_ROUTER"; then
|
||||
_link_root_skill_alias
|
||||
[ -n "$_DISABLED_CSV" ] && echo "Router: disabled skills left out of $_ROUTER"
|
||||
else
|
||||
rm -f "$_ROUTER.tmp.$$"
|
||||
|
||||
@@ -2085,8 +2085,10 @@ if [ "$NEEDS_AGENTS_GEN" -eq 1 ]; then
|
||||
_prune_stale_generated "$SOURCE_GSTACK_DIR" "$AGENTS_DIR" "$CODEX_SKILLS"
|
||||
fi
|
||||
|
||||
# 1c-1g: a build (NEEDS_BUILD=1) already rendered these hosts, but without
|
||||
# --plan-mode=off, so plan_mode_preamble=off renders them again.
|
||||
# 1c. Generate .factory/ Factory Droid skill docs
|
||||
if [ "$INSTALL_FACTORY" -eq 1 ] && [ "$NEEDS_BUILD" -eq 0 ]; then
|
||||
if [ "$INSTALL_FACTORY" -eq 1 ] && { [ "$NEEDS_BUILD" -eq 0 ] || [ -n "$_PLAN_MODE_ARG" ]; }; then
|
||||
log "Generating .factory/ skill docs..."
|
||||
(
|
||||
cd "$SOURCE_GSTACK_DIR"
|
||||
@@ -2096,7 +2098,7 @@ if [ "$INSTALL_FACTORY" -eq 1 ] && [ "$NEEDS_BUILD" -eq 0 ]; then
|
||||
fi
|
||||
|
||||
# 1d. Generate .opencode/ OpenCode skill docs
|
||||
if [ "$INSTALL_OPENCODE" -eq 1 ] && [ "$NEEDS_BUILD" -eq 0 ]; then
|
||||
if [ "$INSTALL_OPENCODE" -eq 1 ] && { [ "$NEEDS_BUILD" -eq 0 ] || [ -n "$_PLAN_MODE_ARG" ]; }; then
|
||||
log "Generating .opencode/ skill docs..."
|
||||
(
|
||||
cd "$SOURCE_GSTACK_DIR"
|
||||
@@ -2106,7 +2108,7 @@ if [ "$INSTALL_OPENCODE" -eq 1 ] && [ "$NEEDS_BUILD" -eq 0 ]; then
|
||||
fi
|
||||
|
||||
# 1e. Generate .cursor/ Cursor skill docs
|
||||
if [ "$INSTALL_CURSOR" -eq 1 ] && [ "$NEEDS_BUILD" -eq 0 ]; then
|
||||
if [ "$INSTALL_CURSOR" -eq 1 ] && { [ "$NEEDS_BUILD" -eq 0 ] || [ -n "$_PLAN_MODE_ARG" ]; }; then
|
||||
log "Generating .cursor/ skill docs..."
|
||||
(
|
||||
cd "$SOURCE_GSTACK_DIR"
|
||||
@@ -2116,7 +2118,7 @@ if [ "$INSTALL_CURSOR" -eq 1 ] && [ "$NEEDS_BUILD" -eq 0 ]; then
|
||||
fi
|
||||
|
||||
# 1f. Generate .copilot/ GitHub Copilot skill docs
|
||||
if [ "$INSTALL_COPILOT" -eq 1 ] && [ "$NEEDS_BUILD" -eq 0 ]; then
|
||||
if [ "$INSTALL_COPILOT" -eq 1 ] && { [ "$NEEDS_BUILD" -eq 0 ] || [ -n "$_PLAN_MODE_ARG" ]; }; then
|
||||
log "Generating .copilot/ skill docs..."
|
||||
(
|
||||
cd "$SOURCE_GSTACK_DIR"
|
||||
@@ -2126,7 +2128,7 @@ if [ "$INSTALL_COPILOT" -eq 1 ] && [ "$NEEDS_BUILD" -eq 0 ]; then
|
||||
fi
|
||||
|
||||
# 1g. Generate .agy/ Antigravity CLI skill docs
|
||||
if [ "$INSTALL_AGY" -eq 1 ] && [ "$NEEDS_BUILD" -eq 0 ]; then
|
||||
if [ "$INSTALL_AGY" -eq 1 ] && { [ "$NEEDS_BUILD" -eq 0 ] || [ -n "$_PLAN_MODE_ARG" ]; }; then
|
||||
log "Generating .agy/ skill docs..."
|
||||
(
|
||||
cd "$SOURCE_GSTACK_DIR"
|
||||
@@ -4211,7 +4213,7 @@ if [ -x "$DETECT_BIN" ]; then
|
||||
# A render from a previous gbrain install would shadow canonical files
|
||||
# on the next link/relink — drop it and restore canonical links. A
|
||||
# pinned overlay re-renders it without brain blocks instead (below).
|
||||
if [ -d "$_GSTACK_RENDER_DIR" ] && [ -z "${GSTACK_SKIP_GBRAIN_REGEN:-}" ] && [ "$INSTALL_CLAUDE" -eq 1 ] && [ "$_GSTACK_OVERLAY" = claude ]; then
|
||||
if [ -d "$_GSTACK_RENDER_DIR" ] && [ -z "${GSTACK_SKIP_GBRAIN_REGEN:-}" ] && [ "$INSTALL_CLAUDE" -eq 1 ] && [ "$_GSTACK_OVERLAY" = claude ] && [ -z "$_PLAN_MODE_ARG" ]; then
|
||||
rm -rf "$_GSTACK_RENDER_DIR"
|
||||
gstack_claude_render_settle "$SOURCE_GSTACK_DIR" "$_GSTACK_RENDER_DIR" absent removed
|
||||
if [ "${_CLAUDE_SKILLS_LINKED:-0}" -eq 1 ]; then
|
||||
|
||||
@@ -45,6 +45,83 @@ describe('every installed-skill render carries the plan-mode arg', () => {
|
||||
expect(fs.readFileSync(path.join(ROOT, 'bin/gstack-config'), 'utf8'))
|
||||
.toContain('[ "$("$0" get plan_mode_preamble 2>/dev/null || true)" != off ] || _PLAN_MODE_ARG="--plan-mode=off"');
|
||||
});
|
||||
|
||||
test('setup: a build (which renders without the arg) does not skip the host re-renders when the key is off', () => {
|
||||
const setup = fs.readFileSync(path.join(ROOT, 'setup'), 'utf8');
|
||||
const guards = setup.split('\n').filter(line => /^if \[ "\$INSTALL_[A-Z]+" -eq 1 \] && .*NEEDS_BUILD/.test(line));
|
||||
expect(guards.length).toBe(5);
|
||||
for (const line of guards) expect(line, line).toContain('{ [ "$NEEDS_BUILD" -eq 0 ] || [ -n "$_PLAN_MODE_ARG" ]; }');
|
||||
});
|
||||
|
||||
test('setup: with the key off the gbrain-absent branch leaves the render for the plain re-render (a failed render keeps it)', () => {
|
||||
const setup = fs.readFileSync(path.join(ROOT, 'setup'), 'utf8');
|
||||
const removal = setup.split('\n').filter(line => /if \[ -d "\$_GSTACK_RENDER_DIR" \].*\$_GSTACK_OVERLAY" = claude/.test(line));
|
||||
expect(removal.length).toBe(1);
|
||||
expect(removal[0]).toContain('[ -z "$_PLAN_MODE_ARG" ]');
|
||||
});
|
||||
});
|
||||
|
||||
describe('gstack-relink router re-render', () => {
|
||||
const BINS = ['gstack-config', 'gstack-relink', 'gstack-patch-names', 'gstack-state-root.sh', 'gstack-install-registry.sh'];
|
||||
const SAFE = '## Plan Mode Safe Operations';
|
||||
|
||||
function relinkFixture() {
|
||||
const install = path.join(tmp, 'gstack');
|
||||
fs.mkdirSync(path.join(install, 'bin'), { recursive: true });
|
||||
for (const b of BINS) {
|
||||
fs.copyFileSync(path.join(ROOT, 'bin', b), path.join(install, 'bin', b));
|
||||
fs.chmodSync(path.join(install, 'bin', b), 0o755);
|
||||
}
|
||||
fs.symlinkSync(path.join(ROOT, 'scripts'), path.join(install, 'scripts'));
|
||||
fs.copyFileSync(path.join(ROOT, 'SKILL.md'), path.join(install, 'SKILL.md'));
|
||||
fs.mkdirSync(path.join(install, 'ship'));
|
||||
fs.writeFileSync(path.join(install, 'ship', 'SKILL.md'), '---\nname: ship\ndescription: test\n---\n# ship\n');
|
||||
const env = {
|
||||
...process.env,
|
||||
GSTACK_HOME: path.join(tmp, 'state'),
|
||||
GSTACK_STATE_ROOT: '',
|
||||
GSTACK_STATE_DIR: '',
|
||||
GSTACK_INSTALL_DIR: install,
|
||||
GSTACK_SKILLS_DIR: path.join(tmp, 'skills'),
|
||||
GSTACK_SETUP_RUNNING: '1',
|
||||
};
|
||||
const run = (bin: string, ...args: string[]) => {
|
||||
const r = spawnSync('bash', [path.join(install, 'bin', bin), ...args], { encoding: 'utf8', timeout: 60_000, env });
|
||||
expect(r.status, r.stderr).toBe(0);
|
||||
return r;
|
||||
};
|
||||
const router = () => fs.readFileSync(path.join(install, 'SKILL.md'), 'utf8');
|
||||
const alias = () => fs.readFileSync(path.join(tmp, 'skills', '_gstack-command', 'SKILL.md'), 'utf8');
|
||||
const asAlias = (text: string) => text.replace(/^name: gstack$/m, 'name: _gstack-command');
|
||||
return { run, router, alias, asAlias };
|
||||
}
|
||||
|
||||
test('off strips the router and its _gstack-command alias; on restores both to the committed bytes', () => {
|
||||
const { run, router, alias, asAlias } = relinkFixture();
|
||||
const committed = fs.readFileSync(path.join(ROOT, 'SKILL.md'), 'utf8');
|
||||
run('gstack-relink');
|
||||
expect(router()).toBe(committed);
|
||||
expect(alias()).toBe(asAlias(committed));
|
||||
|
||||
run('gstack-config', 'set', 'plan_mode_preamble', 'off');
|
||||
run('gstack-relink');
|
||||
expect(router()).not.toContain(SAFE);
|
||||
expect(router()).toContain('PLAN MODE EXCEPTION');
|
||||
expect(alias()).toBe(asAlias(router()));
|
||||
|
||||
run('gstack-config', 'set', 'plan_mode_preamble', 'on');
|
||||
run('gstack-relink');
|
||||
expect(router()).toBe(committed);
|
||||
expect(alias()).toBe(asAlias(committed));
|
||||
});
|
||||
|
||||
test('disabled_skills: the alias follows the re-rendered router in the same run', () => {
|
||||
const { run, router, alias, asAlias } = relinkFixture();
|
||||
run('gstack-config', 'set', 'disabled_skills', 'ship');
|
||||
run('gstack-relink');
|
||||
expect(router()).toContain('Disabled on this install (never invoke or suggest them): `/ship`.');
|
||||
expect(alias()).toBe(asAlias(router()));
|
||||
});
|
||||
});
|
||||
|
||||
let tmp: string;
|
||||
|
||||
@@ -12,17 +12,21 @@
|
||||
import { afterAll, afterEach, describe, expect, test } from 'bun:test';
|
||||
import { existsSync, readFileSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { config, defaultRender, fakeDetect, record, served, setup } from './helpers/claude-overlay-fixture';
|
||||
import { config, defaultRender, failableBun, fakeDetect, record, served, setup } from './helpers/claude-overlay-fixture';
|
||||
import { rmSync } from 'node:fs';
|
||||
import { ROOT, cleanupFixtures, cleanupSeed, makeFixture, makeSource, put } from './helpers/install-fixture';
|
||||
|
||||
afterEach(cleanupFixtures);
|
||||
afterAll(cleanupSeed);
|
||||
|
||||
const SAFE = '## Plan Mode Safe Operations';
|
||||
const alias = (skills: string) => readFileSync(join(skills, '_gstack-command/SKILL.md'), 'utf8');
|
||||
const asAlias = (router: string) => router.replace(/^name: gstack$/m, 'name: _gstack-command');
|
||||
|
||||
describe.skipIf(process.platform === 'win32')('./setup with plan_mode_preamble', () => {
|
||||
test('off renders the default install without the preamble; on (detector failing) serves the committed files again', () => {
|
||||
test('off renders the default install without the preamble and keeps it through a failed render; on (detector failing) serves the committed files again', () => {
|
||||
const f = makeFixture();
|
||||
failableBun(f);
|
||||
const src = makeSource(f, join(f.home, '.claude/skills/gstack'));
|
||||
fakeDetect(src);
|
||||
const skills = join(f.home, '.claude/skills');
|
||||
@@ -39,6 +43,14 @@ describe.skipIf(process.platform === 'win32')('./setup with plan_mode_preamble',
|
||||
expect(served(skills)).not.toContain('## Skill Invocation During Plan Mode');
|
||||
expect(readFileSync(join(src, 'SKILL.md'), 'utf8')).not.toContain(SAFE);
|
||||
expect(off.out).not.toContain('Claude skill overlay:');
|
||||
expect(alias(skills)).toBe(asAlias(readFileSync(join(src, 'SKILL.md'), 'utf8')));
|
||||
|
||||
put(join(f.home, 'fail-render'), '1');
|
||||
const failed = setup(f, src);
|
||||
expect(failed.status, failed.out).toBe(0);
|
||||
expect(record(defaultRender(f))).toMatchObject({ plan_mode: 'off', render: 'user' });
|
||||
expect(served(skills)).not.toContain(SAFE);
|
||||
rmSync(join(f.home, 'fail-render'));
|
||||
|
||||
expect(config(f, src, 'set', 'plan_mode_preamble', 'on').status).toBe(0);
|
||||
put(join(f.home, 'gbrain-mode'), 'fail');
|
||||
@@ -50,5 +62,6 @@ describe.skipIf(process.platform === 'win32')('./setup with plan_mode_preamble',
|
||||
expect(served(skills)).toBe(readFileSync(join(src, 'autoplan/SKILL.md'), 'utf8'));
|
||||
expect(served(skills)).toBe(committedSkill);
|
||||
expect(readFileSync(join(src, 'SKILL.md'), 'utf8')).toBe(committedRouter);
|
||||
expect(alias(skills)).toBe(asAlias(committedRouter));
|
||||
}, 240_000);
|
||||
});
|
||||
Reference in new issue
Block a user