mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-14 08:59:01 +02:00
Branch-name-to-filename had incompatible rules across writer and readers: gstack-review-log WRITES <branch>-reviews.jsonl with the gstack-slug canonical form (tr '/' '-' then tr -cd 'a-zA-Z0-9._-', bin/gstack-slug:178), but Context Recovery PROBED it with raw $_BRANCH from git branch --show-current — so for any branch containing a '/' the REVIEWS line never fired (#1851's reader half of #1127). The probe now uses ${BRANCH:-unknown}, the canonical value the gstack-slug eval on the block's first line already sets. review.ts's plan content-search BRANCH gains the missing tr -cd half so it matches the same canonical pipeline. Full audit of the 5 raw $_BRANCH interpolation sites in scripts/resolvers/ (E3): generate-context-recovery.ts:16 (reviews.jsonl path) -> canonical BRANCH; :19/:21 (timeline.jsonl content greps) KEEP raw $_BRANCH because the timeline writer (preamble's gstack-timeline-log call) stores the raw branch in the "branch" field — slugging the reader would break that pairing; generate-preamble-bash.ts:29 (display echo) and :97 (timeline data write) keep raw by design. The *-$BRANCH-design-*.md family (review.ts:313 + 3 plan-review templates) is a consistent tr '/' '-' writer/reader pair and is deliberately untouched. test/branch-slug-hygiene.test.ts pins the discipline: a rendered-output sweep forbids raw $_BRANCH adjacent to a path separator or as a filename prefix in ANY generated SKILL.md/section, and a live round-trip on a feat/slash branch proves gstack-review-log's write is found by the rendered probe (with the raw-form shape as a negative control). Reader-side fix folded from PR #1851. Contributed by @harjothkhara. Fixes #2550 Fixes #1127 Co-authored-by: harjothkhara <harjothkhara@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
135 lines
5.8 KiB
TypeScript
135 lines
5.8 KiB
TypeScript
/**
|
|
* Branch-name slug hygiene in file-path positions (#2550, #1851/#1127).
|
|
*
|
|
* gstack-review-log WRITES `<canonical-branch>-reviews.jsonl` where the
|
|
* canonical form comes from bin/gstack-slug (tr '/' '-' then
|
|
* tr -cd 'a-zA-Z0-9._-'). Context Recovery used to PROBE the same file with
|
|
* raw $_BRANCH (`git branch --show-current`) — so for any branch containing
|
|
* a `/` (most feature branches) the REVIEWS line never fired. Same class:
|
|
* review.ts's plan content-search sanitized with tr '/' '-' only, missing
|
|
* the tr -cd half of the canonical pipeline.
|
|
*
|
|
* Discipline pinned here:
|
|
* - FILE-PATH positions interpolate the slug-canonical $BRANCH (set by the
|
|
* gstack-slug eval that opens Context Recovery).
|
|
* - Raw $_BRANCH stays for display (BRANCH: echo) and for timeline.jsonl
|
|
* content greps — the timeline writer stores the RAW branch, so slugging
|
|
* the reader would break that pairing.
|
|
*
|
|
* Reader-side fix folded from community PR #1851 by @harjothkhara.
|
|
*/
|
|
import { describe, test, expect } from 'bun:test';
|
|
import { execSync } from 'child_process';
|
|
import * as fs from 'fs';
|
|
import * as os from 'os';
|
|
import * as path from 'path';
|
|
import { HOST_PATHS } from '../scripts/resolvers/types';
|
|
import type { TemplateContext } from '../scripts/resolvers/types';
|
|
import { generateContextRecovery } from '../scripts/resolvers/preamble/generate-context-recovery';
|
|
|
|
const ROOT = path.join(import.meta.dir, '..');
|
|
|
|
// Raw $_BRANCH (either spelling) immediately before/after a path separator.
|
|
const PATH_ADJACENT = /\/\$\{?_BRANCH|\$\{_BRANCH\}\/|\$_BRANCH\//;
|
|
// Raw $_BRANCH as a filename prefix (…-reviews.jsonl and friends).
|
|
const FILENAME_PREFIX = /\$\{?_BRANCH\}?[A-Za-z0-9._-]*\.(?:jsonl|json|md|txt|log)/;
|
|
|
|
function renderedSkillFiles(): string[] {
|
|
const out = execSync(
|
|
`find "${ROOT}" -name 'SKILL.md' -not -path '*/node_modules/*' ; find "${ROOT}" -path '*/sections/*.md' -not -path '*/node_modules/*'`,
|
|
{ encoding: 'utf-8' },
|
|
);
|
|
return out.split('\n').filter(Boolean);
|
|
}
|
|
|
|
describe('branch slug hygiene (#2550, #1851)', () => {
|
|
test('no generated SKILL.md or section interpolates raw $_BRANCH in a path position', () => {
|
|
const offenders: string[] = [];
|
|
for (const file of renderedSkillFiles()) {
|
|
const content = fs.readFileSync(file, 'utf-8');
|
|
if (PATH_ADJACENT.test(content) || FILENAME_PREFIX.test(content)) {
|
|
offenders.push(path.relative(ROOT, file));
|
|
}
|
|
}
|
|
expect(offenders).toEqual([]);
|
|
});
|
|
|
|
test('Context Recovery probes reviews.jsonl with the slug-canonical $BRANCH', () => {
|
|
const ctx: TemplateContext = {
|
|
skillName: 'test-skill',
|
|
tmplPath: 'test.tmpl',
|
|
host: 'claude',
|
|
paths: HOST_PATHS.claude,
|
|
preambleTier: 2,
|
|
};
|
|
const out = generateContextRecovery(ctx);
|
|
expect(out).toContain('${BRANCH:-unknown}-reviews.jsonl');
|
|
expect(out).not.toContain('${_BRANCH}-reviews.jsonl');
|
|
// The gstack-slug eval that defines $BRANCH must render BEFORE the probe.
|
|
const evalIdx = out.indexOf('gstack-slug');
|
|
const probeIdx = out.indexOf('${BRANCH:-unknown}-reviews.jsonl');
|
|
expect(evalIdx).toBeGreaterThan(-1);
|
|
expect(evalIdx).toBeLessThan(probeIdx);
|
|
// Raw $_BRANCH stays for the timeline.jsonl content greps (writer stores raw).
|
|
expect(out).toContain('"branch\\":\\"${_BRANCH}');
|
|
});
|
|
|
|
test('plan content-search BRANCH uses the full gstack-slug canonical pipeline', () => {
|
|
const rendered = fs.readFileSync(
|
|
path.join(ROOT, 'ship', 'sections', 'plan-completion.md'),
|
|
'utf-8',
|
|
);
|
|
expect(rendered).toContain(
|
|
`BRANCH=$(git branch --show-current 2>/dev/null | tr '/' '-' | tr -cd 'a-zA-Z0-9._-')`,
|
|
);
|
|
});
|
|
|
|
test('live round-trip: gstack-review-log writes, Context Recovery probe finds it (slash branch)', () => {
|
|
const home = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-home-'));
|
|
const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-repo-'));
|
|
try {
|
|
const env = { ...process.env, GSTACK_HOME: home };
|
|
execSync(
|
|
'git init -q && git -c user.email=t@t -c user.name=t commit -q --allow-empty -m init && git checkout -q -b feat/slug-hygiene',
|
|
{ cwd: repo, encoding: 'utf-8' },
|
|
);
|
|
|
|
// Writer: the real gstack-review-log (canonicalizes via gstack-slug).
|
|
execSync(
|
|
`"${path.join(ROOT, 'bin', 'gstack-review-log')}" '{"skill":"ship","status":"ok"}'`,
|
|
{ cwd: repo, env, encoding: 'utf-8' },
|
|
);
|
|
|
|
// The slug-canonical filename must exist; the raw form must not.
|
|
const slugVars = execSync(`"${path.join(ROOT, 'bin', 'gstack-slug')}"`, {
|
|
cwd: repo, env, encoding: 'utf-8',
|
|
});
|
|
const slug = slugVars.match(/^SLUG=(.*)$/m)![1];
|
|
const branch = slugVars.match(/^BRANCH=(.*)$/m)![1];
|
|
expect(branch).toBe('feat-slug-hygiene');
|
|
const proj = path.join(home, 'projects', slug);
|
|
expect(fs.existsSync(path.join(proj, 'feat-slug-hygiene-reviews.jsonl'))).toBe(true);
|
|
|
|
// Reader: execute the rendered probe line with $BRANCH from gstack-slug.
|
|
const ctx: TemplateContext = {
|
|
skillName: 'test-skill', tmplPath: 'test.tmpl', host: 'claude',
|
|
paths: HOST_PATHS.claude, preambleTier: 2,
|
|
};
|
|
const probeLine = generateContextRecovery(ctx)
|
|
.split('\n')
|
|
.find((l) => l.includes('-reviews.jsonl'))!;
|
|
const script = `_PROJ="${proj}"\nBRANCH="${branch}"\n${probeLine.trim()}`;
|
|
const out = execSync(`bash -c '${script.replace(/'/g, `'\\''`)}'`, {
|
|
cwd: repo, encoding: 'utf-8',
|
|
});
|
|
expect(out).toContain('REVIEWS: 1 entries');
|
|
|
|
// Negative control: the raw-branch probe (the pre-fix shape) misses.
|
|
expect(fs.existsSync(path.join(proj, 'feat/slug-hygiene-reviews.jsonl'))).toBe(false);
|
|
} finally {
|
|
fs.rmSync(home, { recursive: true, force: true });
|
|
fs.rmSync(repo, { recursive: true, force: true });
|
|
}
|
|
});
|
|
});
|