mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-01 14:34:42 +01:00
* feat(ship): review loop converges (frozen finding scope, 3-pass cap, NO FINDINGS first-class) and receipts carry across conflict-only merges
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* test(ship): keep the Step 9 first-entry and Fix-First phrasing the skip/requeue contract asserts; fill the review record's pass, review_scope and range_base placeholders in the caller template test; narrow a test type
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* fix: pre-landing review fixes (pass 1) and the review-loop budget wording
Two blocking findings from /ship's own pass 1, both reproduced in the QA smoke before the fix:
gstack-ship-receipt carry hashed the product diff with git patch-id --stable, which strips
whitespace, so a space moved in a reviewed shell line carried as "no product change"; and it
classed package.json and every lockfile as release metadata, so a dependency swap carried too.
The patch-id is now --verbatim, package.json and VERSION are metadata only while the change is a
version stamp (lib/pregate/stamp-only.ts), lockfiles are product unless the policy names them,
the test globs name code extensions, a dirty working tree refuses before anything is compared,
and a receipt carried twice keeps the reviewed head as carry.from.
gstack-review-scope: an unresolvable base is POLICY_SOURCE_UNAVAILABLE instead of a silent
working-tree read; --range-base is validated; git runs with core.quotePath=false; range_base is
null on pass 1; a blocking finding with no path is fix; category "testing" is a test finding
below the floor; defer searches once, files nothing on a failed search and exits 0 unfiled;
summary refuses zero matching passes and the receipt write is chained to it with &&; a finding
the diff causes elsewhere names the changed line in `cause` and classify follows it.
Review-loop budget wording (ideas from tstack's sol-loop and pstack's interrogate, rewritten;
NOTICE.md): the blocker / should / nit glossary and two stop rules in Step 9.4, the checklist
and docs/review-policy.md; a lead-judgment stage in the finding merge both /review and /ship
render. The core read diffs $RANGE_BASE from pass 2 on. Caps and pins re-measured with reasons.
Step 7 coverage audit: four tests added in existing files, the --review-loop case moved to its
own test (the shared test tripped bun's 5 s default).
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* fix: pin the renumbered review-army stages and point the wording test at product code
CI at c3e27fc failed three free-tier tests, all this branch's own: the
workflow-required-steps fixture and shared-libs-rendering still pinned the
pre-Lead-judgment stage numbers, and test-of-test-ratchet counted
review-loop-budget-wording.test.ts as test-only code because it read only
generated markdown. The fixture and rendering test now pin stages 1-8, and the
wording test renders both armies through generateReviewArmy and reads the
authored ship template as well as the generated sections.
Pass-2 maintainability notes folded in: docs/troubleshooting.md names the two
new RECEIPT_CARRY_REFUSED causes (dirty tree, unreadable status) and the two new
REVIEW_SCOPE_INPUT causes (non-hex --range-base, summary with no matching
pass); gstack-review-scope's help states that classify reads the base branch's
policy and refuses an unresolvable base unless --policy is passed.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* fix: date a review-log row from its binding start when it has no timestamp
The Step 9.4 template asks the writer to fill `timestamp`; the logger stamps
`review_binding.started_at` but never `timestamp`. `gstack-review-scope
summary` dropped such a row silently, so this PR's own pass 2 was missing from
REVIEW_LOOP (passes=1 for a two-pass loop). A row now dates from `timestamp`,
else from its binding start; a row with neither is still not counted.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* fix: pass-4 review fixes (carry stamp rule, deleted-file locus, scanned defer, diff prefixes, row time)
Five findings at the blocking floor from the Step 11 outside voice (gpt-6.1-sol)
and the native adversarial pass, fixed as a fourth pass approved by GStack
Minder under the three-pass policy:
- carry: a path the policy names under stamp_files, stamp_paths or
release_outputs is metadata only while its change is a stamp (versionStampOnly
for a manifest kind, stampOnlyText otherwise), the rule the tree receipt
applies; a logic change in a stamped source file is product. Every
metadata-listed path is read at both heads and a non-stamp change drops its
globs from the product exclusion.
- review-scope: parseHunks keeps a deleted file as a whole changed location, so
a cause anchored in a removed export is the diff's to fix, not deferred.
- defer: issues are filed through lib/gstack-post's publication scan; a HIGH
finding in the title or body refuses and a MEDIUM one needs a human, both
recorded as unfiled with the reason; --repo names the destination.
- classify: both diffs pin --src-prefix=a/ --dst-prefix=b/ --no-textconv, so
diff.mnemonicPrefix cannot send every finding outside.
- summary: a row dates from review_binding.started_at before the writer-typed
timestamp, so a date-only value cannot drop a real pass.
Docs: ship-policy.md#receipt-carry, review-policy.md, the Step 9.4 template
and the CHANGELOG gate-integrity note record the rule and the pass.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* test(review-army): assert the Red Team branch the run took in the N+1 case
Under the blocking floor the Performance specialist rates a 12-line N+1
INFORMATIONAL, so the conditional Red Team does not activate; the case
encoded the old severity (N+1 is CRITICAL, a Red Team always follows). It
now asserts the branch the run took: a Red Team that ran must follow the
Performance specialist; no Red Team is correct only when the report's own
count says 0 critical and its Red Team line names that reason. The N+1
detection assertion is unchanged. The free contract test gains the
informational-not-activated and informational-claimed controls and the
budget test's synthetic report carries the count line.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* v1.91.256.0 stamp
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
---------
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
176 lines
11 KiB
TypeScript
176 lines
11 KiB
TypeScript
import { afterAll, beforeAll, describe, expect, test } from 'bun:test';
|
|
import { spawnSync } from 'node:child_process';
|
|
import { mkdtempSync, readFileSync, readdirSync, existsSync, rmSync } from 'node:fs';
|
|
import { tmpdir } from 'node:os';
|
|
import { join, resolve } from 'node:path';
|
|
import { ALL_HOST_CONFIGS } from '../hosts';
|
|
import { discoverTemplates } from '../scripts/discover-skills';
|
|
import { sharedLibsPlanExcerpt } from './helpers/shared-libs-plan-excerpt';
|
|
|
|
const ROOT = resolve(import.meta.dir, '..');
|
|
let output: string;
|
|
|
|
function rendered(host: (typeof ALL_HOST_CONFIGS)[number], skill: string): string {
|
|
const dir = host.name === 'claude' ? join(output, skill)
|
|
: join(output, host.hostSubdir, 'skills', `gstack-${skill}`);
|
|
const sections = join(dir, 'sections');
|
|
return readFileSync(join(dir, 'SKILL.md'), 'utf8') + (existsSync(sections)
|
|
? readdirSync(sections).filter(f => f.endsWith('.md')).sort()
|
|
.map(f => '\n' + readFileSync(join(sections, f), 'utf8')).join('') : '');
|
|
}
|
|
|
|
describe('shared-code skill distribution', () => {
|
|
beforeAll(() => {
|
|
output = mkdtempSync(join(tmpdir(), 'gstack-shared-libs-render-'));
|
|
const result = spawnSync('bun', ['run', 'scripts/gen-skill-docs.ts', '--host', 'all', '--out-dir', output],
|
|
{ cwd: ROOT, encoding: 'utf8', timeout: 120_000 });
|
|
expect(result.status, result.stderr).toBe(0);
|
|
}, 120_000);
|
|
afterAll(() => { if (output) rmSync(output, { recursive: true, force: true }); });
|
|
|
|
test('the new authored template is discovered without a hand-maintained installer list', () => {
|
|
expect(discoverTemplates(ROOT)).toContainEqual({
|
|
tmpl: 'deslop-shared-libs/SKILL.md.tmpl', output: 'deslop-shared-libs/SKILL.md',
|
|
});
|
|
expect(readFileSync(join(output, 'gstack/llms.txt'), 'utf8')).toContain('deslop-shared-libs');
|
|
expect(readFileSync(join(output, 'SKILL.md'), 'utf8')).toContain('invoke `/deslop-shared-libs`');
|
|
});
|
|
|
|
for (const host of ALL_HOST_CONFIGS) {
|
|
test(`${host.name}: all three workflows receive the same evaluation criteria`, () => {
|
|
const texts = ['deslop-shared-libs', 'plan-eng-review', 'review'].map(skill => rendered(host, skill));
|
|
for (const text of texts) {
|
|
expect(text.match(/### Shared-code evaluation rubric/g)?.length).toBe(1);
|
|
expect(text).toContain('at least two verified, first-party authored source');
|
|
expect(text).toContain('Count moved code on both sides');
|
|
expect(text).toContain('Only an engineering-plan review may use proposed callers');
|
|
expect(text).not.toContain('{{SHARED_LIBS_RUBRIC}}');
|
|
}
|
|
const standalone = texts[0];
|
|
expect(standalone).not.toContain('gstack-skill-start');
|
|
expect(standalone).not.toContain('gstack-skill-end');
|
|
expect(standalone).not.toContain('## Preamble');
|
|
expect(standalone).toContain('five metadata pages and fifty');
|
|
expect(standalone).toContain('including repeated page numbers');
|
|
expect(standalone).toContain('temporary files and files outside the repository');
|
|
expect(standalone).toContain('Keep API responses and intermediate data on stdout or in memory');
|
|
// Git safety is the installed helper from the trusted global runtime, not a retyped prefix.
|
|
const safeGit = `~/${host.globalRoot}/bin/gstack-safe-git`;
|
|
expect(standalone).toContain(`${safeGit} -C <repo> rev-parse --is-inside-work-tree`);
|
|
expect(standalone).toContain(`${safeGit} ls-files --cached --others --exclude-standard -z`);
|
|
expect(standalone).toContain('never bare `git`');
|
|
expect(standalone).not.toContain('git --no-pager');
|
|
expect(standalone).not.toContain('$GSTACK_ROOT');
|
|
expect(standalone).not.toContain('{{SAFE_GIT}}');
|
|
expect(standalone).toContain('python3 -I -S');
|
|
expect(standalone).toContain('two explicit committed object IDs');
|
|
expect(standalone).toContain('same Git tree');
|
|
expect(standalone).toContain('Do not invoke Git');
|
|
expect(standalone).toContain('transports, including `ls-remote`');
|
|
expect(standalone).toContain('including by decoding loose');
|
|
expect(texts[2]).toContain('snapshot_covered_paths');
|
|
expect(texts[2]).toContain('Exclude assume-unchanged, skip-worktree');
|
|
expect(texts[2]).toContain('byte-for-byte with its blob');
|
|
for (const [skill, parent] of [['review', texts[2]], ['ship', rendered(host, 'ship')]]) {
|
|
const validation = parent.indexOf(skill === 'ship' ? '1. **Validate severity.**' : '**Validate advisory severity first.**');
|
|
expect(validation).toBeGreaterThanOrEqual(0);
|
|
expect(validation).toBeLessThan(parent.indexOf(skill === 'ship' ? '2. **Read decisions.**' : 'Before classifying findings, check'));
|
|
if (skill === 'ship') {
|
|
const matching = parent.slice(validation).replace(/\s+/g, ' ');
|
|
expect(matching).toContain('For CRITICAL/advisory contradictions, remove `advisory`, never downgrade severity');
|
|
expect(matching).toContain('Valid INFORMATIONAL advisories stay advisory, including simplification');
|
|
expect(matching).toContain('Reject contradictory saved decisions');
|
|
expect(matching).toContain('they cannot suppress defects');
|
|
} else {
|
|
expect(parent).toContain('remove `advisory` and retain its `CRITICAL` severity');
|
|
expect(parent).toContain('Never downgrade severity to make advisory metadata consistent');
|
|
expect(parent).toContain('Valid INFORMATIONAL advisories remain advisory in every category, including simplification');
|
|
expect(parent).toContain('contradictory CRITICAL/advisory metadata cannot establish a skipped defect or advisory decision');
|
|
}
|
|
if (host.name !== 'codex' && !host.suppressedResolvers?.includes('REVIEW_ARMY')) {
|
|
const stages = ['#### 1. Parse outputs', '#### 2. Validate severity', '#### 3. Identify and merge',
|
|
'#### 4. Lead judgment', '#### 5. Apply specialist confidence gates', '#### 6. Score and present specialists'];
|
|
const positions = stages.map(stage => parent.indexOf(stage));
|
|
expect(positions.every(position => position >= 0)).toBe(true);
|
|
expect(positions).toEqual([...positions].sort((a, b) => a - b));
|
|
const merge = parent.slice(positions[1], positions[2]).replace(/\s+/g, ' ');
|
|
expect(merge).toContain('remove `advisory` and retain its `CRITICAL` severity');
|
|
expect(merge).toContain('Never downgrade severity');
|
|
expect(parent).toContain('Only specialist findings enter this header and `quality_score`; core findings do not');
|
|
} else {
|
|
expect(parent).not.toContain('#### 1. Parse outputs');
|
|
expect(parent).not.toContain('SPECIALIST REVIEW: N findings');
|
|
}
|
|
}
|
|
});
|
|
}
|
|
|
|
test('Codex keeps the core check when Review Army is suppressed', () => {
|
|
const codex = ALL_HOST_CONFIGS.find(host => host.name === 'codex')!;
|
|
const review = rendered(codex, 'review');
|
|
expect(review).not.toContain('### Dispatch specialists');
|
|
expect(review).toContain('### Shared-code evaluation rubric');
|
|
expect(review).toContain('sharedLibsFingerprint');
|
|
expect(review).toContain('advisory');
|
|
expect(review).toContain('ASK');
|
|
});
|
|
|
|
test('the bounded plan fixture includes the generated decision prerequisites before Code Quality', () => {
|
|
const entrypoint = readFileSync(join(output, 'plan-eng-review/SKILL.md'), 'utf8');
|
|
const review = readFileSync(join(output, 'plan-eng-review/sections/review-sections.md'), 'utf8');
|
|
const excerpt = sharedLibsPlanExcerpt(entrypoint, review);
|
|
const headings = ['## AskUserQuestion Format', '## My engineering preferences',
|
|
'## Review record and write policy', '**Plan-review evidence:**',
|
|
'## Confidence Calibration', '## Decision procedure',
|
|
'### Prepare an unanswered choice', '**Separate independent choices.**',
|
|
'**Compare one choice.**', '**Pending-record checkpoint.**',
|
|
'### Send once and wait', '### Record the answer',
|
|
'### 2. Code quality review', '### Shared-code evaluation rubric', '**Blocked outcome:**'];
|
|
const positions = headings.map(heading => excerpt.indexOf(heading));
|
|
expect(positions.every(index => index >= 0)).toBe(true);
|
|
expect(positions).toEqual([...positions].sort((a, b) => a - b));
|
|
expect(excerpt.match(/^## Decision procedure$/gm)).toHaveLength(1);
|
|
expect(excerpt).toContain('AskUserQuestion({ questions: [currentDecision] })');
|
|
expect(excerpt).toContain('D-numbering: exclude the initial target menu');
|
|
expect(excerpt).toContain('**Check each artifact and parent directory\'s permission before writing.**');
|
|
expect(excerpt).toContain('Check the Write/Edit result, then use Read to fetch the entire saved record.');
|
|
expect(excerpt).toContain('Check the save result, then Read the entire resolution block, including State.');
|
|
expect(excerpt).toContain('Only an engineering-plan review may use proposed callers');
|
|
expect(excerpt).not.toMatch(/^## (?:Scope Challenge|Outside Voice|Required outputs)/m);
|
|
expect(excerpt).not.toMatch(/^### [134]\. (?:Architecture|Test|Performance) review/m);
|
|
expect(excerpt).not.toContain('gstack-skill-start --');
|
|
|
|
// Exact source slices preserve the complete native brief and decision loop.
|
|
for (const [source, start, end] of [
|
|
[entrypoint, '## AskUserQuestion Format', '## Artifacts Sync'],
|
|
[review, '## Review record and write policy', '## Prior Learnings'],
|
|
[review, '## Decision procedure', '## Scope Challenge'],
|
|
[review, '### 2. Code quality review', '### 3. Test review'],
|
|
]) expect(excerpt).toContain(source.slice(source.indexOf(start), source.indexOf(end, source.indexOf(start))));
|
|
|
|
for (const [source, marker] of [[entrypoint, '## AskUserQuestion Format'],
|
|
[review, '## Review record and write policy'], [review, '## Decision procedure'],
|
|
[review, '## Scope Challenge']] as const) {
|
|
const damaged = source.replace(marker, '## Missing prerequisite');
|
|
expect(() => sharedLibsPlanExcerpt(source === entrypoint ? damaged : entrypoint,
|
|
source === review ? damaged : review)).toThrow(/marker not found/);
|
|
}
|
|
});
|
|
|
|
test('every rendered recommendation names a concrete risk, never "none"', () => {
|
|
// C3: shared-libs-opportunity-judgment reds 37193478719 t3 and 37195203538 t1 wrote
|
|
// "Main risk: none found inside the repo" / "none in behavior" for the top recommendation.
|
|
for (const host of ALL_HOST_CONFIGS) {
|
|
const text = rendered(host, 'deslop-shared-libs').replace(/\s+/g, ' ');
|
|
expect(text, host.name).toContain('name the main risk or uncertainty: a concrete behavior that could differ after migration, or the shared-failure blast radius, never "none".');
|
|
}
|
|
});
|
|
|
|
test('standalone catalog entry adds exactly the approved 82 bytes', () => {
|
|
const skill = readFileSync(join(output, 'deslop-shared-libs/SKILL.md'), 'utf8');
|
|
const description = skill.match(/^description: (.+)$/m)?.[1];
|
|
expect(description).toBe('Find worthwhile shared-code extractions in recent work. (gstack)');
|
|
expect(Buffer.byteLength('deslop-shared-libs') + Buffer.byteLength(description!)).toBe(82);
|
|
});
|
|
});
|