From d69ba2c6fb8a3749ef44a9d1d2d529f56b9d4b20 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 3 Nov 2026 02:52:35 -0800 Subject: [PATCH] v1.94.13.0 fix(review): bare --base reads origin/ in review and ship bins; diff-scope matches auth as a word (GST-1036, GST-1065) (#3906) * fix(review): bare --base reads origin/ in review and ship bins; diff-scope matches auth as a word (GST-1036, GST-1065) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * changelog: fragment for GST-1036 and GST-1065 Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(diff-scope): a token that starts or ends with auth is an auth path; author* is not (review fix, GST-1065) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(diff-scope): authoriz/authenticat mid-word and a leading oauth are auth paths (granted fix round, GST-1065) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(review-policy): REVIEW_BASE line and SCOPE_AUTH word matching (GST-1036, GST-1065) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * diff-scope: fail-safe auth match; resolveBase never re-qualifies a remote-qualified base (GST-1065, GST-1036) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Revert the resolveBase remote-prefix guard (regressed alice/base stacked bases); fringe case stays an open note 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> --- CHANGELOG.md | 5 ++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- bin/gstack-banned-terms | 3 +- bin/gstack-design-detect.ts | 3 +- bin/gstack-diff-scope | 23 +++++- bin/gstack-docs-candidate | 3 +- bin/gstack-gate-diff | 6 +- bin/gstack-prose-lint | 3 +- bin/gstack-review-plan | 11 +-- docs/review-policy.md | 9 ++- lib/gate-diff/git.ts | 21 ++++++ lib/pr-shots.ts | 5 +- lib/review-collect.ts | 17 +++-- package.json | 2 +- test/diff-scope.test.ts | 22 ++++++ test/gstack-design-detect.test.ts | 28 ++++++++ test/stale-local-base.test.ts | 112 ++++++++++++++++++++++++++++++ 18 files changed, 253 insertions(+), 24 deletions(-) create mode 100644 test/stale-local-base.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e5b016d95..0d60dba31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## [1.94.13.0] - 2026-11-03 + +- **A bare `--base main` reads origin/main in every review and ship bin (GST-1036).** `gstack-review-plan` now prints `REVIEW_BASE: ` and names a lagging local base on stderr. `gstack-banned-terms`, `gstack-prose-lint --base`, `gstack-docs-candidate`, `gstack-design-detect.ts scan --changed` and `gstack-pr-shots --base` no longer merge against a stale local main and report lines the branch only inherited. They all use the one resolver gstack-gate-diff already used. +- **`gstack-diff-scope` stops treating authorship files as auth (GST-1065).** `author-spec.md`, `AUTHORS`, `authority.ts` and `coauthors.ts` no longer raise a docs diff to gate class. Every other name with auth in it still does, including `auth.ts`, `/auth/`, `oauth`, `nextauthmiddleware.ts` and camelCase names like `useAuth.ts`. + ## [1.94.12.0] - 2026-11-03 ### Fixed diff --git a/VERSION b/VERSION index 3d542cb66..d3b3b1444 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.94.12.0 +1.94.13.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index e01c398ef..256f21068 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.94.12.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.94.13.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-banned-terms b/bin/gstack-banned-terms index 8c5f5ec1c..f67d51e9f 100755 --- a/bin/gstack-banned-terms +++ b/bin/gstack-banned-terms @@ -16,6 +16,7 @@ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { BANNED_TERMS_PATH, addedLines, exampleBannedTermsJson, loadBannedTerms, renderBannedTerms, scanBannedTerms, validateBannedTerms, wholeFiles } from '../lib/banned-terms'; +import { baseArg } from '../lib/gate-diff/git'; import { EXIT, exitTableHelp } from '../lib/headless-artifacts'; import { resultLine } from '../lib/result-codes'; import { resolveBaseBranch } from '../lib/ship-policy'; @@ -75,7 +76,7 @@ function main(argv: string[]): number { let files; if (filesFlag) files = wholeFiles(repoRoot, filesFlag.split(',').map(s => s.trim()).filter(Boolean)); else { - const diffBase = flag(rest, '--diff-base') ?? (() => { const mb = spawnSync('git', ['merge-base', base, 'HEAD'], { cwd: repoRoot, encoding: 'utf8', timeout: 30_000 }); return mb.status === 0 ? mb.stdout.trim() : undefined; })(); + const diffBase = flag(rest, '--diff-base') ?? (() => { const mb = spawnSync('git', ['merge-base', baseArg('gstack-banned-terms', base, repoRoot), 'HEAD'], { cwd: repoRoot, encoding: 'utf8', timeout: 30_000 }); return mb.status === 0 ? mb.stdout.trim() : undefined; })(); if (!diffBase) { process.stderr.write(`gstack-banned-terms: no merge base with ${base}; pass --diff-base or --files \n`); return EXIT.usage; } const diff = spawnSync('git', ['diff', '--unified=0', '--no-color', diffBase], { cwd: repoRoot, encoding: 'utf8', timeout: 120_000, maxBuffer: 256 * 1024 * 1024 }); if (diff.status !== 0) { process.stderr.write(`gstack-banned-terms: git diff ${diffBase} failed\n`); return EXIT.fail; } diff --git a/bin/gstack-design-detect.ts b/bin/gstack-design-detect.ts index 22a6cd77a..b5ffd67cc 100755 --- a/bin/gstack-design-detect.ts +++ b/bin/gstack-design-detect.ts @@ -97,6 +97,7 @@ import { writeReceipt, writeOutcome } from '../lib/egress-receipt'; import { readConfigKey, resolveStateRoot } from '../lib/state-root'; import { DESIGN_SLOP_CATALOG, entryForImpeccableId } from '../lib/design-catalog'; import { isFrontendPath } from '../lib/frontend-scope'; +import { baseArg } from '../lib/gate-diff/git'; // ── Environment ────────────────────────────────────────────────────────────── @@ -821,7 +822,7 @@ function resolveTargets(args: ScanArgs, p: Probe): { targets: string[]; refusedB for (const rel of r.stdout.toString('utf-8').split('\0')) if (rel) files.add(rel); return true; }; - if (!runZ(['diff', '-z', '--name-only', '--diff-filter=ACMR', `${base}...HEAD`])) { + if (!runZ(['diff', '-z', '--name-only', '--diff-filter=ACMR', `${baseArg('gstack-design-detect', base, top)}...HEAD`])) { // A base that does not resolve must not read as "no frontend changes". refuse(base, 'git diff against this base failed (unknown ref or unfetched base)'); return { targets: out, refusedBase: true }; diff --git a/bin/gstack-diff-scope b/bin/gstack-diff-scope index 4f0fb7d04..8671aa7fd 100755 --- a/bin/gstack-diff-scope +++ b/bin/gstack-diff-scope @@ -128,6 +128,26 @@ _session_data() { return "$r" } +# An auth path holds a token, split the way lib/review-risk.ts tokens() splits +# paths (camelCase, then every non-alphanumeric run), that contains auth +# (GST-1065). It fails safe: every such token raises the class except the +# authorship words below, so author-spec.md, AUTHORS, authority.ts and +# coauthors.ts do not, while auth.ts, /auth/, nextauthmiddleware.ts, +# noauthrequired.ts, unauthenticated.ts and HTTPAuthorization.ts do. +_auth_path() { + local s="$1" t r=1 + while [[ "$s" =~ ^(.*[a-z0-9])([A-Z].*)$ ]]; do s="${BASH_REMATCH[1]} ${BASH_REMATCH[2]}"; done + shopt -s nocasematch + for t in ${s//[^a-zA-Z0-9]/ }; do + case "$t" in + author|authors|authored|authoring|authorship|authority|authorities|coauthor|coauthors) ;; + *auth*) r=0; break ;; + esac + done + shopt -u nocasematch + return "$r" +} + _print_flags() { echo "SCOPE_FRONTEND=$FRONTEND" echo "SCOPE_BACKEND=$BACKEND" @@ -293,9 +313,10 @@ for f in ${FILES_LIST[@]+"${FILES_LIST[@]}"}; do # named for sessions (recovered-sessions.jsonl, GST-911) holds rows, not # session handling, so it is not auth; session config still is. case "$f" in - *auth*|*jwt*|*oauth*|*permission*|*role*) m_auth=true ;; + *jwt*|*permission*|*role*) m_auth=true ;; *session*) _session_data "$f" || m_auth=true ;; esac + _auth_path "$f" && m_auth=true # Backend: code that isn't a frontend component/view file. Includes ESM/CJS # (.mjs/.cjs) and explicit-module TS (.mts/.cts) — #1810: these matched no diff --git a/bin/gstack-docs-candidate b/bin/gstack-docs-candidate index a7e04e6ed..a2a0a5fbc 100755 --- a/bin/gstack-docs-candidate +++ b/bin/gstack-docs-candidate @@ -38,6 +38,7 @@ import { spawnSync } from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; import { baseContainsHead } from '../lib/default-base'; +import { baseArg } from '../lib/gate-diff/git'; import { resultLine } from '../lib/result-codes'; import { parseReceiptDocs, type ReceiptDocs } from '../lib/ship-receipt-docs'; @@ -138,7 +139,7 @@ function main(argv: string[]): void { if (out === top || out.startsWith(top + path.sep)) throw new Error('--out must be outside the repository'); const mode = one(args, '--mode'); if (mode !== 'edit' && mode !== 'read-only') throw new Error('--mode must be edit or read-only'); - const base = git(['rev-parse', '--verify', one(args, '--base') + '^{commit}']).trim(); + const base = git(['rev-parse', '--verify', baseArg('gstack-docs-candidate', one(args, '--base'), process.cwd()) + '^{commit}']).trim(); const self = baseContainsHead(top, base); if (self) throw new Error(resultLine('snapshot', 'BASE_CONTAINS_HEAD', self)); const exceptionFile = flag(args, '--exception')[0]; diff --git a/bin/gstack-gate-diff b/bin/gstack-gate-diff index 6025ecaf9..ad92815d2 100755 --- a/bin/gstack-gate-diff +++ b/bin/gstack-gate-diff @@ -32,7 +32,7 @@ import * as path from 'node:path'; import * as os from 'node:os'; import { scanCandidate, parseGateIntegrityBlock, specDirectoryHoldsSpecs, type FileDiff, type CommitInfo } from '../lib/gate-diff/index'; import { - commitInfo, commitsInRange, defaultBase, diffCommit, diffWorkingTree, firstParent, headSha, mergeBase, noBaseFix, readClaudeMd, repoRoot, + baseNote, commitInfo, commitsInRange, defaultBase, diffCommit, diffWorkingTree, firstParent, headSha, mergeBase, noBaseFix, readClaudeMd, repoRoot, resolveBase, resolvesToCommit, treePaths, untrackedAsAdded, } from '../lib/gate-diff/git'; import { baseContainsHead, noDefaultBaseLine } from '../lib/default-base'; @@ -162,8 +162,8 @@ function main(argv: string[]): number { process.stdout.write(`GATE_ERROR=no_base ref=${baseRef} fix=${noBaseFix(baseRef, root)}\n`); return 2; } - if (base.local === 'fallback') console.error(`gstack-gate-diff: ${base.unrelatedRemote ? `${base.unrelatedRemote} shares no history with HEAD` : `no remote-tracking branch for ${baseRef}`}; base is the local ${baseRef}, which may lag its remote`); - else if (base.local) console.error(`gstack-gate-diff: ${{ unrelated: `local ${baseRef} shares no history with HEAD`, missing: `no local branch ${baseRef}`, behind: `local ${baseRef} is behind ${base.ref}`, ahead: `local ${baseRef} is ahead of ${base.ref}`, diverged: `local ${baseRef} has diverged from ${base.ref}` }[base.local]}; base is ${base.ref}`); + const note = baseNote(baseRef, base); + if (note) console.error(`gstack-gate-diff: ${note}`); const mb = mergeBase(base.sha, head, root); if (!mb) { process.stdout.write(`GATE_ERROR=no_base ref=${baseRef} fix=${noBaseFix(baseRef, root)}\n`); diff --git a/bin/gstack-prose-lint b/bin/gstack-prose-lint index e01b8080f..46356150b 100755 --- a/bin/gstack-prose-lint +++ b/bin/gstack-prose-lint @@ -18,6 +18,7 @@ */ import * as fs from 'node:fs'; import { EXIT, exitTableHelp } from '../lib/headless-artifacts'; +import { baseArg } from '../lib/gate-diff/git'; import { lintFiles, renderProseLint } from '../lib/prose-lint'; import { resultLine } from '../lib/result-codes'; @@ -54,7 +55,7 @@ function main(argv: string[]): number { if ([base, sections, flags.has('--unreleased') || undefined].filter(Boolean).length > 1) { process.stderr.write('gstack-prose-lint: --unreleased, --base and --sections are different selections; pass one\n'); return EXIT.usage; } for (const f of files) if (!fs.existsSync(f) || fs.statSync(f).isDirectory()) { process.stderr.write(`gstack-prose-lint: cannot read ${f}\n`); return EXIT.usage; } let result; - try { result = lintFiles(files, { unreleased: flags.has('--unreleased'), base, sections }); } + try { result = lintFiles(files, { unreleased: flags.has('--unreleased'), base: base && baseArg('gstack-prose-lint', base, process.cwd()), sections }); } catch (e: any) { process.stderr.write(`gstack-prose-lint: ${e.message}\n`); return EXIT.usage; } if (sections && flags.has('--gate') && result.empty.length) { process.stderr.write(`gstack-prose-lint: no "##
" heading from --sections in ${result.empty.join(', ')}; nothing would be gated\n`); diff --git a/bin/gstack-review-plan b/bin/gstack-review-plan index be34391f3..b74496475 100755 --- a/bin/gstack-review-plan +++ b/bin/gstack-review-plan @@ -28,6 +28,7 @@ import { locate, readRecord } from '../lib/approval-record'; import { EXIT, exitTableHelp } from '../lib/headless-artifacts'; import { resultLine } from '../lib/result-codes'; import { runScriptSync } from '../lib/script-invocation'; +import { baseNote } from '../lib/gate-diff/git'; import { collectCommit, collectLive } from '../lib/review-collect'; import { excludeLines, reviewExclude, type Exclusion } from '../lib/review-exclude'; import { isUnavailable } from '../lib/ship-policy'; @@ -126,8 +127,11 @@ function main(): number { for (const e of collected.errors) process.stderr.write(`gstack-review-plan: git could not list the diff (${e.input}: ${e.detail}); fix: ${e.repair}; no plan is safer than a plan for an empty diff\n`); return EXIT.fail; } + const note = collected.base && baseNote(base, collected.base); + if (note) process.stderr.write(`gstack-review-plan: ${note}\n`); + const ref = collected.base?.ref ?? base; const head = git(['rev-parse', 'HEAD'])?.trim() ?? 'unknown'; - const self = collected.mergeBase === head ? baseContainsHead(root, git(['rev-parse', '--verify', '-q', `origin/${base}^{commit}`]) ? `origin/${base}` : base) : null; + const self = collected.mergeBase === head ? baseContainsHead(root, ref) : null; if (self) { process.stderr.write(`${resultLine('gstack-review-plan', 'BASE_CONTAINS_HEAD', self)}; no plan is safer than a plan for a diff compared with itself\n`); return EXIT.fail; } const binDir = path.dirname(Bun.fileURLToPath(import.meta.url)); const scopeOf = (args: string[]): Record => { @@ -145,7 +149,6 @@ function main(): number { const gateTagsOf = (args: string[]): number | null => { const g = runGateDiff(args); return tagCount(g.stdout, g.status); }; const diffBase = collected.mergeBase ?? base; const scope = scopeOf([diffBase]); - const ref = git(['rev-parse', '--verify', '-q', `origin/${base}^{commit}`]) !== null ? `origin/${base}` : base; const baseSha = collected.registry.sha ?? git(['rev-parse', '--verify', '-q', `${ref}^{commit}`])?.trim() ?? ''; const policy = baseSha ? loadPolicyAt(root, baseSha, ref) : null; const declares = !!policy && !isUnavailable(policy) && policy.policy.review_exclude_paths.length > 0; @@ -193,11 +196,11 @@ function main(): number { const gateScan = { tagged: gateTagged, head }; if (has('--json')) { const { runtimeLines, stepDepth, depthReasons, ...rest } = plan; - process.stdout.write(JSON.stringify({ ...rest, targeted: { ...targeted, brief, targeted_patch_id: patch }, step_depth: stepDepth, runtime_lines: runtimeLines, depth_reasons: depthReasons, depth_floor: floor ?? null, gate_scan: gateScan, fix_range: fixRange ?? null, registry: { sha: registry.sha, outputs: registry.outputs }, per_path: perPath, diff_lines: diffLines, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly, consumed: collected.consumed, drawings: collected.drawings, + process.stdout.write(JSON.stringify({ ...rest, targeted: { ...targeted, brief, targeted_patch_id: patch }, step_depth: stepDepth, runtime_lines: runtimeLines, depth_reasons: depthReasons, depth_floor: floor ?? null, gate_scan: gateScan, fix_range: fixRange ?? null, registry: { sha: registry.sha, outputs: registry.outputs }, per_path: perPath, diff_lines: diffLines, base: collected.base, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly, consumed: collected.consumed, drawings: collected.drawings, ...(exclusion.state === 'head-only' ? { review_exclude: exclusion } : active ? { review_exclude: { state: 'active', ...active.record, specs: active.specs, keep: active.keep, kept_files: active.kept, content: active.content, references: active.references } } : {}) }) + '\n'); return EXIT.ok; } - const out = [`REVIEW_CLASS: ${plan.class} (${plan.reasons.slice(0, 3).join('; ') || 'no signals'})`, reviewDepthLine(plan), `GATE_SCAN: ${gateTagged === null ? 'unavailable' : `tagged=${gateTagged}`} head=${head}`, + const out = [`REVIEW_CLASS: ${plan.class} (${plan.reasons.slice(0, 3).join('; ') || 'no signals'})`, reviewDepthLine(plan), `GATE_SCAN: ${gateTagged === null ? 'unavailable' : `tagged=${gateTagged}`} head=${head}`, `REVIEW_BASE: ${ref} ${baseSha.slice(0, 12)}`, `TARGETED: classes=${classList(targeted.classes)} rules=${targeted.rules} brief=${brief} targeted_patch_id=${patch}${targeted.reasons.length ? ` reasons=${targeted.reasons.join('; ')}` : ''}`]; out.push(...excludeLines(exclusion, base)); if (fixRange) out.push(`FIX_RANGE: ${fixRange.runtime_lines === null ? `unread (${fixRange.error})` : `runtime_lines=${fixRange.runtime_lines}`} base=${fixRange.base}`); diff --git a/docs/review-policy.md b/docs/review-policy.md index b6e561207..bc1bee4fc 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -174,9 +174,12 @@ applies no edits is its exit too. `gstack-review-plan --base ` classifies the diff, picks a **depth** and prints the reviewers the next step dispatches: `REVIEW_CLASS:`, `REVIEW_DEPTH: runtime_lines= rules= reasons=<…> codes=<…>`, `GATE_SCAN: tagged=|unavailable -head=`, one `DEPTH_ERROR:` per input that failed closed, one `DISPATCH:` per reviewer +head=`, `REVIEW_BASE: ` (the base the diff was taken against: a bare +`--base main` reads `origin/main` when it shares history with HEAD, and a local `main` that lags +it is named on stderr), one `DEPTH_ERROR:` per input that failed closed, one `DISPATCH:` per reviewer with its effort and a shell-quoted `DIFF_CMD:` (plus `DIFF_EXCLUDE: none` when it -reads everything), one `SKIP:` per reviewer left out, and `--json` with the same fields. +reads everything), one `SKIP:` per reviewer left out, and `--json` with the same fields (the +base as `base`). A plan that fails closed to full exits 0; a diff git cannot list exits 1. **Classes** are unchanged. A mixed diff takes its most restrictive class: **docs** @@ -384,7 +387,7 @@ and matching is token-bounded: `costume` never reads as cost. |---|---| | money | a path segment naming cost, spend, budget, ledger, billing, price, fee or invoice; or a changed line with an identifier ending in `_usd` or `Usd`, any `spend_cap` spelling, or a call to a paid provider `lib/pricing.ts` prices | | gate | a hunk the gate scan tagged: a relaxed threshold, a skipped test, a loosened check | -| auth | `SCOPE_AUTH` from `gstack-diff-scope`, on a code path with an auth, session, jwt, permission or role token in its name; a data file named for sessions (`recovered-sessions.jsonl`) outside a config location does not set `SCOPE_AUTH` | +| auth | `SCOPE_AUTH` from `gstack-diff-scope`, on a code path with an auth, session, jwt, permission or role token in its name; a data file named for sessions (`recovered-sessions.jsonl`) outside a config location does not set `SCOPE_AUTH`, and `SCOPE_AUTH` fails safe: any name token containing auth sets it (`auth.ts`, `/auth/`, `oauth`, `useAuth.ts`, `nextauthmiddleware.ts`) except the authorship words `author`, `authors`, `authored`, `authoring`, `authorship`, `authority`, `authorities`, `coauthor` and `coauthors` | | secret | a secrets, credentials, keychain or token path, or a changed line that reads a key-named environment variable (`OPENAI_API_KEY`, `GH_TOKEN`) or a credential store | An input the classifier cannot read (the diff, `gstack-diff-scope`, the gate diff --git a/lib/gate-diff/git.ts b/lib/gate-diff/git.ts index adc46fbbf..daebe14d4 100644 --- a/lib/gate-diff/git.ts +++ b/lib/gate-diff/git.ts @@ -137,6 +137,27 @@ export function resolveBase(ref: string, cwd: string, head?: string): { ref: str return sha ? { ref, sha } : null; } +export type ResolvedBase = NonNullable>; + +/** Why resolveBase read another ref than the name given (`local main is behind origin/main; base is origin/main`); null when the name stood as given. */ +export function baseNote(name: string, base: ResolvedBase): string | null { + if (base.local === 'fallback') return `${base.unrelatedRemote ? `${base.unrelatedRemote} shares no history with HEAD` : `no remote-tracking branch for ${name}`}; base is the local ${name}, which may lag its remote`; + if (!base.local) return null; + return `${{ unrelated: `local ${name} shares no history with HEAD`, missing: `no local branch ${name}`, behind: `local ${name} is behind ${base.ref}`, ahead: `local ${name} is ahead of ${base.ref}`, diverged: `local ${name} has diverged from ${base.ref}` }[base.local]}; base is ${base.ref}`; +} + +/** + * A --base argument as the commit a review or ship bin reads (GST-1036): resolveBase against HEAD, so a bare `main` + * reads origin/main over a stale local main, with the note on stderr as `: ` when it read another ref than + * the name. An unresolvable name comes back as given, so the caller's own git error still names it. + */ +export function baseArg(bin: string, name: string, cwd: string): string { + const base = resolveBase(name, cwd, 'HEAD'); + const note = base && baseNote(name, base); + if (note) process.stderr.write(`${bin}: ${note}\n`); + return base?.sha ?? name; +} + /** A word for a printed shell line: as is when it is plain, single-quoted otherwise (ref names may hold `;`, `$`, `(`, `{1,2}`). */ function shellWord(s: string): string { return /^[A-Za-z0-9._\/@^~+:-]+$/.test(s) ? s : `'${s.replace(/'/g, "'\\''")}'`; diff --git a/lib/pr-shots.ts b/lib/pr-shots.ts index 821569dc8..0e8c4659a 100644 --- a/lib/pr-shots.ts +++ b/lib/pr-shots.ts @@ -17,6 +17,7 @@ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as path from 'node:path'; +import { resolveBase } from './gate-diff/git'; import { findSection } from './markdown-sections'; import { scan } from './redact-engine'; import type { ResultCodeName } from './result-codes'; @@ -55,9 +56,11 @@ export function readCapture(file: string, roots: string[], what: string): string return cleanText(fs.readFileSync(real), `${what} file`); } -/** Resolve `ref` to a commit sha without letting it be read as an option. */ +/** Resolve `ref` to a commit sha without letting it be read as an option; a bare branch name reads origin's tracking branch first (resolveBase, GST-1036). */ export function resolveCommit(cwd: string, ref: string): string | ShotError { if (!ref || ref.startsWith('-') || /\s/.test(ref)) return { code: 'PR_SHOTS_INPUT_REFUSED', message: `ref ${JSON.stringify(ref)} is not a plain ref` }; + const base = resolveBase(ref, cwd, 'HEAD'); + if (base) return base.sha; const r = git(cwd, ['rev-parse', '--verify', '--quiet', '--end-of-options', `${ref}^{commit}`]); return r.status === 0 ? r.stdout.trim() : { code: 'PR_SHOTS_INPUT_NOT_FOUND', message: `base ref ${ref}` }; } diff --git a/lib/review-collect.ts b/lib/review-collect.ts index ac16ba0d2..f5e10709f 100644 --- a/lib/review-collect.ts +++ b/lib/review-collect.ts @@ -10,6 +10,7 @@ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as path from 'node:path'; +import { resolveBase, type ResolvedBase } from './gate-diff/git'; import { GENERATED_PATH, parseRegistry } from './regen'; import { RESULT_CODES } from './result-codes'; import { classifyPath, dataCandidate, svgCandidate, svgDrawing, type ClassInput, type DepthError, type PathLines, type RegistryInfo } from './review-depth'; @@ -31,6 +32,8 @@ export interface Collected extends ClassInput { untracked: string[]; consumed: string[] | 'all'; drawings: string[]; + /** collectLive only: the ref resolveBase read for the base name (GST-1036), null when nothing resolved. */ + base?: ResolvedBase | null; } function git(root: string, args: string[]): { ok: boolean; status: number | null; out: string; err: string } { @@ -255,16 +258,20 @@ export function collectCommit(root: string, baseSha: string, headSha: string): C return result(diff, { root: skillDirs, registry, errors, mergeBase, untracked: [], consumed, drawings }); } -/** The working tree against the merge base with origin/ (or ; a remote-qualified base reads refs/remotes/ only), untracked files counted per path; the registry is the base ref's commit. */ +/** + * The working tree against the merge base with the base resolveBase reads (lib/gate-diff/git.ts: a bare name is + * origin/ when it shares history with HEAD, else the local branch; a remote-qualified base reads + * refs/remotes/ only), untracked files counted per path; the registry is the base ref's commit. + */ export function collectLive(root: string, base: string): Collected { const errors: DepthError[] = []; - const qualified = base.startsWith('origin/') || git(root, ['remote']).out.split('\n').some(r => r && base.startsWith(`${r}/`)); - const ref = (qualified ? [`refs/remotes/${base}`] : [`refs/remotes/origin/${base}`, `refs/heads/${base}`]).find(r => git(root, ['rev-parse', '--verify', '-q', `${r}^{commit}`]).ok) ?? base; + const resolved = resolveBase(base, root, 'HEAD'); + const ref = resolved?.sha ?? base; const registry = loadRegistryAt(root, ref); const mb = git(root, ['merge-base', ref, 'HEAD']); if (!mb.ok) { errors.push({ code: 'numstat_failed', detail: oneLine(mb.err || `no merge base with ${ref}`), input: `git merge-base ${ref} HEAD`, repair: `fetch the base first (git fetch origin ${base})` }); - return result({ perPath: [], deleted: [], stampOnly: [], created: [], added: {}, removed: {} }, { root, registry, errors, mergeBase: null, untracked: [], consumed: 'all' }); + return { ...result({ perPath: [], deleted: [], stampOnly: [], created: [], added: {}, removed: {} }, { root, registry, errors, mergeBase: null, untracked: [], consumed: 'all' }), base: resolved }; } const mergeBase = mb.out.trim(); const diff = readDiff(root, [mergeBase], errors); @@ -284,5 +291,5 @@ export function collectLive(root: string, base: string): Collected { const drawings = drawingsAmong(diff.perPath, root, p => { try { return fs.statSync(path.join(root, p)).size <= DRAWING_READ_CAP ? fs.readFileSync(path.join(root, p)) : null; } catch { return null; } }); - return result(diff, { root, registry, errors, mergeBase, untracked, consumed, drawings }); + return { ...result(diff, { root, registry, errors, mergeBase, untracked, consumed, drawings }), base: resolved }; } diff --git a/package.json b/package.json index e8716b2f6..83c7ac316 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.94.12", + "version": "1.94.13", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/test/diff-scope.test.ts b/test/diff-scope.test.ts index 882b04af7..8d1e12650 100644 --- a/test/diff-scope.test.ts +++ b/test/diff-scope.test.ts @@ -223,6 +223,28 @@ describe('gstack-diff-scope', () => { expect(scope.SCOPE_AUTH).toBe('true'); }); + // Value: protects=auth matches as a word or path segment, so docs about authors stay out of the gate class; + // fails_when=the auth match is a bare substring again (author-spec.md, AUTHORS, authority.ts raise SCOPE_AUTH); + // why_new=GST-1065 (author-spec.md raised a docs bundle to gate class with four full readers); seam=bin/gstack-diff-scope + for (const file of ['docs/author-spec.md', 'AUTHORS', 'lib/authority.ts', 'docs/authored-by.md', 'src/coauthors.ts', 'AUTHORS.md']) { + test(`${file} is not an auth path`, () => { + expect(runScope(createRepo([file])).SCOPE_AUTH).toBe('false'); + }); + } + + // Value: protects=real auth paths still raise SCOPE_AUTH after the word-boundary fix (negative controls for GST-1065); + // fails_when=the boundary drops a segment, a separator, a camelCase name, an authn/authz/oauth spelling or a glued token (auth0, basicauth, nextauth); + // why_new=GST-1065 and its review (the first word-only matcher dropped glued tokens the old glob caught); seam=bin/gstack-diff-scope + for (const file of ['src/auth.ts', 'app/auth/login.rb', 'lib/oauth.ts', 'src/oauth2/token.ts', 'lib/authn/check.go', 'src/authz.py', 'src/authService.ts', + 'src/useAuth.ts', 'lib/OAuthClient.ts', 'lib/authentication.rb', 'lib/authorization_policy.rb', 'src/Authorize.tsx', 'config/AUTH_KEYS.md', 'pkg/auth-middleware.go', + 'lib/auth0.ts', 'src/basicauth.go', 'src/authguard.ts', 'lib/reauth.ts', 'pages/api/nextauth.ts', 'src/authtoken.ts', 'lib/preauth.rb', 'src/HTTPAuth.ts', + 'src/unauthenticated.ts', 'lib/reauthentication.ts', 'src/deauthorize.ts', 'app/oauth2client.py', 'api/HTTPAuthorization.ts', + 'src/nextauthmiddleware.ts', 'src/reauthn.ts', 'src/noauthrequired.ts']) { + test(`${file} is an auth path`, () => { + expect(runScope(createRepo([file])).SCOPE_AUTH).toBe('true'); + }); + } + test('detects config via bun.lock (Bun v1.2+ text lockfile)', () => { const dir = createRepo(['bun.lock']); const scope = runScope(dir); diff --git a/test/gstack-design-detect.test.ts b/test/gstack-design-detect.test.ts index 39388ac41..84a4e4e62 100644 --- a/test/gstack-design-detect.test.ts +++ b/test/gstack-design-detect.test.ts @@ -935,6 +935,34 @@ describe('scan', () => { expect(r.err).toContain(`${SENTINEL.DETECT_REFUSED}: main (not a repository)`); expect(r.code).toBe(1); // a base that cannot be diffed is a failed target, never a silent clean scan }); + + test.skipIf(!POSIX)('--changed main reads origin/main over a stale local main (GST-1036)', () => { + // Value: protects=design review scans only the branch's frontend files; fails_when=a bare --changed base diffs against the stale local main + // and scans a stylesheet only upstream changed; why_new=GST-1036 (ship review-army passes a bare ); seam=bin/gstack-design-detect.ts + const bare = path.join(SANDBOX, 'stale-origin.git'); + const work = path.join(SANDBOX, 'stale-work'); + const other = path.join(SANDBOX, 'stale-other'); + const log = path.join(SANDBOX, 'argv-stale.log'); + try { + git(SANDBOX, 'clone', '-q', '--bare', REPO, bare); + git(SANDBOX, 'clone', '-q', bare, work); + git(SANDBOX, 'clone', '-q', bare, other); + for (const d of [work, other]) { git(d, 'config', 'user.email', 't@example.com'); git(d, 'config', 'user.name', 't'); } + fs.writeFileSync(path.join(other, 'src', 'styles.css'), '.hero { color: blue }\n'); + git(other, 'commit', '-qam', 'upstream'); + git(other, 'push', '-q', 'origin', 'HEAD:main'); + git(work, 'fetch', '-q', 'origin'); + git(work, 'checkout', '-q', '-b', 'feature', 'origin/main'); + fs.writeFileSync(path.join(work, 'src', 'components', 'Card.tsx'), 'export const Card = () =>

;\n'); + git(work, 'commit', '-qam', 'change'); + const r = run(['scan', '--changed', 'main', '--format', 'gstack'], { env: { IMPECCABLE_BIN: FAKE, IMPECCABLE_FAKE_LOG: log }, cwd: work }); + const argv = JSON.parse(fs.readFileSync(log, 'utf-8').trim().split('\n')[0]).argv as string[]; + expect(argv.slice(2).map(a => path.relative(fs.realpathSync(work), a)).sort()).toEqual(['src/components/Card.tsx']); + expect(r.err).toContain('gstack-design-detect: local main is behind origin/main; base is origin/main'); + } finally { + for (const p of [bare, work, other, log]) fs.rmSync(p, { recursive: true, force: true }); + } + }); }); describe('design-review REPORT_DIR agrees with the allow-list', () => { diff --git a/test/stale-local-base.test.ts b/test/stale-local-base.test.ts new file mode 100644 index 000000000..5c2c749d6 --- /dev/null +++ b/test/stale-local-base.test.ts @@ -0,0 +1,112 @@ +/** + * A bare --base reads origin's tracking branch, not a stale local branch (GST-1036). Capy clones and linked + * worktrees never move the local base branch, so `--base main` there merged against an old local main: review and ship + * bins then counted upstream commits the branch only inherited (a docs diff planned at gate depth, banned terms and + * prose findings in lines the branch never wrote). Every such bin resolves the base through resolveBase + * (lib/gate-diff/git.ts) and names the ref it used. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { BANNED_TERMS_SCHEMA } from '../lib/banned-terms'; +import { resolveCommit } from '../lib/pr-shots'; + +const BIN = path.join(import.meta.dir, '..', 'bin'); +const dirs: string[] = []; +afterEach(() => { while (dirs.length) fs.rmSync(dirs.pop()!, { recursive: true, force: true }); }); + +function fixture() { + const root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'stale-base-'))); + dirs.push(root); + const work = path.join(root, 'work'); + const env = { ...process.env, GSTACK_STATE_ROOT: path.join(root, 'state'), GIT_AUTHOR_NAME: 't', GIT_AUTHOR_EMAIL: 't@t', GIT_COMMITTER_NAME: 't', GIT_COMMITTER_EMAIL: 't@t', GIT_CONFIG_GLOBAL: path.join(root, 'gitconfig') }; + fs.writeFileSync(path.join(root, 'gitconfig'), '[commit]\n\tgpgsign = false\n'); + const sh = (cwd: string, c: string) => { const r = spawnSync('bash', ['-c', c], { cwd, env, encoding: 'utf8', timeout: 60_000 }); if (r.status !== 0) throw new Error(`${c}: ${r.stderr}`); return r.stdout.trim(); }; + sh(root, 'git init -q --bare -b main origin.git && git clone -q origin.git work 2>/dev/null'); + fs.mkdirSync(path.join(work, '.gstack')); + fs.writeFileSync(path.join(work, '.gstack', 'banned-terms.json'), JSON.stringify({ $schema: BANNED_TERMS_SCHEMA, terms: ['frobnicate'] }) + '\n'); + fs.writeFileSync(path.join(work, 'README.md'), '# t\n'); + sh(work, 'git add -A && git commit -qm base && git push -q origin main'); + sh(root, "git clone -q origin.git other && cd other && mkdir docs && printf 'Upstream notes frobnicate the parser.\\n\\nShowcasing vibrant features.\\n' > docs/up.md && git add -A && git commit -qm upstream && git push -q origin main"); + sh(work, "git fetch -q origin && git checkout -qb feat origin/main && mkdir src && echo 'export const v = 1;' > src/v.ts && git add -A && git commit -qm feat"); + const origin = sh(work, 'git rev-parse refs/remotes/origin/main'); + expect(sh(work, 'git rev-parse refs/heads/main')).not.toBe(origin); + const run = (bin: string, args: string[]) => { const r = spawnSync(path.join(BIN, bin), args, { cwd: work, env, encoding: 'utf8', timeout: 180_000 }); return { status: r.status, out: r.stdout ?? '', err: r.stderr ?? '' }; }; + return { root, work, sh, run, origin }; +} + +const NOTE = 'local main is behind origin/main; base is origin/main'; + +describe('a bare --base reads origin/ over a stale local branch (GST-1036)', () => { + // Value: protects=review-plan names the base ref its plan was computed against, so a stale-base plan shows in its output; + // fails_when=review-plan prints no REVIEW_BASE line, or names the local main while origin/main is ahead, or re-qualifies origin/main; + // why_new=GST-1036 (#3882 planned a docs diff at full depth against local main 319ef500); seam=bin/gstack-review-plan, lib/review-collect.ts collectLive + test('gstack-review-plan prints the ref it used and why', () => { + const { run, origin } = fixture(); + const bare = run('gstack-review-plan', ['--base', 'main']); + expect(bare.status, bare.out + bare.err).toBe(0); + expect(bare.out).toContain(`REVIEW_BASE: origin/main ${origin.slice(0, 12)}\n`); + expect(bare.err).toContain(`gstack-review-plan: ${NOTE}`); + const json = run('gstack-review-plan', ['--base', 'main', '--json']); + expect(JSON.parse(json.out)).toMatchObject({ paths: ['src/v.ts'], base: { ref: 'origin/main', sha: origin, local: 'behind' } }); + const qualified = run('gstack-review-plan', ['--base', 'origin/main']); + expect(qualified.out).toContain(`REVIEW_BASE: origin/main ${origin.slice(0, 12)}\n`); + expect(qualified.err).not.toContain('base is'); + }, 240_000); + + // Value: protects=banned-terms scans only the lines the branch adds; fails_when=it merges against the stale local main and reports a term an upstream commit added; + // why_new=GST-1036 sibling bin (/review passes a bare ); seam=bin/gstack-banned-terms + test('gstack-banned-terms check ignores inherited upstream lines', () => { + const { run } = fixture(); + for (const base of ['main', 'origin/main']) { + const r = run('gstack-banned-terms', ['check', '--base', base]); + expect(r.status, r.out + r.err).toBe(0); + expect(r.out).toContain('BANNED_TERMS: ok'); + expect(r.err).toContain(`gstack-banned-terms: ${NOTE}`); + } + }, 120_000); + + // Value: protects=without a remote-tracking twin the local base still stands, so the origin preference never hides lines; + // fails_when=the fix drops the local fallback; why_new=negative control for GST-1036; seam=bin/gstack-banned-terms + test('no origin: gstack-banned-terms keeps the local main and reports what the branch carries over it', () => { + const { sh, work, run } = fixture(); + sh(work, 'git remote remove origin'); + const r = run('gstack-banned-terms', ['check', '--base', 'main']); + expect(r.status, r.out + r.err).toBe(1); + expect(r.out).toContain('frobnicate'); + expect(r.err).not.toContain('base is'); + }, 120_000); + + // Value: protects=prose-lint --base lints only the lines the branch adds; fails_when=a bare base merges against the stale local main; + // why_new=GST-1036 sibling bin; seam=lib/prose-lint.ts addedOnly + test('gstack-prose-lint --base main skips a file only upstream changed; origin/main is read as given', () => { + const { run } = fixture(); + for (const base of ['main', 'origin/main']) { + const r = run('gstack-prose-lint', ['docs/up.md', '--base', base, '--gate']); + expect(r.status, r.out + r.err).toBe(0); + expect(r.out).not.toContain('PROSE:'); + } + }, 120_000); + + // Value: protects=the docs-audit snapshot selects only the branch's files; fails_when=--base main resolves to the stale local main and selects upstream docs; + // why_new=GST-1036 sibling bin; seam=bin/gstack-docs-candidate + test('gstack-docs-candidate snapshot --base main records origin/main and selects only the branch', () => { + const { root, run, origin } = fixture(); + const r = run('gstack-docs-candidate', ['snapshot', '--out', path.join(root, 'c.json'), '--audit-id', 'a1', '--mode', 'read-only', '--base', 'main']); + expect(r.status, r.out + r.err).toBe(0); + expect(r.out).toContain(`base=${origin} `); + expect(r.out).toContain('committed: "src/v.ts"\n'); + expect(r.err).toContain(`gstack-docs-candidate: ${NOTE}`); + }, 120_000); + + // Value: protects=the PR screenshot's before side is origin's base, not a stale local copy; fails_when=resolveCommit reads refs/heads/main first; + // why_new=GST-1036 sibling bin; seam=lib/pr-shots.ts resolveCommit + test('gstack-pr-shots --base main reads origin/main; a sha resolves as given', () => { + const { work, sh, origin } = fixture(); + expect(resolveCommit(work, 'main')).toBe(origin); + const local = sh(work, 'git rev-parse refs/heads/main'); + expect(resolveCommit(work, local)).toBe(local); + }, 120_000); +});