diff --git a/CHANGELOG.md b/CHANGELOG.md index ec57837bc..ffb1fe35f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,119 @@ # Changelog +## [1.93.33.0] - 2026-11-03 + +- On a checkout whose `origin/HEAD` points at the checked-out branch itself (a PR-branch clone), every base-relative step resolved the base to HEAD and reported zero changes, so review depth, `ready`, pregate and the docs audit passed on an empty diff (GST-774). Base resolution now skips a remote HEAD that names the checked-out branch, its upstream or HEAD's own commit (so a local branch under another name and a detached checkout of the PR branch are caught too), then uses the PR's base ref when `gh` finds one, then the remote's live default branch from `git ls-remote --symref`, then `main` or `master`. A repository whose default branch is `develop` or `trunk` keeps it. When the resolved base equals HEAD or contains it while the branch has commits its real base lacks, `gstack-diff-scope`, `gstack-gate-diff`, `gstack-review-plan`, `gstack-pregate`, `gstack-docs-candidate`, `/ship` policy base resolution and the receipt's depth binding refuse with `BASE_CONTAINS_HEAD` instead of reporting no changes. A pull request's head is refused whenever the base contains it; on the base branch itself, on a fresh branch with only uncommitted work, and on a detached HEAD given an explicit base, the step runs as before. `gstack-ship-queue land` resuming a merge-only PR that GitHub already merged with a true merge commit still lands, its depth check included. See docs/troubleshooting.md#base-contains-head. + +**The /ship receipt now carries the targeted reader's record.** When the review plan asked for the targeted reader on money, gate, auth or secret code and the reader ran, `gstack-ship-queue ready` still printed `REVIEW_TARGETED: … receipt=legacy verdict=missing` (gstack-evals#95, #3682, #3612), because nothing passed the record from Step 11 to the receipt. Enforcement stays report-only (GST-464). + +**A stale base no longer stands in for the PR's base (GST-462).** `gstack-review-plan --base main` read a local `main` that was behind `origin/main`, so commits the base already had counted as the branch's own change. #3597 was labeled gate code at full depth until it passed `--base origin/main`, and gbrain hit the same problem with a stale local `master` (GBRA-218). On harness-evals#152, a stale base made /ship review a 17,000-file diff of main's history instead of the PR (HARN-15). + +### Fixed +- **Step 11** saves the targeted record to `.gstack/tmp/targeted.json` (`gstack-review-dispatch verify --targeted-out`), and the Step 19 and handoff `gstack-ship-receipt write` commands pass it as `--review-targeted` whenever that file exists. Every Step 9 entry creates `.gstack/tmp`, adds it to `.git/info/exclude` and deletes the file. So a pass without the reader leaves the receipt legacy, an earlier pass's record can't carry over, and the record never counts as an uncommitted edit. +- **`gstack-ship-preflight`** fetches the base into `origin/` with an explicit refspec, so a single-branch clone also diffs against the current base tip. When a PR is open at HEAD and the diff lists far more files than the PR changes (more than twice as many, plus 50), or fewer, it stops before any reader runs. Fewer files means origin's base is ahead of the PR's, as in a fork, and the gate scan would miss commits the PR lands. The stop names the PR's base, or the fork case when the two names match. +- **`gstack-gate-diff` and `gstack-diff-scope`** compare against origin's tracking branch whenever it shares history with HEAD, the same base `gstack-review-plan`'s path list reads. A local `main` behind it counted commits the base already had; one ahead of or diverged from it left out commits the branch lands. gate-diff says which (`local main is ahead of origin/main; base is origin/main`). This applies whether you name the base or let the tool detect the default. A local branch stays the base only when origin's is missing or unrelated. Tracking branches are read by their full names (`refs/remotes/origin/main`), so while that branch exists, a tag or a fetched branch called `origin/main` can't stand in, whether you pass `main`, `origin/main` or nothing. A pinned sha is never redirected. `gstack-review-plan` reads both, so its gate scan and scope now match the merge base it already took from `origin/`. + +### Tests +- `test/review-targeted.test.ts`: the generated sections' own commands carry the record from Step 11's verify to `gstack-ship-receipt write`, and ready's line reads `verdict=ok`. A run without the reader passes no flag, and Step 9 clears the file, even when `.git/info` is missing or the exclude file has no final newline. +- `test/ship-preflight.test.ts`: a single-branch clone's preflight refreshes `origin/main`, and a 60-file diff against a 3-file or a 61-file PR stops at the pushed head. The stop quotes an unsafe base name and names the fork case. Negative controls: another head, a matching size, or a zero count goes on. +- `test/review-plan-cli.test.ts`, `test/gate-diff-cli.test.ts`, `test/diff-scope.test.ts`: a local `main` behind, ahead of or diverged from `origin/main` yields to it, so an unpushed `test.skip` on local `main` is scanned. One next to an unrelated `origin/main` stays the base. A tag or fetched branch named `origin/main` never stands in, and a pinned sha and `HEAD` are never redirected. The review plan of a docs-only branch that merged the newer `origin/main` stays docs; before the fix, the gate scan counted origin's skipped test. +- `test/review-packets-removed.test.ts`: the two ship section pins move under replay GST-464, a brief-identity check. Every reviewer brief `lib/review-brief.ts` cuts from those sections must stay byte-identical to the no-loss-tested bodies; a changed brief fails it. + +**/ship can leave a results PR's receipt files out of the review, and it stops instead of handing a reader a diff it cannot read.** A results PR in harness-evals or gbrain's eval lanes can carry thousands of receipt files. One HARN-1 results PR spent its outside-voice budget reading receipts and the pass died anyway (GST-507, GST-508). The repository owner can now list those folders in `.gstack/ship-policy.json`, and every reviewer reads the code without them. + +### Added +- **`review_exclude_paths` in `.gstack/ship-policy.json`** (`["receipts/**"]`, with `min_gstack_version` at least the release that adds it). It is read from the base branch only, so a branch cannot exclude its own files; a branch that adds the key gets a normal review and `REVIEW_EXCLUDE: none (head sets review_exclude_paths; …)`. Patterns are git glob pathspecs, root-anchored, and git itself lists the matches. +- **Only content is excluded**: data files, docs, `.log` files and files whose bytes are binary and whose extension is not source or script (a branch's `.gitattributes` cannot make a text file count as binary, and a NUL byte cannot hide a `.js` file). Untracked files are never part of the selection. Code, tests, executables, symlinks, submodule commits, gate paths, agent prose, config, `.gstack/` and `.github/` files, gate-tagged files and files with a credential line stay in every reader's diff, each with a reason code. A data file a code or test file names by its full path stays too, and when the gate scan fails nothing is excluded. A basename, a directory reference or a data manifest that lists receipts does not keep anything. +- **`gstack-review-plan` prints `REVIEW_EXCLUDE:`** with the counts, the excluded files by kind, the kept files and a digest. Every reader's `DIFF_CMD:`, the fixture command the security specialist and the Red Team use, and the outside voice's diff leave the files out; kept files come back through `DIFF_KEEP:`. Every brief says what was left out. The kept-file command is bounded at 24,000 argument bytes, and a pattern over that bound is not applied and the plan says why. +- **Excluded lines no longer trigger the numeric-claim scan or the targeted money, gate and auth classes**, and they count as docs for the review depth. +- **The receipt records `review_exclude`.** `gstack-review-dispatch verify` prints `REVIEW_EXCLUDE_RECORD: digest=` from the briefs the readers got, and `gstack-ship-receipt write --review-exclude ` must match the digest HEAD has under the base policy (`RECEIPT_INVALID field=review_exclude`, nothing written). Carry and the queue compare the digest, not the base commit, so a base merge that leaves the policy alone still carries. +- **A review ceiling.** `gstack-review-dispatch pass`, `stage` and `render` read each reader's diff once before writing anything. Over 2,000,000 bytes (about 500K tokens) is `REVIEW_DIFF_TOO_LARGE` (exit 3) with a fix for where the policy stands and pasteable JSON; a `git diff` that fails or runs past 120 seconds is `REVIEW_DIFF_UNREADABLE` (exit 1) with the reason, the timeout and a redacted stderr log. + +### Changed +- A repository without the key plans, briefs and diffs byte for byte as before. The one change it can see is the ceiling. A reader's diff over 2,000,000 bytes now stops the pass instead of being read in part. + +### Tests +- `test/review-exclude.test.ts`: what is excluded and kept (binary files under `config/`, a file named only by a data manifest or by its basename, a file a test names by full path), every brief's diff commands run against git, the numeric scan and targeted classes, the base policy read (no file, a missing blob, a bad commit, an invalid key), the argument-byte bound, both ceiling lines, and the receipt write, carry and queue checks. + +**The review planner no longer fails closed on repositories with big data trees just because its consumer search ran past 60 seconds (GST-544).** `gstack-review-plan` looks for code that names each changed data file. It used to run one `git grep` over the whole tree with every data file name, and each call had a hard 60-second limit. In garrytan/harness-evals, with 8,000+ result files, that search took about 150 seconds, so the plan fell back to a full review every time. + +### Changed +- **The consumer scan searches only files that can match** (`lib/review-collect.ts`). A cheap first pass lists the files that contain a changed file's extension, such as `.json`; the full search then reads only those files, a batch of paths per `git grep` call, with docs files left out. The result is the same as before. +- **One deadline per scan:** 120 seconds by default, a minute under the 180 seconds /ship gives the whole planner, set in milliseconds with `GSTACK_CONSUMER_SCAN_TIMEOUT_MS` (docs/review-policy.md#review-depth). Past it, or past 256 MB of matches across all the calls, the depth still fails closed with `consumer_scan_failed`. The detail now says whether git timed out, exited with an error or could not start, and names the deadline. Other git reads keep their 60-second limit. + +### Tests +- `test/review-collect.test.ts`: the narrowed search prints the same lines as one whole-tree `git grep` at the head, the merge base and the working tree; the same holds when every file is its own call; consumed sets on a mixed fixture (a chain through a data manifest, an untracked consumer, docs mentions ignored); a tiny deadline fails closed and says it timed out; the default deadline stays a minute under the planner timeout in `gstack-review-dispatch` and `gstack-ship-preflight`. The new behavior tests failed before the fix. + +**/review now names the kind of PR it is reviewing, so a CI or script change gets checks that fit it instead of a page of "not applicable".** The review checklist was written for application code. A PR that only changed CI or shell scripts came back with every category marked not applicable, which looked thorough but checked nothing, and nothing asked whether a new script even fit the repo (#356: a Windows `.bat` build script added to a bash + Makefile repo with Linux-only CI). + +### Changed +- **`review/checklist.md` starts with a PR type taken from the changed paths:** docs-only, CI/infra, scripts/tooling, application or mixed. Code under `bin/`, `scripts/` or `tools/` that is not shell, batch or PowerShell, package manifests and lockfiles, prompt templates (`*.tmpl`) and skill Markdown count as application, and so does any path the reviewer can't place. Application and mixed diffs run every category exactly as before; nothing is skipped. +- **On a CI/infra, scripts/tooling or docs-only diff,** Gate Integrity, Shell Injection, Completeness Gaps, Distribution & CI/CD Pipeline and the new Repo Conventions & Platform Fit category always run. Every other category with nothing to check is reported once, in one line (`Not applicable (PR type: ): `), and never counts as passed. +- **A new Repo Conventions & Platform Fit category** checks that new files use the languages, tools and naming the repo already uses for the same job, scripts run on the platforms the CI runners and docs target, and new tooling doesn't repeat an existing tool. A finding has to name its concrete consequence. +- **Distribution & CI/CD Pipeline gains script checks:** strict mode (`set -euo pipefail` or the repo's equivalent) in scripts CI or a build relies on, quoted variables, commands that work on the CI runners' OS, third-party actions pinned to a release tag or commit SHA, and tools installed at a pinned version. These checks apply to test-only CI changes too. + +### Tests +- `test/review-checklist-pr-type.test.ts` builds a classifier from the patterns the checklist itself lists and runs real path sets through it, among them a `.tmpl` change, `bin/` TypeScript, an extensionless `bin/` executable, `package.json` + `bun.lock`, a workflow-only change, a `.bat` add, docs, and mixed changes. It also checks the five types, the always-run list, the one-line not-applicable report, the new category and the line budget. All 19 tests fail on the previous checklist. + +**/review and /ship now triage comments from every review bot you configured, not only Greptile.** Teams on CodeRabbit, Macroscope or another bot got no triage at all: Step 2.5 and Step 10 read only comments whose author was exactly `greptile-apps[bot]` (#3122). The bot list is the one gstack-pr-wait already uses, so `gstack-config set pr_wait_gating_bots coderabbit` makes CodeRabbit the bot gstack waits on and triages, and `none` in both lists turns triage off. + +### Changed +- **`gstack-review-bots fetch`** reads every root line comment, conversation comment and the newest review body of each bot in `pr_wait_gating_bots` and `pr_wait_advisory_bots`, across all pages, through `gh api` (no `--slurp`, so the gh 2.45 that Ubuntu 24.04 packages works too). Only Bot accounts count, with gstack-pr-wait's login rule (a bot owns the logins that start with its name), so a person named `greptile-ci` is never triaged. It prints `REVIEW_BOTS: skip` (no PR, no `gh`, no GitHub remote, or both lists `none`), `REVIEW_BOTS: error` (a lookup or any page failed, so never a false zero) or a private directory with per-bot counts. +- **False-positive suppressions match only the same bot.** New triage outcomes go to `review-bot-history.md` with a sixth `bot` column through `gstack-review-bots record --dir --id `. The helper reads the bot and file path from the fetched comment, so a contributor-named file never reaches a shell command, and a path with `*` is refused so it can't become a wildcard suppression. Rows in the old `greptile-history.md` still count, as Greptile's. Nothing is migrated or deleted. +- **The PR body section is `## Review bot comments`** with a count per bot, and retro shows one signal row per bot (`REVIEW_BOT_HISTORY` beside `GREPTILE_HISTORY`). +- **Greptile's early PR (Step 6.5) runs only when `greptile` is in one of the two lists**, and detection now needs a Greptile Bot account instead of any login matching `/greptile/i`. Tier 2 replies and re-ranking stay Greptile-only; other bots get the plain evidence reply when you pick it. +- **/ship's driver routes Step 10** to the bot triage section whenever a PR exists, instead of running it only when Greptile is detected. +- **docs/skills.md** has a "Choosing your bots" section. + +### Tests +- `test/review-bots.test.ts` (fake `gh` runner): page-two comments, a CodeRabbit review body, one item from a root comment plus replies, human lookalikes and deleted accounts left out, overlapping bot names, an error on any page giving no directory, lookup failures giving `error`, an empty PR giving zero counts, cross-bot suppression isolation, old greptile-history rows suppressing Greptile only, and `record` refusing hostile paths, colliding ids and unknown categories. +- `test/greptile-early.test.ts`: early detection is off when `greptile` is in neither list, and a human account named like Greptile is no signal. + +## [1.93.32.0] - 2026-11-03 + +- **`bin/dev-setup` works in a fresh clone again.** It installed dependencies, then setup refused with "project-local Codex destination overlaps the source tree" and exited 1, because setup's overlap guard read gstack's own `.agents/skills` as a foreign project's skills directory pointing into the checkout. Setup now allows that one layout, where the project is the gstack checkout itself and its Codex skills directory is that checkout's own `.agents/skills`. Every other overlap is still refused. That includes a user project whose `.agents/skills` leads into a gstack checkout, a checkout whose `.agents/skills` points at its own root, and `--host codex` run from a dev checkout, which the Codex preflight still stops before any write. test/dev-setup-fresh-clone.test.ts covers the fresh clone and all three refusals (GST-768). + +## [1.93.31.0] - 2026-11-03 + +**A CSO hardening test now fails at the Git step that broke instead of passing without testing anything.** `fixtureGit` in `test/cso-git-hardening.test.ts` threw only on a non-zero exit status. When Git was killed by its 30-second timeout or could not start, `spawnSync` returned a `null` status, and the helper treated that as success. If that happened to the `git replace` step, the test that checks a security snapshot ignores replaced history would pass without a replacement in place. The helper now throws on a spawn error or any status other than 0, and the message names the Git arguments, the status and the signal. The stricter helper showed that the file's warm-up step had been calling `/usr/bin/git` on Windows, where it doesn't exist, on every run. It was passing only because the error was swallowed. Every test in the file already skips on Windows, so the warm-up now skips there too. + +### Tests +- `test/cso-git-hardening.test.ts`: `fixtureGit` throws on `result.error` or `status !== 0` (#3476), and the `beforeAll` warm-up returns early on Windows (Windows Free Tests run 41236157469 failed in it with `ENOENT … uv_spawn '/usr/bin/git'` once the error stopped being swallowed). + +## [1.93.30.0] - 2026-11-03 + +**`browse` starts again when proxy credentials come from `BROWSE_PROXY_USER` and `BROWSE_PROXY_PASS`.** With those two variables set beside `--proxy`, `BROWSE_PROXY_URL` or a standard proxy variable such as `HTTPS_PROXY`, the daemon exited at startup with `proxy creds set in both env (BROWSE_PROXY_USER) and URL — pick one source`, and the CLI reported only `Server failed to start within 15s`. The CLI puts those credentials into the proxy URL it hands the daemon, and the daemon also inherited the two variables, so it saw the same credentials twice and refused them. The daemon now gets the variables blanked whenever the CLI passes it a resolved proxy URL, so it reads the credentials once, from the URL. Setting credentials both in the URL and in the variables is still refused, with the same message, before any daemon starts. + +### Fixed +- **`browse/src/cli.ts`**: the daemon environment for a resolved proxy sets `BROWSE_PROXY_USER` and `BROWSE_PROXY_PASS` to empty. This covers a normal start, an auto-restart, a headed `connect` and its supervisor respawn, and the Windows launcher (#3457). + +### Tests +- `browse/test/restart-env.test.ts`: for `--proxy`, `BROWSE_PROXY_URL` and `HTTPS_PROXY` with `BROWSE_PROXY_USER`/`PASS`, the restart and headed daemon environments parse the way `server.ts` parses them and yield the credentials once. All three failed before the fix with the "pick one source" error. +- `browse/test/proxy-env-daemon.test.ts`: an end-to-end start with `HTTP_PROXY` and `BROWSE_PROXY_USER`/`PASS` against a local proxy that demands Basic auth loads the page through the proxy. It timed out at the 15 s start limit before the fix. +- `browse/test/cli-supervisor.test.ts`: the expected headed respawn environment includes the two blanked variables. + +## [1.93.29.0] - 2026-11-03 + +**A `gbrain serve --http` on this machine's loopback address now counts as a local brain, so transcripts get imported again.** gbrain recommends this setup for local agents on a Postgres brain (`gbrain bootstrap harness`). gstack treated every http MCP entry as a brain on another machine (Path 4). So `/sync-gbrain` staged transcripts for a brain admin who doesn't exist and never ran `gbrain import` (#3539). Ported from Willy Morin's #3552. + +### Fixed +- **One locality rule** (`lib/gbrain-mcp-locality.ts`). An http, url or sse entry is local when its URL is http(s) on `localhost`, `127.0.0.0/8` or `::1`, and this machine's gbrain config names a Postgres engine (a `database_url`, not PGLite) with no `remote_mcp` marker. A loopback URL next to a local PGLite stays remote, so Path 4's split-engine setup through a tunnel is unchanged. A URL with userinfo or `${` is never local. Every other entry is classified as before. +- **`gstack-memory-ingest`** imports through gbrain in that case. Pages an earlier run left `staged` / `remote-http` count as new and are imported on the next local run. Pages older than the 90-day window need one `gstack-memory-ingest --bulk --all-history` run. +- **`gstack-gbrain-detect`** reports `local-stdio` for it in all three tiers, and `gbrain-local-status` and the `gstack-skill-start` banner use the same rule. A remote `(SSE)` line in `claude mcp list` no longer reads as local. With several gbrain servers, the exact `gbrain:` entry wins, as in the other tiers. + +### Docs +- `USING_GBRAIN_WITH_GSTACK.md`: a loopback serve over this machine's Postgres brain is not Path 4; a loopback tunnel next to a local PGLite still is. + +### Tests +- `test/gbrain-mcp-locality.test.ts` covers the rule, the list-line parser, and a check that the shipped jq copy never says local where the TypeScript rule says remote. The detect, local-status and memory-ingest tests add loopback and stuck-page cases. On main's code (the tests-first commit f3712618), 13 of the new cases fail and `test/gbrain-mcp-locality.test.ts` cannot load, because the module it tests is new. + +## [1.93.28.0] - 2026-11-03 + +**`test/cso-snapshot-state.test.ts` no longer hangs when a paused contender is resumed too early.** + +### Fixed +- The five paused-contender tests resume a child only once the process table shows it stopped (`/proc//stat` on Linux, `ps` on macOS). Each child marks its pause and then stops itself in two steps, and a SIGCONT that arrived between the two did nothing, so the child stayed stopped and the test timed out (Free Tests 41117003351). With a 200 ms delay injected between the two steps, main failed 3 and then 5 of the 5 tests across two runs, and this change passed 5 of 5 both times. A child that exits, or never stops within 2 s, now fails with a named error instead of hanging. Timing bounds are unchanged. + ## [1.93.27.0] - 2026-11-02 **A test can keep an old value on purpose without turning off `literals` (#3490).** When a minimum version goes up, the test that checks the old version is refused has to keep the old value, and pregate failed it with `PREGATE_LITERALS`. The only way out was `warn: ["literals"]`, which turns the check off for every constant. Now that one line can carry `// pregate-literal: NAME= `. diff --git a/USING_GBRAIN_WITH_GSTACK.md b/USING_GBRAIN_WITH_GSTACK.md index 7b8d40cf4..3153388a0 100644 --- a/USING_GBRAIN_WITH_GSTACK.md +++ b/USING_GBRAIN_WITH_GSTACK.md @@ -74,6 +74,8 @@ If you accept the local PGLite, you end up in **split-engine mode**: - **Brain/context queries** (`mcp__gbrain__search`, `mcp__gbrain__query`, `mcp__gbrain__get_page`) route to the remote MCP. Plans, retros, learnings, cross-machine memory — all on the shared server. - **Code queries** (`gbrain code-def`, `code-refs`, `code-callers`, `code-callees`, `gbrain search` for code) route to the local PGLite via the `.gbrain-source` pin in each worktree. Indexed locally, fast, never leaves the machine. +A `gbrain serve --http` on this machine's loopback address (`localhost`, `127.0.0.1`, `::1`) over this machine's own Postgres brain is not Path 4. That is gbrain's recommended setup for local agents on a Postgres brain (`gbrain bootstrap harness`), and gstack treats it as a local brain: transcripts go through `gbrain import` here. gstack decides this from the URL and the local gbrain config alone, and only a config with a Postgres `database_url` counts. A loopback URL next to a local PGLite stays Path 4, so the split-engine setup above keeps working when you reach the remote brain through an SSH tunnel or port forward on a loopback port. + The two engines are independent. Wiping the local PGLite doesn't touch the remote brain; rotating the remote MCP bearer doesn't affect local code search. This is also the right configuration if your remote brain admin can't (or shouldn't) index every developer's checkout — local code stays local. ## MCP registration for Claude Code diff --git a/VERSION b/VERSION index 000a12e4a..bd584c813 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.93.27.0 +1.93.33.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index e9707255b..9170ca05c 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.93.27.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.93.33.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-gbrain-detect b/bin/gstack-gbrain-detect index e79f4d558..0183f549d 100755 --- a/bin/gstack-gbrain-detect +++ b/bin/gstack-gbrain-detect @@ -45,6 +45,7 @@ import { readGbrainVersion, } from "../lib/gbrain-local-status"; import { gbrainConfigDir, isTransactionModePooler } from "../lib/gbrain-exec"; +import { classifyGbrainMcpEntry, classifyGbrainMcpListLine, type GbrainMcpEntry } from "../lib/gbrain-mcp-locality"; import { resolveStateRoot } from "../lib/state-root"; const STATE_DIR = resolveStateRoot(); @@ -154,7 +155,19 @@ function detectBrainGit(): boolean { // 1. `claude mcp get gbrain --json` — public CLI surface, structured output // 2. `claude mcp list` text-grep — older claude versions without --json // 3. `~/.claude.json` jq read — last resort if `claude` isn't on PATH -function detectMcpMode(): "local-stdio" | "remote-http" | "none" { +// +// #3539: an HTTP entry on a loopback host whose local gbrain config has an +// engine and no remote_mcp marker is a resident `gbrain serve --http` of this +// machine's own brain. It reports "local-stdio" (the local-engine mode every +// consumer already understands: memory ingest imports, sync-gbrain skips the +// remote trust-policy question) rather than a new value. +type McpMode = "local-stdio" | "remote-http"; +function classify(entry: GbrainMcpEntry): McpMode | null { + const c = classifyGbrainMcpEntry(entry); + return c === "remote" ? "remote-http" : c === "local" ? "local-stdio" : null; +} + +function detectMcpMode(): McpMode | "none" { const claudeOnPath = tryExec("sh", ["-c", "command -v claude"], 1_000) !== null; if (claudeOnPath) { // Tier 1: `claude mcp get gbrain --json` @@ -167,11 +180,8 @@ function detectMcpMode(): "local-stdio" | "remote-http" | "none" { command?: string; url?: string; }; - const mtype = parsed.type || parsed.transport || ""; - if (mtype === "http" || mtype === "sse") return "remote-http"; - if (mtype === "stdio") return "local-stdio"; - if (parsed.url) return "remote-http"; - if (parsed.command) return "local-stdio"; + const c = classify(parsed); + if (c) return c; } catch { // fall through } @@ -179,34 +189,28 @@ function detectMcpMode(): "local-stdio" | "remote-http" | "none" { // Tier 2: `claude mcp list` text-grep. Name-pattern generalized (#2051): // a gbrain server registered as e.g. "gbrain-remote" or "gbrain_work" // still counts. Anchored to the gbrain token so unrelated servers can't - // false-positive. + // false-positive. The exact `gbrain:` line wins over a variant, the same + // order as tier 3 (and memory-ingest, which reads mcpServers.gbrain): + // {gbrain-a: loopback, gbrain: remote} is remote-http. const list = tryExec("claude", ["mcp", "list"], 3_000); if (list) { - const line = list.split("\n").find((l) => /^gbrain([-_][\w-]*)?:/.test(l)); + const lines = list.split("\n"); + const line = + lines.find((l) => l.startsWith("gbrain:")) ?? lines.find((l) => /^gbrain[-_][\w-]*:/.test(l)); if (line) { - if (/\b(http|HTTP)\b/.test(line)) return "remote-http"; - return "local-stdio"; + // The list line carries the URL for HTTP and SSE servers + // ("gbrain: http://127.0.0.1:3131/mcp (HTTP) - ✓ Connected"), so the + // loopback rule (#3539) applies here too. The whole URL token is + // classified, never cut at ")"; an HTTP or SSE line without a usable + // URL keeps the old remote-http answer. + return classifyGbrainMcpListLine(line) === "local" ? "local-stdio" : "remote-http"; } } } // Tier 3: read ~/.claude.json directly - interface McpServerEntry { - type?: string; - transport?: string; - command?: string; - url?: string; - } const cj = tryReadJSON(CLAUDE_JSON) as - | { mcpServers?: Record } + | { mcpServers?: Record } | null; - const classify = (entry: McpServerEntry): "local-stdio" | "remote-http" | null => { - const mtype = entry.type || entry.transport || ""; - if (mtype === "url" || mtype === "http" || mtype === "sse") return "remote-http"; - if (mtype === "stdio") return "local-stdio"; - if (entry.url) return "remote-http"; - if (entry.command) return "local-stdio"; - return null; - }; const servers = cj?.mcpServers || {}; const exact = servers["gbrain"]; if (exact) { diff --git a/bin/gstack-memory-ingest.ts b/bin/gstack-memory-ingest.ts index b2a69fe1a..dba4605fe 100644 --- a/bin/gstack-memory-ingest.ts +++ b/bin/gstack-memory-ingest.ts @@ -69,6 +69,7 @@ import { withErrorContext, } from "../lib/gstack-memory-helpers"; import { execGbrainText, gbrainConfigDir, spawnGbrain, spawnGbrainAsync } from "../lib/gbrain-exec"; +import { classifyGbrainMcpEntry, type GbrainMcpEntry } from "../lib/gbrain-mcp-locality"; import { constrainSourceId, parseSourcesList } from "../lib/gbrain-sources"; import { BISECT_AFTER_REFUSALS, @@ -427,9 +428,19 @@ function fileSha256(path: string): string { } } +/** + * #3539: a page staged for a remote brain's pull (`staged` / `remote-http`) + * was never imported here. Once the brain is local (a loopback serve, or the + * entry switched back to stdio), that page has not landed, so it must be + * prepared again instead of counting as unchanged. + */ +function stagedForRemotePull(entry: StateEntry | undefined): boolean { + return !!entry && entry.status === "staged" && entry.source_id === "remote-http" && !isRemoteHttpMcpMode(); +} + function fileChangedSinceState(path: string, state: IngestState, verifyHash = false): boolean { const entry = state.sessions[path]; - if (isRetryable(entry)) return true; + if (isRetryable(entry) || stagedForRemotePull(entry)) return true; try { const st = statSync(path); const mtimeNs = Math.floor(st.mtimeMs * 1e6); @@ -1418,7 +1429,7 @@ async function probeMode(args: CliArgs): Promise { totalBytes += size; const entry = state.sessions[path]; - if (!entry) newCount++; + if (!entry || stagedForRemotePull(entry)) newCount++; else if (fileChangedSinceState(path, state, args.scanSecrets)) updatedCount++; else unchangedCount++; } @@ -1800,23 +1811,27 @@ function makePersistentTranscriptDir(): string { * * Reads ~/.claude.json directly (same fallback chain as gstack-gbrain-detect * Tier 3). Cheap: one fs read, no fork-exec. + * + * #3539: an HTTP entry on a loopback host that reaches this machine's own + * gbrain engine (a resident `gbrain serve --http`) is not remote; see + * lib/gbrain-mcp-locality.ts. Memoized: the change check consults it per + * state entry, and ~/.claude.json can be megabytes. */ +let remoteHttpMcpMode: boolean | undefined; function isRemoteHttpMcpMode(): boolean { + remoteHttpMcpMode ??= readRemoteHttpMcpMode(); + return remoteHttpMcpMode; +} + +function readRemoteHttpMcpMode(): boolean { const home = process.env.HOME || homedir(); const claudeJsonPath = join(home, ".claude.json"); if (!existsSync(claudeJsonPath)) return false; try { const parsed = JSON.parse(readFileSync(claudeJsonPath, "utf-8")) as { - mcpServers?: { - gbrain?: { type?: string; transport?: string; url?: string }; - }; + mcpServers?: { gbrain?: GbrainMcpEntry }; }; - const entry = parsed.mcpServers?.gbrain; - if (!entry) return false; - const mtype = entry.type || entry.transport || ""; - if (mtype === "url" || mtype === "http" || mtype === "sse") return true; - if (entry.url) return true; - return false; + return classifyGbrainMcpEntry(parsed.mcpServers?.gbrain) === "remote"; } catch { return false; } diff --git a/bin/gstack-skill-start b/bin/gstack-skill-start index fb26ee8e5..16b1c9133 100755 --- a/bin/gstack-skill-start +++ b/bin/gstack-skill-start @@ -426,7 +426,29 @@ if command -v jq >/dev/null 2>&1 && [ -f "$HOME/.claude.json" ] && grep -q '"gbr _GBRAIN_MCP_ENTRY=$(jq -c --arg cwd "$PWD" '((.projects // {}) | to_entries | map(select((.key as $k | $cwd == $k or ($cwd | startswith($k + "/")) or ($cwd | startswith($k + "\\"))) and ((try .value.mcpServers.gbrain catch null) != null))) | sort_by(.key | length) | last | .value.mcpServers.gbrain) // .mcpServers.gbrain // empty' "$HOME/.claude.json" 2>/dev/null) _GBRAIN_MCP_TYPE=$(printf '%s' "$_GBRAIN_MCP_ENTRY" | jq -r '.type // .transport // empty' 2>/dev/null) case "$_GBRAIN_MCP_TYPE" in - url|http|sse) _GBRAIN_MCP_MODE="remote-http" ;; + url|http|sse) + _GBRAIN_MCP_MODE="remote-http" + # #3539: a loopback URL whose local gbrain config names a Postgres + # engine (a database_url, engine not pglite) and no remote_mcp marker is + # this machine's own `gbrain serve --http`, not a remote brain. A conservative subset of lib/gbrain-mcp-locality.ts, not + # an exact mirror: the URL test accepts only canonical loopback + # spellings (localhost, 127.a.b.c with valid decimal octets, [::1], a + # valid port), so other spellings stay remote here. It lowercases with + # ascii_downcase, not the "i" flag, which case-folds U+017F to "s". As in + # TS, a URL containing "${" is remote (Claude Code expands it), and the + # regex rejects userinfo. The config test + # matches the TS one: remote_mcp is a marker when JS-truthy, as in + # gbrain's isThinClient (null, false, 0 and "" are not), and + # database_url counts when non-blank after JS trim() and engine is not + # pglite. GBRAIN_HOME is the + # parent of .gbrain, as in gbrainConfigDir. test/gbrain-mcp-locality.test.ts + # runs both programs below against the TS rule. + if printf '%s' "$_GBRAIN_MCP_ENTRY" | jq -e '(.url // "") | ascii_downcase | (contains("${") | not) and test("\\Ahttps?://(localhost|127(\\.(25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9]?[0-9])){3}|\\[::1\\])(:([0-9]{0,4}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5]))?([/?#]|\\z)")' >/dev/null 2>&1 \ + && jq -e '((.remote_mcp // false) | . == false or . == 0 or . == "") and .engine != "pglite" and ((.database_url // "") | (type == "string" and test("[^\\s\\x{feff}]|\\x{0085}")))' \ + "${GBRAIN_HOME:-$HOME}/.gbrain/config.json" >/dev/null 2>&1; then + _GBRAIN_MCP_MODE="local-stdio" + fi + ;; stdio) _GBRAIN_MCP_MODE="local-stdio" ;; esac fi diff --git a/browse/src/cli.ts b/browse/src/cli.ts index e884952b0..208889ec2 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -523,12 +523,16 @@ export function proxyMismatchLines(flags: GlobalFlags, state: ServerState, env: * The server env for this invocation's resolved proxy (#915); empty when * direct, except BROWSE_PROXY_URL=direct where that bypasses the system proxy * settings (#3440), so a daemon start or auto-restart carries the opt-out the - * session's hash was computed from. + * session's hash was computed from. The resolved URL already carries any + * BROWSE_PROXY_USER/PASS, so those are blanked for the daemon, which inherits + * this environment and would otherwise see the creds twice (#3457). */ function proxyServerEnv(flags: Partial> | null | undefined): Record { if (!flags?.proxyUrl) return flags?.explicitDirect ? { BROWSE_PROXY_URL: 'direct' } : {}; return { BROWSE_PROXY_URL: flags.proxyUrl, + BROWSE_PROXY_USER: '', + BROWSE_PROXY_PASS: '', ...(flags.proxySource ? { BROWSE_PROXY_SOURCE: flags.proxySource } : {}), ...(flags.proxyBypass ? { BROWSE_PROXY_BYPASS: flags.proxyBypass } : {}), }; diff --git a/browse/test/cli-supervisor.test.ts b/browse/test/cli-supervisor.test.ts index eed733983..3e9e27809 100644 --- a/browse/test/cli-supervisor.test.ts +++ b/browse/test/cli-supervisor.test.ts @@ -124,7 +124,7 @@ describe('runHeadedSupervisor (behavior)', () => { expect(h.calls.startEnv).toHaveLength(1); expect(h.calls.startEnv[0]).toEqual({ BROWSE_HEADED: '1', BROWSE_PORT: '34567', BROWSE_PARENT_PID: '0', - BROWSE_PROXY_URL: 'socks5://127.0.0.1:9050', BROWSE_CONFIG_HASH: 'abc123', + BROWSE_PROXY_URL: 'socks5://127.0.0.1:9050', BROWSE_PROXY_USER: '', BROWSE_PROXY_PASS: '', BROWSE_CONFIG_HASH: 'abc123', }); expect(h.calls.startEnv[0]).toBe(h.deps.env); expect(h.calls.agents).toEqual([5000]); diff --git a/browse/test/proxy-env-daemon.test.ts b/browse/test/proxy-env-daemon.test.ts index 785eef9bd..68ab854da 100644 --- a/browse/test/proxy-env-daemon.test.ts +++ b/browse/test/proxy-env-daemon.test.ts @@ -14,6 +14,8 @@ import * as path from 'node:path'; const CLI = path.resolve(import.meta.dir, '../src/cli.ts'); const PROXIED_HOST = 'proxied.gstack-915.test'; +const AUTH_HOST = 'auth.gstack-3457.test'; +const AUTH_HEADER = `Basic ${Buffer.from('proxy-user:proxy-secret').toString('base64')}`; let scratch = ''; let stateFile = ''; @@ -27,6 +29,11 @@ beforeAll(async () => { stateFile = path.join(scratch, 'state', 'browse.json'); proxy = http.createServer((req, res) => { const host = new URL(req.url ?? '/', 'http://unknown').hostname; + if (host === AUTH_HOST && req.headers['proxy-authorization'] !== AUTH_HEADER) { + res.writeHead(407, { 'Proxy-Authenticate': 'Basic realm="gstack-3457"' }); + res.end(); + return; + } proxiedHosts.push(host); res.writeHead(200, { 'Content-Type': 'text/html' }); res.end(`

via proxy for ${host}

`); @@ -177,4 +184,15 @@ describe.skipIf(process.platform === 'win32')('#915 proxy from the environment, } expect(daemonPid()).toBe(pid); }, 120_000); + + test('BROWSE_PROXY_USER/PASS with HTTP_PROXY: the daemon starts and Chromium answers the proxy auth challenge (#3457)', async () => { + expect((await browse(['stop'], {})).code).toBe(0); + const env = { HTTP_PROXY: proxyUrl, BROWSE_PROXY_USER: 'proxy-user', BROWSE_PROXY_PASS: 'proxy-secret' }; + const r = await browse(['goto', `http://${AUTH_HOST}/`], env); + expect(r.code, r.out).toBe(0); + expect(r.stderr).toContain(`Starting server with proxy ${proxyUrl.replace('http://', 'http://***:***@')}/ from HTTP_PROXY...`); + expect(r.out).not.toContain('proxy-secret'); + expect(proxiedHosts).toContain(AUTH_HOST); + expect((await browse(['text'], env)).stdout).toContain(`via proxy for ${AUTH_HOST}`); + }, 120_000); }); diff --git a/browse/test/restart-env.test.ts b/browse/test/restart-env.test.ts index a48eb1a32..6a9aea842 100644 --- a/browse/test/restart-env.test.ts +++ b/browse/test/restart-env.test.ts @@ -1,5 +1,6 @@ import { describe, test, expect } from "bun:test"; -import { buildRestartEnv } from "../src/cli"; +import { buildHeadedServerEnv, buildRestartEnv, extractGlobalFlags } from "../src/cli"; +import { parseProxyConfig, serverProxyEnv } from "../src/proxy-config"; // #1781: an auto-restart triggered by a plain command (no --headed flag) must // NOT silently downgrade a headed session to headless. buildRestartEnv reapplies @@ -48,3 +49,36 @@ describe("buildRestartEnv (#1781 headed persistence)", () => { expect(env.BROWSE_CONFIG_HASH).toBe("fromstate"); }); }); + +// #3457: the CLI folds BROWSE_PROXY_USER/PASS into the BROWSE_PROXY_URL it hands +// the daemon, and the daemon inherits the CLI's environment. The daemon parses +// that URL with the env creds exactly as server.ts does, so it must see one source. +describe("daemon env carries one proxy credential source (#3457)", () => { + const userEnv = { BROWSE_PROXY_USER: "a", BROWSE_PROXY_PASS: "b" }; + const daemonParse = (env: Record) => { + const { url, source } = serverProxyEnv(env, "linux"); + return parseProxyConfig({ proxyUrl: url!, envUser: env.BROWSE_PROXY_USER, envPass: env.BROWSE_PROXY_PASS, source }); + }; + const spellings: Array<[string, string[], Record]> = [ + ["--proxy", ["--proxy", "http://127.0.0.1:3128", "status"], {}], + ["BROWSE_PROXY_URL", ["status"], { BROWSE_PROXY_URL: "http://127.0.0.1:3128" }], + ["HTTPS_PROXY", ["status"], { HTTPS_PROXY: "http://127.0.0.1:3128" }], + ]; + + for (const [name, args, proxyEnv] of spellings) { + test(`${name} with BROWSE_PROXY_USER/PASS: start, restart and headed daemons parse the creds once`, () => { + const env = { ...userEnv, ...proxyEnv }; + const flags = extractGlobalFlags(args, env, "linux"); + for (const childEnv of [buildRestartEnv(flags, null), buildHeadedServerEnv(flags)]) { + const parsed = daemonParse({ ...env, ...childEnv }); + expect(parsed).toMatchObject({ host: "127.0.0.1", port: 3128, userId: "a", password: "b" }); + } + }); + } + + test("without a resolved proxy the daemon env leaves BROWSE_PROXY_USER/PASS alone", () => { + const env = buildRestartEnv(extractGlobalFlags(["status"], userEnv, "linux"), null); + expect(env.BROWSE_PROXY_USER).toBeUndefined(); + expect(env.BROWSE_PROXY_PASS).toBeUndefined(); + }); +}); diff --git a/changelog.d/capy-refuse-a-base-that.md b/changelog.d/capy-refuse-a-base-that.md deleted file mode 100644 index cd9c3222c..000000000 --- a/changelog.d/capy-refuse-a-base-that.md +++ /dev/null @@ -1 +0,0 @@ -- On a checkout whose `origin/HEAD` points at the checked-out branch itself (a PR-branch clone), every base-relative step resolved the base to HEAD and reported zero changes, so review depth, `ready`, pregate and the docs audit passed on an empty diff (GST-774). Base resolution now skips a remote HEAD that names the checked-out branch, its upstream or HEAD's own commit (so a local branch under another name and a detached checkout of the PR branch are caught too), then uses the PR's base ref when `gh` finds one, then the remote's live default branch from `git ls-remote --symref`, then `main` or `master`. A repository whose default branch is `develop` or `trunk` keeps it. When the resolved base equals HEAD or contains it while the branch has commits its real base lacks, `gstack-diff-scope`, `gstack-gate-diff`, `gstack-review-plan`, `gstack-pregate`, `gstack-docs-candidate`, `/ship` policy base resolution and the receipt's depth binding refuse with `BASE_CONTAINS_HEAD` instead of reporting no changes. A pull request's head is refused whenever the base contains it; on the base branch itself, on a fresh branch with only uncommitted work, and on a detached HEAD given an explicit base, the step runs as before. `gstack-ship-queue land` resuming a merge-only PR that GitHub already merged with a true merge commit still lands, its depth check included. See docs/troubleshooting.md#base-contains-head. diff --git a/changelog.d/gst-464-receipt-targeted-base.md b/changelog.d/gst-464-receipt-targeted-base.md deleted file mode 100644 index 145394224..000000000 --- a/changelog.d/gst-464-receipt-targeted-base.md +++ /dev/null @@ -1,14 +0,0 @@ -**The /ship receipt now carries the targeted reader's record.** When the review plan asked for the targeted reader on money, gate, auth or secret code and the reader ran, `gstack-ship-queue ready` still printed `REVIEW_TARGETED: … receipt=legacy verdict=missing` (gstack-evals#95, #3682, #3612), because nothing passed the record from Step 11 to the receipt. Enforcement stays report-only (GST-464). - -**A stale base no longer stands in for the PR's base (GST-462).** `gstack-review-plan --base main` read a local `main` that was behind `origin/main`, so commits the base already had counted as the branch's own change. #3597 was labeled gate code at full depth until it passed `--base origin/main`, and gbrain hit the same problem with a stale local `master` (GBRA-218). On harness-evals#152, a stale base made /ship review a 17,000-file diff of main's history instead of the PR (HARN-15). - -### Fixed -- **Step 11** saves the targeted record to `.gstack/tmp/targeted.json` (`gstack-review-dispatch verify --targeted-out`), and the Step 19 and handoff `gstack-ship-receipt write` commands pass it as `--review-targeted` whenever that file exists. Every Step 9 entry creates `.gstack/tmp`, adds it to `.git/info/exclude` and deletes the file. So a pass without the reader leaves the receipt legacy, an earlier pass's record can't carry over, and the record never counts as an uncommitted edit. -- **`gstack-ship-preflight`** fetches the base into `origin/` with an explicit refspec, so a single-branch clone also diffs against the current base tip. When a PR is open at HEAD and the diff lists far more files than the PR changes (more than twice as many, plus 50), or fewer, it stops before any reader runs. Fewer files means origin's base is ahead of the PR's, as in a fork, and the gate scan would miss commits the PR lands. The stop names the PR's base, or the fork case when the two names match. -- **`gstack-gate-diff` and `gstack-diff-scope`** compare against origin's tracking branch whenever it shares history with HEAD, the same base `gstack-review-plan`'s path list reads. A local `main` behind it counted commits the base already had; one ahead of or diverged from it left out commits the branch lands. gate-diff says which (`local main is ahead of origin/main; base is origin/main`). This applies whether you name the base or let the tool detect the default. A local branch stays the base only when origin's is missing or unrelated. Tracking branches are read by their full names (`refs/remotes/origin/main`), so while that branch exists, a tag or a fetched branch called `origin/main` can't stand in, whether you pass `main`, `origin/main` or nothing. A pinned sha is never redirected. `gstack-review-plan` reads both, so its gate scan and scope now match the merge base it already took from `origin/`. - -### Tests -- `test/review-targeted.test.ts`: the generated sections' own commands carry the record from Step 11's verify to `gstack-ship-receipt write`, and ready's line reads `verdict=ok`. A run without the reader passes no flag, and Step 9 clears the file, even when `.git/info` is missing or the exclude file has no final newline. -- `test/ship-preflight.test.ts`: a single-branch clone's preflight refreshes `origin/main`, and a 60-file diff against a 3-file or a 61-file PR stops at the pushed head. The stop quotes an unsafe base name and names the fork case. Negative controls: another head, a matching size, or a zero count goes on. -- `test/review-plan-cli.test.ts`, `test/gate-diff-cli.test.ts`, `test/diff-scope.test.ts`: a local `main` behind, ahead of or diverged from `origin/main` yields to it, so an unpushed `test.skip` on local `main` is scanned. One next to an unrelated `origin/main` stays the base. A tag or fetched branch named `origin/main` never stands in, and a pinned sha and `HEAD` are never redirected. The review plan of a docs-only branch that merged the newer `origin/main` stays docs; before the fix, the gate scan counted origin's skipped test. -- `test/review-packets-removed.test.ts`: the two ship section pins move under replay GST-464, a brief-identity check. Every reviewer brief `lib/review-brief.ts` cuts from those sections must stay byte-identical to the no-loss-tested bodies; a changed brief fails it. diff --git a/changelog.d/gst-508-review-exclude-paths.md b/changelog.d/gst-508-review-exclude-paths.md deleted file mode 100644 index addd6c7ca..000000000 --- a/changelog.d/gst-508-review-exclude-paths.md +++ /dev/null @@ -1,15 +0,0 @@ -**/ship can leave a results PR's receipt files out of the review, and it stops instead of handing a reader a diff it cannot read.** A results PR in harness-evals or gbrain's eval lanes can carry thousands of receipt files. One HARN-1 results PR spent its outside-voice budget reading receipts and the pass died anyway (GST-507, GST-508). The repository owner can now list those folders in `.gstack/ship-policy.json`, and every reviewer reads the code without them. - -### Added -- **`review_exclude_paths` in `.gstack/ship-policy.json`** (`["receipts/**"]`, with `min_gstack_version` at least the release that adds it). It is read from the base branch only, so a branch cannot exclude its own files; a branch that adds the key gets a normal review and `REVIEW_EXCLUDE: none (head sets review_exclude_paths; …)`. Patterns are git glob pathspecs, root-anchored, and git itself lists the matches. -- **Only content is excluded**: data files, docs, `.log` files and files whose bytes are binary and whose extension is not source or script (a branch's `.gitattributes` cannot make a text file count as binary, and a NUL byte cannot hide a `.js` file). Untracked files are never part of the selection. Code, tests, executables, symlinks, submodule commits, gate paths, agent prose, config, `.gstack/` and `.github/` files, gate-tagged files and files with a credential line stay in every reader's diff, each with a reason code. A data file a code or test file names by its full path stays too, and when the gate scan fails nothing is excluded. A basename, a directory reference or a data manifest that lists receipts does not keep anything. -- **`gstack-review-plan` prints `REVIEW_EXCLUDE:`** with the counts, the excluded files by kind, the kept files and a digest. Every reader's `DIFF_CMD:`, the fixture command the security specialist and the Red Team use, and the outside voice's diff leave the files out; kept files come back through `DIFF_KEEP:`. Every brief says what was left out. The kept-file command is bounded at 24,000 argument bytes, and a pattern over that bound is not applied and the plan says why. -- **Excluded lines no longer trigger the numeric-claim scan or the targeted money, gate and auth classes**, and they count as docs for the review depth. -- **The receipt records `review_exclude`.** `gstack-review-dispatch verify` prints `REVIEW_EXCLUDE_RECORD: digest=` from the briefs the readers got, and `gstack-ship-receipt write --review-exclude ` must match the digest HEAD has under the base policy (`RECEIPT_INVALID field=review_exclude`, nothing written). Carry and the queue compare the digest, not the base commit, so a base merge that leaves the policy alone still carries. -- **A review ceiling.** `gstack-review-dispatch pass`, `stage` and `render` read each reader's diff once before writing anything. Over 2,000,000 bytes (about 500K tokens) is `REVIEW_DIFF_TOO_LARGE` (exit 3) with a fix for where the policy stands and pasteable JSON; a `git diff` that fails or runs past 120 seconds is `REVIEW_DIFF_UNREADABLE` (exit 1) with the reason, the timeout and a redacted stderr log. - -### Changed -- A repository without the key plans, briefs and diffs byte for byte as before. The one change it can see is the ceiling. A reader's diff over 2,000,000 bytes now stops the pass instead of being read in part. - -### Tests -- `test/review-exclude.test.ts`: what is excluded and kept (binary files under `config/`, a file named only by a data manifest or by its basename, a file a test names by full path), every brief's diff commands run against git, the numeric scan and targeted classes, the base policy read (no file, a missing blob, a bad commit, an invalid key), the argument-byte bound, both ceiling lines, and the receipt write, carry and queue checks. diff --git a/changelog.d/gst-544-consumer-scan-timeout.md b/changelog.d/gst-544-consumer-scan-timeout.md deleted file mode 100644 index 6a0f9bfeb..000000000 --- a/changelog.d/gst-544-consumer-scan-timeout.md +++ /dev/null @@ -1,8 +0,0 @@ -**The review planner no longer fails closed on repositories with big data trees just because its consumer search ran past 60 seconds (GST-544).** `gstack-review-plan` looks for code that names each changed data file. It used to run one `git grep` over the whole tree with every data file name, and each call had a hard 60-second limit. In garrytan/harness-evals, with 8,000+ result files, that search took about 150 seconds, so the plan fell back to a full review every time. - -### Changed -- **The consumer scan searches only files that can match** (`lib/review-collect.ts`). A cheap first pass lists the files that contain a changed file's extension, such as `.json`; the full search then reads only those files, a batch of paths per `git grep` call, with docs files left out. The result is the same as before. -- **One deadline per scan:** 120 seconds by default, a minute under the 180 seconds /ship gives the whole planner, set in milliseconds with `GSTACK_CONSUMER_SCAN_TIMEOUT_MS` (docs/review-policy.md#review-depth). Past it, or past 256 MB of matches across all the calls, the depth still fails closed with `consumer_scan_failed`. The detail now says whether git timed out, exited with an error or could not start, and names the deadline. Other git reads keep their 60-second limit. - -### Tests -- `test/review-collect.test.ts`: the narrowed search prints the same lines as one whole-tree `git grep` at the head, the merge base and the working tree; the same holds when every file is its own call; consumed sets on a mixed fixture (a chain through a data manifest, an untracked consumer, docs mentions ignored); a tiny deadline fails closed and says it timed out; the default deadline stays a minute under the planner timeout in `gstack-review-dispatch` and `gstack-ship-preflight`. The new behavior tests failed before the fix. diff --git a/changelog.d/gsta224-h1-review-pr-type.md b/changelog.d/gsta224-h1-review-pr-type.md deleted file mode 100644 index dd1ff866f..000000000 --- a/changelog.d/gsta224-h1-review-pr-type.md +++ /dev/null @@ -1,10 +0,0 @@ -**/review now names the kind of PR it is reviewing, so a CI or script change gets checks that fit it instead of a page of "not applicable".** The review checklist was written for application code. A PR that only changed CI or shell scripts came back with every category marked not applicable, which looked thorough but checked nothing, and nothing asked whether a new script even fit the repo (#356: a Windows `.bat` build script added to a bash + Makefile repo with Linux-only CI). - -### Changed -- **`review/checklist.md` starts with a PR type taken from the changed paths:** docs-only, CI/infra, scripts/tooling, application or mixed. Code under `bin/`, `scripts/` or `tools/` that is not shell, batch or PowerShell, package manifests and lockfiles, prompt templates (`*.tmpl`) and skill Markdown count as application, and so does any path the reviewer can't place. Application and mixed diffs run every category exactly as before; nothing is skipped. -- **On a CI/infra, scripts/tooling or docs-only diff,** Gate Integrity, Shell Injection, Completeness Gaps, Distribution & CI/CD Pipeline and the new Repo Conventions & Platform Fit category always run. Every other category with nothing to check is reported once, in one line (`Not applicable (PR type: ): `), and never counts as passed. -- **A new Repo Conventions & Platform Fit category** checks that new files use the languages, tools and naming the repo already uses for the same job, scripts run on the platforms the CI runners and docs target, and new tooling doesn't repeat an existing tool. A finding has to name its concrete consequence. -- **Distribution & CI/CD Pipeline gains script checks:** strict mode (`set -euo pipefail` or the repo's equivalent) in scripts CI or a build relies on, quoted variables, commands that work on the CI runners' OS, third-party actions pinned to a release tag or commit SHA, and tools installed at a pinned version. These checks apply to test-only CI changes too. - -### Tests -- `test/review-checklist-pr-type.test.ts` builds a classifier from the patterns the checklist itself lists and runs real path sets through it, among them a `.tmpl` change, `bin/` TypeScript, an extensionless `bin/` executable, `package.json` + `bun.lock`, a workflow-only change, a `.bat` add, docs, and mixed changes. It also checks the five types, the always-run list, the one-line not-applicable report, the new category and the line budget. All 19 tests fail on the previous checklist. diff --git a/changelog.d/gsta224-review-bots.md b/changelog.d/gsta224-review-bots.md deleted file mode 100644 index 3c87c65d5..000000000 --- a/changelog.d/gsta224-review-bots.md +++ /dev/null @@ -1,13 +0,0 @@ -**/review and /ship now triage comments from every review bot you configured, not only Greptile.** Teams on CodeRabbit, Macroscope or another bot got no triage at all: Step 2.5 and Step 10 read only comments whose author was exactly `greptile-apps[bot]` (#3122). The bot list is the one gstack-pr-wait already uses, so `gstack-config set pr_wait_gating_bots coderabbit` makes CodeRabbit the bot gstack waits on and triages, and `none` in both lists turns triage off. - -### Changed -- **`gstack-review-bots fetch`** reads every root line comment, conversation comment and the newest review body of each bot in `pr_wait_gating_bots` and `pr_wait_advisory_bots`, across all pages, through `gh api` (no `--slurp`, so the gh 2.45 that Ubuntu 24.04 packages works too). Only Bot accounts count, with gstack-pr-wait's login rule (a bot owns the logins that start with its name), so a person named `greptile-ci` is never triaged. It prints `REVIEW_BOTS: skip` (no PR, no `gh`, no GitHub remote, or both lists `none`), `REVIEW_BOTS: error` (a lookup or any page failed, so never a false zero) or a private directory with per-bot counts. -- **False-positive suppressions match only the same bot.** New triage outcomes go to `review-bot-history.md` with a sixth `bot` column through `gstack-review-bots record --dir --id `. The helper reads the bot and file path from the fetched comment, so a contributor-named file never reaches a shell command, and a path with `*` is refused so it can't become a wildcard suppression. Rows in the old `greptile-history.md` still count, as Greptile's. Nothing is migrated or deleted. -- **The PR body section is `## Review bot comments`** with a count per bot, and retro shows one signal row per bot (`REVIEW_BOT_HISTORY` beside `GREPTILE_HISTORY`). -- **Greptile's early PR (Step 6.5) runs only when `greptile` is in one of the two lists**, and detection now needs a Greptile Bot account instead of any login matching `/greptile/i`. Tier 2 replies and re-ranking stay Greptile-only; other bots get the plain evidence reply when you pick it. -- **/ship's driver routes Step 10** to the bot triage section whenever a PR exists, instead of running it only when Greptile is detected. -- **docs/skills.md** has a "Choosing your bots" section. - -### Tests -- `test/review-bots.test.ts` (fake `gh` runner): page-two comments, a CodeRabbit review body, one item from a root comment plus replies, human lookalikes and deleted accounts left out, overlapping bot names, an error on any page giving no directory, lookup failures giving `error`, an empty PR giving zero counts, cross-bot suppression isolation, old greptile-history rows suppressing Greptile only, and `record` refusing hostile paths, colliding ids and unknown categories. -- `test/greptile-early.test.ts`: early detection is off when `greptile` is in neither list, and a human account named like Greptile is no signal. diff --git a/lib/gbrain-local-status.ts b/lib/gbrain-local-status.ts index 7f18a8c46..1dbeb7caa 100644 --- a/lib/gbrain-local-status.ts +++ b/lib/gbrain-local-status.ts @@ -54,6 +54,7 @@ import { atomicWriteSync } from "./fs-atomic"; import { homedir } from "os"; import { dirname, join } from "path"; import { buildGbrainEnv, gbrainChildCwd, gbrainConfigDir, isExecTimeout, NEEDS_SHELL_ON_WINDOWS } from "./gbrain-exec"; +import { classifyGbrainMcpEntry } from "./gbrain-mcp-locality"; import { resolveStateRoot } from "./state-root"; export type LocalEngineStatus = @@ -187,16 +188,11 @@ export function hasRemoteOnlyGbrainMcp( } catch { return false; } - // Same classification rules as gstack-gbrain-detect's detectMcpMode tier 3, - // including the #2051 name generalization (gbrain, gbrain-remote, gbrain_work). - const classify = (entry: McpEntry): "remote" | "local" | null => { - const mtype = entry.type || entry.transport || ""; - if (mtype === "url" || mtype === "http" || mtype === "sse") return "remote"; - if (mtype === "stdio") return "local"; - if (entry.url) return "remote"; - if (entry.command) return "local"; - return null; - }; + // The classifier gstack-gbrain-detect's detectMcpMode uses, so a loopback + // `gbrain serve --http` over this machine's own engine is local here too + // (#3539): detect never prints local-stdio next to thin-client. Name + // generalization (#2051: gbrain, gbrain-remote, gbrain_work) is below. + const classify = (entry: McpEntry): "remote" | "local" | null => classifyGbrainMcpEntry(entry, env ?? process.env); /** Extract the gbrain-relevant entries from an mcpServers object. */ const gbrainEntries = (servers: unknown): Record => { const out: Record = {}; diff --git a/lib/gbrain-mcp-locality.ts b/lib/gbrain-mcp-locality.ts new file mode 100644 index 000000000..559f15fb9 --- /dev/null +++ b/lib/gbrain-mcp-locality.ts @@ -0,0 +1,139 @@ +/** + * gbrain-mcp-locality — is a registered gbrain MCP entry a remote brain? (#3539) + * + * An http/url/sse MCP entry used to mean "Path 4: the brain lives on another + * machine", so transcripts were staged for a brain admin's pull instead of + * going through `gbrain import`. gbrain's own recommended setup for local + * agents on a Postgres brain is one resident `gbrain serve --http` on + * loopback, registered by `gbrain bootstrap harness`. That entry is HTTP but + * the brain is on this machine, and no brain admin will ever index the + * staged pages. + * + * Rule: an HTTP entry is LOCAL when its URL is http(s) on a loopback host AND + * this machine's gbrain config names a Postgres engine (a `database_url`, + * engine not `pglite`) and no `remote_mcp` thin-client marker. Every other + * HTTP entry stays remote, as before. A PGLite config does not count: Path 4's + * split-engine setup pairs a remote brain with a local PGLite for code search, + * and a remote brain reached through a tunnel on a loopback port must stay + * remote there; a PGLite brain held open by a resident serve is engine-locked + * for imports anyway. stdio entries are local, as before. Any + * other URL scheme (ws:, file:) stays remote. The marker test is gbrain's own + * isThinClient() truthiness (`!!remote_mcp`), so `remote_mcp: 0` or `""` is + * not a marker. + * + * File reads only: no subprocess, no network (a classifier network probe is + * the #1964 pathology). + * + * bin/gstack-skill-start carries a jq copy of this rule for its hot path. It + * is a conservative subset, not an exact mirror: its URL regex accepts only + * canonical loopback spellings (localhost, 127.a.b.c without leading zeros, + * [::1]), so whenever jq says local this file says local too, while some + * spellings this file accepts (127.1, 0x7f.1, 127.0.0.1.) stay remote in jq. + * The config half matches exactly. test/gbrain-mcp-locality.test.ts runs both + * copies over the same table and enforces that relation. + */ + +import { readFileSync } from "fs"; +import { join } from "path"; +import { gbrainConfigDir } from "./gbrain-exec"; + +export interface GbrainMcpEntry { + type?: string; + transport?: string; + command?: string; + url?: string; +} + +/** + * True when the URL is http(s) and its host is localhost, 127.0.0.0/8 or ::1. + * A URL with userinfo, or containing `${`, is never loopback: Claude Code + * expands `${VAR}` in `url` before connecting, so `http://${X}@127.0.0.1/mcp` + * can dial any host. + */ +export function isLoopbackMcpUrl(url: string | undefined | null): boolean { + if (typeof url !== "string" || url.trim() === "" || url.includes("${")) return false; + let parsed: URL; + try { + parsed = new URL(url.trim()); + } catch { + return false; + } + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") return false; + if (parsed.username || parsed.password) return false; + const host = parsed.hostname.toLowerCase(); + if (host === "localhost" || host === "::1" || host === "[::1]") return true; + // WHATWG URL normalizes IPv4 forms (127.1, 0x7f.1) to dotted quads. + return /^127\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(host); +} + +/** + * This machine's gbrain config (gbrainConfigDir()/config.json, honoring + * GBRAIN_HOME) names a Postgres engine (a non-blank database_url, engine not + * pglite) and carries no remote_mcp marker (truthy, as in gbrain's + * isThinClient). + */ +export function hasLocalPostgresEngine(env: NodeJS.ProcessEnv = process.env): boolean { + try { + const cfg = JSON.parse(readFileSync(join(gbrainConfigDir(env), "config.json"), "utf-8")) as { + engine?: unknown; + database_url?: unknown; + remote_mcp?: unknown; + } | null; + if (!cfg || typeof cfg !== "object") return false; + if (cfg.remote_mcp) return false; + return cfg.engine !== "pglite" && typeof cfg.database_url === "string" && cfg.database_url.trim() !== ""; + } catch { + return false; + } +} + +/** An HTTP MCP URL that reaches this machine's own Postgres gbrain engine. */ +export function isLocalHttpGbrain(url: string | undefined | null, env: NodeJS.ProcessEnv = process.env): boolean { + return isLoopbackMcpUrl(url) && hasLocalPostgresEngine(env); +} + +/** + * Classify one MCP registration: "remote" for an HTTP brain on another + * machine, "local" for stdio or a loopback HTTP serve of the local engine, + * null when the entry carries no transport signal. + */ +export function classifyGbrainMcpEntry( + entry: GbrainMcpEntry | null | undefined, + env: NodeJS.ProcessEnv = process.env, +): "local" | "remote" | null { + if (!entry || typeof entry !== "object") return null; + // Same precedence as the transport-only rule this replaces: explicit HTTP + // type, explicit stdio, then a bare url, then a bare command. + const mtype = entry.type || entry.transport || ""; + const remoteUnlessLoopback = (): "local" | "remote" => (isLocalHttpGbrain(entry.url, env) ? "local" : "remote"); + if (mtype === "url" || mtype === "http" || mtype === "sse") return remoteUnlessLoopback(); + if (mtype === "stdio") return "local"; + if (entry.url) return remoteUnlessLoopback(); + if (entry.command) return "local"; + return null; +} + +/** + * Classify the gbrain line of `claude mcp list`. HTTP and SSE servers print + * as `gbrain: (HTTP) - ✓ Connected` (or `(SSE)`); stdio servers print + * their command with no transport marker. + * + * The URL token is everything between the name and the LAST ` (HTTP)` / + * ` (SSE)` marker. It is never cut at `)`: a cut turns + * `http://127.0.0.1)@evil.com/mcp` (host evil.com) into `http://127.0.0.1`. + * A token containing whitespace, or one that is not a loopback http(s) URL + * over a local engine, is remote. A line that names HTTP/SSE but has no + * such token is remote, as before; a line with neither is stdio (local). + */ +export function classifyGbrainMcpListLine( + line: string, + env: NodeJS.ProcessEnv = process.env, +): "local" | "remote" { + const m = /^gbrain(?:[-_][\w-]*)?:\s+(.+)\s+\((?:HTTP|SSE)\)/.exec(line); + if (m) { + const token = m[1]; + return !/\s/.test(token) && isLocalHttpGbrain(token, env) ? "local" : "remote"; + } + if (/\b(?:http|HTTP)\b/.test(line) || /\((?:SSE|sse)\)/.test(line)) return "remote"; + return "local"; +} diff --git a/lib/ship-policy.ts b/lib/ship-policy.ts index d7dbefd83..0092b3be4 100644 --- a/lib/ship-policy.ts +++ b/lib/ship-policy.ts @@ -236,7 +236,7 @@ export const BUMP_NEVER_KEY_VERSION = '1.91.353.0'; export const BASE_RED_KEY_VERSION = '1.91.374.0'; /** `train.gates` and `train.land` are values of the older train key, so compatErrors checks them by value, as for bump: never. */ export const TRAIN_GATES_KEY_VERSION = '1.92.0.0'; -export const REVIEW_EXCLUDE_KEY_VERSION = '1.94.0.0'; +export const REVIEW_EXCLUDE_KEY_VERSION = '1.93.33.0'; export const POLICY_KEY_INTRODUCED: Record = { bump: '1.91.73.0', queue_mode: '1.91.73.0', release_tool: '1.91.73.0', release_outputs: '1.91.73.0', stamp_paths: '1.91.73.0', changelog: '1.91.73.0', mirror: '1.91.73.0', history: '1.91.73.0', preregistration_shas: '1.91.73.0', stamp_sensitive_gates: '1.91.73.0', diff --git a/package.json b/package.json index 048a570d4..2444dbcd8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.93.27", + "version": "1.93.33", "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/setup b/setup index 5bd5937ff..81acc11a4 100755 --- a/setup +++ b/setup @@ -641,10 +641,15 @@ if [ "$CODEX_REPO_LOCAL" -eq 1 ]; then "$_codex_project"|"$_codex_project"/*) ;; *) echo "Refusing: project-local Codex destination resolves outside the project: $CODEX_SKILLS -> $_codex_parent. Use a separate project-local skills directory; setup will not relink it automatically." >&2; exit 1 ;; esac + _codex_self_host=0 + if [ "$_codex_project" = "$SOURCE_GSTACK_DIR" ] && [ "$_codex_parent" = "$SOURCE_GSTACK_DIR/.agents/skills" ]; then + _codex_self_host=1 + fi _codex_payload="$_codex_parent" while :; do - if [ "$_codex_payload" = "$SOURCE_GSTACK_DIR" ] \ - || { [ -f "$_codex_payload/setup" ] && [ -f "$_codex_payload/VERSION" ] && [ -f "$_codex_payload/bin/gstack-relink" ]; }; then + if { [ "$_codex_payload" = "$SOURCE_GSTACK_DIR" ] \ + || { [ -f "$_codex_payload/setup" ] && [ -f "$_codex_payload/VERSION" ] && [ -f "$_codex_payload/bin/gstack-relink" ]; }; } \ + && ! { [ "$_codex_self_host" -eq 1 ] && [ "$_codex_payload" = "$SOURCE_GSTACK_DIR" ]; }; then echo "Refusing: project-local Codex destination overlaps the source tree at $_codex_payload: $CODEX_SKILLS -> $_codex_parent. Use a separate project-local skills directory; setup will not relink it automatically." >&2 exit 1 fi diff --git a/test/cso-git-hardening.test.ts b/test/cso-git-hardening.test.ts index 7c6d3eb96..99021e7d4 100644 --- a/test/cso-git-hardening.test.ts +++ b/test/cso-git-hardening.test.ts @@ -9,7 +9,7 @@ import { childEnvironment, executable, git as readGitMetadata, runProcess } from const roots:string[]=[]; afterEach(()=>{for(const root of roots.splice(0))fs.rmSync(root,{recursive:true,force:true});}); -const fixtureGit=(repo:string,home:string)=>(...args:string[])=>{const result=spawnSync('/usr/bin/git',['-C',repo,...args],{encoding:'utf8',env:{HOME:home,PATH:'/usr/bin:/bin'},timeout:30_000});if(result.status)throw new Error(result.stderr);return result.stdout;}; +const fixtureGit=(repo:string,home:string)=>(...args:string[])=>{const result=spawnSync('/usr/bin/git',['-C',repo,...args],{encoding:'utf8',env:{HOME:home,PATH:'/usr/bin:/bin'},timeout:30_000});if(result.error||result.status!==0)throw new Error(`fixture git ${args.join(' ')} failed (status ${result.status}, signal ${result.signal}): ${result.error?.message??result.stderr}`);return result.stdout;}; // Each fixture builds its own repository from Git; nothing is copied from a shared // template (Free Tests 40288142343 shard 17: fs.cpSync of the template threw inside // its directory walk once and passed on the serial retry). Auto-maintenance is off @@ -25,7 +25,9 @@ function seedRepo(repo:string,home:string){ return git; } // The first /usr/bin/git spawns on a cold macOS runner can eat a test's whole 5 s budget before the code under test runs; pay them once here. +// Every test below skips on Windows, which has no /usr/bin/git, so the warm-up does too. beforeAll(()=>{ + if(process.platform==='win32')return; const warm=fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(),'cso-git-hardening-warm-'))); try{seedRepo(path.join(warm,'repo'),warm);}finally{fs.rmSync(warm,{recursive:true,force:true});} },{timeout:30_000}); diff --git a/test/cso-snapshot-state.test.ts b/test/cso-snapshot-state.test.ts index 417d3d921..a18799a73 100644 --- a/test/cso-snapshot-state.test.ts +++ b/test/cso-snapshot-state.test.ts @@ -8,6 +8,7 @@ function repo(){const p=tmp();git(p,'init','-q');git(p,'config','user.email','fi async function waitForPath(file:string,timeout=5000):Promise{const deadline=Date.now()+timeout;while(!fs.existsSync(file)){if(Date.now()>=deadline)throw new Error(`Timed out waiting for ${path.basename(file)}`);await Bun.sleep(5);}} function childExit(child:ReturnType,timeout=5000):Promise{return new Promise((resolve,reject)=>{if(child.exitCode!==null){resolve(child.exitCode);return;}const timer=setTimeout(()=>reject(new Error(`Child ${child.pid} did not exit`)),timeout);child.once('error',error=>{clearTimeout(timer);reject(error);});child.once('close',code=>{clearTimeout(timer);resolve(code);});});} function stopChildren(children:Array>):void{for(const child of children){if(!child.pid||child.exitCode!==null)continue;try{process.kill(child.pid,'SIGCONT');}catch{}try{process.kill(child.pid,'SIGKILL');}catch{}}} +async function resumeStopped(child:ReturnType,timeout=2000):Promise{const deadline=Date.now()+timeout;for(;;){if(child.exitCode!==null||child.signalCode!==null)throw new Error(`Child ${child.pid} exited (${child.exitCode??child.signalCode}) after marking its pause, before it stopped`);let state='';if(process.platform==='linux'){try{const stat=fs.readFileSync(`/proc/${child.pid}/stat`,'utf8');state=stat.slice(stat.lastIndexOf(')')+2);}catch(error){if((error as NodeJS.ErrnoException).code!=='ENOENT')throw new Error(`Child ${child.pid} stop check failed: ${(error as Error).message}`);}}else{const ps=spawnSync('ps',['-o','stat=','-p',String(child.pid)],{encoding:'utf8',timeout:2000});if(ps.error)throw new Error(`Child ${child.pid} stop check failed: ${ps.error.message}`);state=(ps.stdout??'').trim();}if(state[0]==='T')break;if(Date.now()>=deadline)throw new Error(`Child ${child.pid} marked its pause but never stopped`);await Bun.sleep(5);}process.kill(child.pid!,'SIGCONT');} function leaseContenderSource():string{return `import fs from 'node:fs';import * as fsModule from 'node:fs';import * as cryptoModule from 'node:crypto';import path from 'node:path';import {spyOn} from 'bun:test'; const dir=process.env.RACE_DIR!,barrier=process.env.BARRIER!,id=process.env.RACE_ID!,mode=process.env.RACE_MODE??'normal',token=process.env.RACE_TOKEN,leases=path.join(dir,'.mutation-lock-leases'),wait=new Int32Array(new SharedArrayBuffer(4));let triggered=false; const write=fs.writeFileSync.bind(fs),mark=(name,value='ready')=>write(path.join(barrier,name),value,{flag:'wx'}),waitFor=(name)=>{const deadline=Date.now()+5000;while(!fs.existsSync(path.join(barrier,name))){if(Date.now()>=deadline)throw new Error('barrier timeout: '+name);Atomics.wait(wait,0,0,1);}}; @@ -240,15 +241,15 @@ let outcome='success';try{withLock(dir,()=>{const active=path.join(barrier,'acti try{ const low=spawn(process.execPath,[script],{env:{...env,RACE_ID:'L',RACE_MODE:'pause-before-decision',RACE_TOKEN:lowToken,RACE_HOLD:'1'},stdio:'ignore'});children.push(low);const lowDone=childExit(low);await waitForPath(path.join(barrier,'paused-L'));expect(fs.readFileSync(path.join(barrier,'choice-L'),'utf8')).toBe('0000000000000001'); const high=spawn(process.execPath,[script],{env:{...env,RACE_ID:'H',RACE_MODE:'pause-before-withdraw',RACE_TOKEN:highToken},stdio:'ignore'});children.push(high);const highDone=childExit(high);await waitForPath(path.join(barrier,'paused-H'),2500);expect(fs.readFileSync(path.join(barrier,'choice-H'),'utf8')).toBe('0000000000000001'); - process.kill(low.pid!,'SIGCONT');await waitForPath(path.join(barrier,'entered-L'));const decision=JSON.parse(fs.readFileSync(path.join(dir,'.mutation-lock-leases',lowToken+'.decision'),'utf8'));expect(decision.kind).toBe('ticket');expect(decision.ticket).toBe('0000000000000001'); - process.kill(high.pid!,'SIGCONT');await waitForPath(path.join(barrier,'result-H'));expect(fs.readFileSync(path.join(barrier,'result-H'),'utf8')).toBe('INSUFFICIENT_CAPACITY');expect(await highDone).toBe(0); + await resumeStopped(low);await waitForPath(path.join(barrier,'entered-L'));const decision=JSON.parse(fs.readFileSync(path.join(dir,'.mutation-lock-leases',lowToken+'.decision'),'utf8'));expect(decision.kind).toBe('ticket');expect(decision.ticket).toBe('0000000000000001'); + await resumeStopped(high);await waitForPath(path.join(barrier,'result-H'));expect(fs.readFileSync(path.join(barrier,'result-H'),'utf8')).toBe('INSUFFICIENT_CAPACITY');expect(await highDone).toBe(0); fs.writeFileSync(path.join(barrier,'release-L'),'release');await waitForPath(path.join(barrier,'result-L'));expect(fs.readFileSync(path.join(barrier,'result-L'),'utf8')).toBe('success');expect(await lowDone).toBe(0);expect(fs.existsSync(path.join(barrier,'overlap'))).toBe(false);expect(fs.readdirSync(path.join(dir,'.mutation-lock-leases'))).toEqual([]); }finally{stopChildren(children);} }); - test('a paused candidate is fenced so a progressing ticket wins within the bound',async()=>{if(process.platform==='win32')return;const dir=tmp(),barrier=path.join(dir,'barrier'),script=path.join(dir,'contender.ts');fs.mkdirSync(barrier);expect(withLock(dir,()=>1)).toBe(1);fs.writeFileSync(script,leaseContenderSource());const env={...process.env,RACE_DIR:dir,BARRIER:barrier},children:Array>=[];try{const earlier=spawn(process.execPath,[script],{env:{...env,RACE_ID:'L',RACE_MODE:'pause-candidate'},stdio:'ignore'});children.push(earlier);const earlierDone=childExit(earlier);await waitForPath(path.join(barrier,'paused-L'));const started=Date.now(),later=spawn(process.execPath,[script],{env:{...env,RACE_ID:'H',RACE_MODE:'normal'},stdio:'ignore'});children.push(later);const laterDone=childExit(later);await waitForPath(path.join(barrier,'result-H'),2500);expect(Date.now()-started).toBeLessThan(2000);expect(fs.readFileSync(path.join(barrier,'result-H'),'utf8')).toBe('success');expect(await laterDone).toBe(0);process.kill(earlier.pid!,'SIGCONT');await waitForPath(path.join(barrier,'result-L'));expect(fs.readFileSync(path.join(barrier,'result-L'),'utf8')).toBe('INSUFFICIENT_CAPACITY');expect(await earlierDone).toBe(0);expect(fs.existsSync(path.join(barrier,'overlap'))).toBe(false);expect(fs.readdirSync(path.join(dir,'.mutation-lock-leases'))).toEqual([]);}finally{stopChildren(children);}}); - test('a paused earlier ticket bounds later contenders without withdrawing the winner',async()=>{if(process.platform==='win32')return;const dir=tmp(),barrier=path.join(dir,'barrier'),script=path.join(dir,'contender.ts');fs.mkdirSync(barrier);expect(withLock(dir,()=>1)).toBe(1);fs.writeFileSync(script,leaseContenderSource());const env={...process.env,RACE_DIR:dir,BARRIER:barrier},children:Array>=[];try{const earlier=spawn(process.execPath,[script],{env:{...env,RACE_ID:'L',RACE_MODE:'pause-ticket'},stdio:'ignore'});children.push(earlier);const earlierDone=childExit(earlier);await waitForPath(path.join(barrier,'paused-L'));const started=Date.now(),later=spawn(process.execPath,[script],{env:{...env,RACE_ID:'H',RACE_MODE:'normal'},stdio:'ignore'});children.push(later);const laterDone=childExit(later);await waitForPath(path.join(barrier,'result-H'),2500);expect(Date.now()-started).toBeLessThan(2000);expect(fs.readFileSync(path.join(barrier,'result-H'),'utf8')).toBe('INSUFFICIENT_CAPACITY');expect(await laterDone).toBe(0);process.kill(earlier.pid!,'SIGCONT');await waitForPath(path.join(barrier,'result-L'));expect(fs.readFileSync(path.join(barrier,'result-L'),'utf8')).toBe('success');expect(await earlierDone).toBe(0);expect(fs.existsSync(path.join(barrier,'overlap'))).toBe(false);expect(fs.readdirSync(path.join(dir,'.mutation-lock-leases'))).toEqual([]);}finally{stopChildren(children);}}); - test('a paused loser cleanup leaves only an ineligible decision and cannot block the winner',async()=>{if(process.platform==='win32')return;const dir=tmp(),barrier=path.join(dir,'barrier'),script=path.join(dir,'contender.ts');fs.mkdirSync(barrier);expect(withLock(dir,()=>1)).toBe(1);fs.writeFileSync(script,leaseContenderSource());const env={...process.env,RACE_DIR:dir,BARRIER:barrier},children:Array>=[];try{const winner=spawn(process.execPath,[script],{env:{...env,RACE_ID:'L',RACE_MODE:'pause-ticket'},stdio:'ignore'});children.push(winner);const winnerDone=childExit(winner);await waitForPath(path.join(barrier,'paused-L'));const loser=spawn(process.execPath,[script],{env:{...env,RACE_ID:'H',RACE_MODE:'pause-withdraw-cleanup'},stdio:'ignore'});children.push(loser);const loserDone=childExit(loser);await waitForPath(path.join(barrier,'paused-H'));const phases=fs.readdirSync(path.join(dir,'.mutation-lock-leases'));expect(phases.filter(name=>name.endsWith('.decision')).length).toBe(2);expect(phases.filter(name=>name.endsWith('.json')).length).toBe(1);process.kill(winner.pid!,'SIGCONT');await waitForPath(path.join(barrier,'result-L'));expect(fs.readFileSync(path.join(barrier,'result-L'),'utf8')).toBe('success');expect(await winnerDone).toBe(0);process.kill(loser.pid!,'SIGCONT');await waitForPath(path.join(barrier,'result-H'));expect(fs.readFileSync(path.join(barrier,'result-H'),'utf8')).toBe('INSUFFICIENT_CAPACITY');expect(await loserDone).toBe(0);expect(fs.existsSync(path.join(barrier,'overlap'))).toBe(false);expect(fs.readdirSync(path.join(dir,'.mutation-lock-leases'))).toEqual([]);}finally{stopChildren(children);}}); - test('a paused later contender cannot turn a cached lower ticket into an all-loser outcome',async()=>{if(process.platform==='win32')return;const dir=tmp(),barrier=path.join(dir,'barrier'),script=path.join(dir,'contender.ts');fs.mkdirSync(barrier);expect(withLock(dir,()=>1)).toBe(1);fs.writeFileSync(script,leaseContenderSource());const env={...process.env,RACE_DIR:dir,BARRIER:barrier},children:Array>=[];try{const earlier=spawn(process.execPath,[script],{env:{...env,RACE_ID:'L',RACE_MODE:'wait-active',RACE_HOLD:'1'},stdio:'ignore'});children.push(earlier);const earlierDone=childExit(earlier);await waitForPath(path.join(barrier,'doorway-L'));const later=spawn(process.execPath,[script],{env:{...env,RACE_ID:'H',RACE_MODE:'pause-later-scan'},stdio:'ignore'});children.push(later);const laterDone=childExit(later);await waitForPath(path.join(barrier,'paused-H'));await waitForPath(path.join(barrier,'entered-L'));process.kill(later.pid!,'SIGCONT');await waitForPath(path.join(barrier,'result-H'),2500);expect(fs.readFileSync(path.join(barrier,'result-H'),'utf8')).toBe('INSUFFICIENT_CAPACITY');expect(await laterDone).toBe(0);fs.writeFileSync(path.join(barrier,'release-L'),'release');await waitForPath(path.join(barrier,'result-L'));expect(fs.readFileSync(path.join(barrier,'result-L'),'utf8')).toBe('success');expect(await earlierDone).toBe(0);expect(fs.existsSync(path.join(barrier,'overlap'))).toBe(false);expect(fs.readdirSync(path.join(dir,'.mutation-lock-leases'))).toEqual([]);}finally{stopChildren(children);}}); + test('a paused candidate is fenced so a progressing ticket wins within the bound',async()=>{if(process.platform==='win32')return;const dir=tmp(),barrier=path.join(dir,'barrier'),script=path.join(dir,'contender.ts');fs.mkdirSync(barrier);expect(withLock(dir,()=>1)).toBe(1);fs.writeFileSync(script,leaseContenderSource());const env={...process.env,RACE_DIR:dir,BARRIER:barrier},children:Array>=[];try{const earlier=spawn(process.execPath,[script],{env:{...env,RACE_ID:'L',RACE_MODE:'pause-candidate'},stdio:'ignore'});children.push(earlier);const earlierDone=childExit(earlier);await waitForPath(path.join(barrier,'paused-L'));const started=Date.now(),later=spawn(process.execPath,[script],{env:{...env,RACE_ID:'H',RACE_MODE:'normal'},stdio:'ignore'});children.push(later);const laterDone=childExit(later);await waitForPath(path.join(barrier,'result-H'),2500);expect(Date.now()-started).toBeLessThan(2000);expect(fs.readFileSync(path.join(barrier,'result-H'),'utf8')).toBe('success');expect(await laterDone).toBe(0);await resumeStopped(earlier);await waitForPath(path.join(barrier,'result-L'));expect(fs.readFileSync(path.join(barrier,'result-L'),'utf8')).toBe('INSUFFICIENT_CAPACITY');expect(await earlierDone).toBe(0);expect(fs.existsSync(path.join(barrier,'overlap'))).toBe(false);expect(fs.readdirSync(path.join(dir,'.mutation-lock-leases'))).toEqual([]);}finally{stopChildren(children);}}); + test('a paused earlier ticket bounds later contenders without withdrawing the winner',async()=>{if(process.platform==='win32')return;const dir=tmp(),barrier=path.join(dir,'barrier'),script=path.join(dir,'contender.ts');fs.mkdirSync(barrier);expect(withLock(dir,()=>1)).toBe(1);fs.writeFileSync(script,leaseContenderSource());const env={...process.env,RACE_DIR:dir,BARRIER:barrier},children:Array>=[];try{const earlier=spawn(process.execPath,[script],{env:{...env,RACE_ID:'L',RACE_MODE:'pause-ticket'},stdio:'ignore'});children.push(earlier);const earlierDone=childExit(earlier);await waitForPath(path.join(barrier,'paused-L'));const started=Date.now(),later=spawn(process.execPath,[script],{env:{...env,RACE_ID:'H',RACE_MODE:'normal'},stdio:'ignore'});children.push(later);const laterDone=childExit(later);await waitForPath(path.join(barrier,'result-H'),2500);expect(Date.now()-started).toBeLessThan(2000);expect(fs.readFileSync(path.join(barrier,'result-H'),'utf8')).toBe('INSUFFICIENT_CAPACITY');expect(await laterDone).toBe(0);await resumeStopped(earlier);await waitForPath(path.join(barrier,'result-L'));expect(fs.readFileSync(path.join(barrier,'result-L'),'utf8')).toBe('success');expect(await earlierDone).toBe(0);expect(fs.existsSync(path.join(barrier,'overlap'))).toBe(false);expect(fs.readdirSync(path.join(dir,'.mutation-lock-leases'))).toEqual([]);}finally{stopChildren(children);}}); + test('a paused loser cleanup leaves only an ineligible decision and cannot block the winner',async()=>{if(process.platform==='win32')return;const dir=tmp(),barrier=path.join(dir,'barrier'),script=path.join(dir,'contender.ts');fs.mkdirSync(barrier);expect(withLock(dir,()=>1)).toBe(1);fs.writeFileSync(script,leaseContenderSource());const env={...process.env,RACE_DIR:dir,BARRIER:barrier},children:Array>=[];try{const winner=spawn(process.execPath,[script],{env:{...env,RACE_ID:'L',RACE_MODE:'pause-ticket'},stdio:'ignore'});children.push(winner);const winnerDone=childExit(winner);await waitForPath(path.join(barrier,'paused-L'));const loser=spawn(process.execPath,[script],{env:{...env,RACE_ID:'H',RACE_MODE:'pause-withdraw-cleanup'},stdio:'ignore'});children.push(loser);const loserDone=childExit(loser);await waitForPath(path.join(barrier,'paused-H'));const phases=fs.readdirSync(path.join(dir,'.mutation-lock-leases'));expect(phases.filter(name=>name.endsWith('.decision')).length).toBe(2);expect(phases.filter(name=>name.endsWith('.json')).length).toBe(1);await resumeStopped(winner);await waitForPath(path.join(barrier,'result-L'));expect(fs.readFileSync(path.join(barrier,'result-L'),'utf8')).toBe('success');expect(await winnerDone).toBe(0);await resumeStopped(loser);await waitForPath(path.join(barrier,'result-H'));expect(fs.readFileSync(path.join(barrier,'result-H'),'utf8')).toBe('INSUFFICIENT_CAPACITY');expect(await loserDone).toBe(0);expect(fs.existsSync(path.join(barrier,'overlap'))).toBe(false);expect(fs.readdirSync(path.join(dir,'.mutation-lock-leases'))).toEqual([]);}finally{stopChildren(children);}}); + test('a paused later contender cannot turn a cached lower ticket into an all-loser outcome',async()=>{if(process.platform==='win32')return;const dir=tmp(),barrier=path.join(dir,'barrier'),script=path.join(dir,'contender.ts');fs.mkdirSync(barrier);expect(withLock(dir,()=>1)).toBe(1);fs.writeFileSync(script,leaseContenderSource());const env={...process.env,RACE_DIR:dir,BARRIER:barrier},children:Array>=[];try{const earlier=spawn(process.execPath,[script],{env:{...env,RACE_ID:'L',RACE_MODE:'wait-active',RACE_HOLD:'1'},stdio:'ignore'});children.push(earlier);const earlierDone=childExit(earlier);await waitForPath(path.join(barrier,'doorway-L'));const later=spawn(process.execPath,[script],{env:{...env,RACE_ID:'H',RACE_MODE:'pause-later-scan'},stdio:'ignore'});children.push(later);const laterDone=childExit(later);await waitForPath(path.join(barrier,'paused-H'));await waitForPath(path.join(barrier,'entered-L'));await resumeStopped(later);await waitForPath(path.join(barrier,'result-H'),2500);expect(fs.readFileSync(path.join(barrier,'result-H'),'utf8')).toBe('INSUFFICIENT_CAPACITY');expect(await laterDone).toBe(0);fs.writeFileSync(path.join(barrier,'release-L'),'release');await waitForPath(path.join(barrier,'result-L'));expect(fs.readFileSync(path.join(barrier,'result-L'),'utf8')).toBe('success');expect(await earlierDone).toBe(0);expect(fs.existsSync(path.join(barrier,'overlap'))).toBe(false);expect(fs.readdirSync(path.join(dir,'.mutation-lock-leases'))).toEqual([]);}finally{stopChildren(children);}}); test('legacy report imports expire after thirty days without following links',()=>{const old=process.env.GSTACK_HOME,base=tmp();process.env.GSTACK_HOME=base;try{const archive=secureDirectory(path.join(base,'security','cso','legacy-imports')),expired=path.join(archive,'a'.repeat(64)+'.json'),retained=path.join(archive,'b'.repeat(64)+'.json');fs.writeFileSync(expired,'{}');fs.writeFileSync(retained,'{}');const now=Date.now();fs.utimesSync(expired,new Date(now-31*86400_000),new Date(now-31*86400_000));retention(now);expect(fs.existsSync(expired)).toBe(false);expect(fs.existsSync(retained)).toBe(true);const unsafe=path.join(archive,'c'.repeat(64)+'.json');fs.symlinkSync(retained,unsafe);expect(()=>retention(now)).toThrow('unsafe artifact');expect(fs.readFileSync(retained,'utf8')).toBe('{}');}finally{if(old===undefined)delete process.env.GSTACK_HOME;else process.env.GSTACK_HOME=old;}}); test('an active recheck pins only its expired parent report, never expired repair material',()=>{const now=Date.now(),repo='f'.repeat(24),oldRun=`${now-31*86400_000}-${'a'.repeat(16)}`,childRun=`${now}-${'b'.repeat(16)}`,findingId='c'.repeat(32),repoDir=secureDirectory(path.join(privateRoot(),repo)),parent=secureDirectory(path.join(repoDir,oldRun)),child=secureDirectory(path.join(repoDir,childRun)),bundles=secureDirectory(path.join(parent,'bundles')),reviews=secureDirectory(path.join(parent,'reviews')),attempts=secureDirectory(path.join(parent,'verification-attempts'));writeHelperJson(path.join(parent,'report.json'),{schemaVersion:3,runId:oldRun,repoId:repo,status:'finished',coverage:[],findings:[{id:findingId}]});writeJson(path.join(bundles,'bundle.json'),{schemaVersion:3});writeJson(path.join(reviews,'review.json'),{schemaVersion:3});writeJson(path.join(attempts,'attempt.json'),{schemaVersion:3});writeHelperJson(path.join(child,'report.json'),{schemaVersion:3,runId:childRun,repoId:repo,status:'running',deadline:new Date(now+60_000).toISOString(),coverage:[],findings:[],parent:{runId:oldRun,findingId,kind:'recheck'}});retention(now);expect(fs.existsSync(path.join(parent,'report.json'))).toBe(true);expect(fs.existsSync(bundles)).toBe(false);expect(fs.existsSync(reviews)).toBe(false);expect(fs.existsSync(attempts)).toBe(false);fs.rmSync(child,{recursive:true});retention(now);expect(fs.existsSync(parent)).toBe(false);}); test('finished, expired, and malformed child reports cannot extend parent retention',()=>{const now=Date.now(),repo='e'.repeat(24),repoDir=secureDirectory(path.join(privateRoot(),repo)),findingId='d'.repeat(32);for(const [index,childValue] of [[0,{status:'finished',repoId:repo,deadline:new Date(now+60_000).toISOString()}],[1,{status:'running',repoId:repo,deadline:new Date(now-1).toISOString()}],[2,{status:'running',repoId:'0'.repeat(24),deadline:new Date(now+60_000).toISOString()}]] as const){const parentRun=`${now-(31+index)*86400_000}-${String(index+1).repeat(16)}`,childRun=`${now-index}-${String(index+4).repeat(16)}`,parent=secureDirectory(path.join(repoDir,parentRun)),child=secureDirectory(path.join(repoDir,childRun));writeHelperJson(path.join(parent,'report.json'),{schemaVersion:3,runId:parentRun,repoId:repo,status:'finished',coverage:[],findings:[{id:findingId}]});writeHelperJson(path.join(child,'report.json'),{schemaVersion:3,runId:childRun,repoId:childValue.repoId,status:childValue.status,deadline:childValue.deadline,coverage:[],findings:[],parent:{runId:parentRun,findingId,kind:'recheck'}});}retention(now);for(const name of fs.readdirSync(repoDir).filter(name=>Number(name.split('-')[0])) => put(join(f.commands, 'bun'), `#!/usr/bin/env bash +case "$*" in + install|'install --production --frozen-lockfile') exit 0 ;; + 'build --help') echo 'Fixture Bun has no CSO compile flags'; exit 0 ;; + 'run build') echo 'Unexpected build in a dev-setup fixture' >&2; exit 90 ;; + *) exec '${process.execPath}' "$@" ;; +esac +`, 0o755); + +const freshClone = (rel = 'gstack') => { + const f = makeFixture(); + stubBun(f); + const source = makeSource(f, join(f.dir, rel)); + const init = spawnSync('git', ['init', '-q', source], { encoding: 'utf8', timeout: 30_000 }); + if (init.status !== 0) throw new Error(init.stderr); + return { f, source }; +}; + +describe.skipIf(process.platform === 'win32')('bin/dev-setup in a fresh gstack clone', () => { + test('links gstack into its own .claude and .agents skills and exits 0', () => { + const { f, source } = freshClone(); + for (let run = 0; run < 2; run++) { + const result = spawnSync('bash', [join(source, 'bin/dev-setup')], { cwd: source, env: f.env, encoding: 'utf8', timeout: 120_000, input: '' }); + expect(result.stderr).not.toContain('overlaps the source tree'); + expect(result.status, result.stdout + result.stderr).toBe(0); + expect(readlinkSync(join(source, '.claude/skills/gstack'))).toBe(source); + expect(readlinkSync(join(source, '.agents/skills/gstack'))).toBe(source); + } + }, 240_000); + + test('a user project whose .agents/skills resolves to the checkout\'s own .agents/skills is still refused', () => { + const { f, source } = freshClone('project/vendor/gstack'); + const project = join(f.dir, 'project'); + fixtureMkdirSync(join(source, '.agents/skills'), { recursive: true }); + fixtureMkdirSync(join(project, '.claude/skills'), { recursive: true }); + fixtureMkdirSync(join(project, '.agents')); + symlinkSync(source, join(project, '.claude/skills/gstack'), 'dir'); + symlinkSync(join(source, '.agents/skills'), join(project, '.agents/skills'), 'dir'); + const before = tree(f.dir); + const result = spawnSync('bash', [join(project, '.claude/skills/gstack/setup'), '--host', 'codex', '--no-team', '--no-plan-tune-hooks', '--no-timeline-stop-hook'], { + cwd: project, env: f.env, encoding: 'utf8', timeout: 60_000, input: '', + }); + expect(result.status, result.stdout + result.stderr).toBe(1); + expect(result.stderr).toContain(`overlaps the source tree at ${source}`); + expect(tree(f.dir)).toEqual(before); + }, 90_000); + + test('the checkout itself is still refused when its .agents/skills aliases the source root', () => { + const { f, source } = freshClone(); + fixtureMkdirSync(join(source, '.claude/skills'), { recursive: true }); + fixtureMkdirSync(join(source, '.agents')); + symlinkSync(source, join(source, '.claude/skills/gstack'), 'dir'); + symlinkSync(source, join(source, '.agents/skills'), 'dir'); + const before = tree(f.dir); + const result = spawnSync('bash', [join(source, '.claude/skills/gstack/setup'), '--host', 'codex', '--no-team', '--no-plan-tune-hooks', '--no-timeline-stop-hook'], { + cwd: source, env: f.env, encoding: 'utf8', timeout: 60_000, input: '', + }); + expect(result.status, result.stdout + result.stderr).toBe(1); + expect(result.stderr).toContain('overlaps the source tree'); + expect(tree(f.dir)).toEqual(before); + }, 90_000); + + test('setup --host codex from the checkout\'s own layout is still refused by the Codex preflight, without writes', () => { + const { f, source } = freshClone(); + const devSetup = spawnSync('bash', [join(source, 'bin/dev-setup')], { cwd: source, env: f.env, encoding: 'utf8', timeout: 120_000, input: '' }); + expect(devSetup.status, devSetup.stdout + devSetup.stderr).toBe(0); + const before = tree(f.dir); + const result = spawnSync('bash', [join(source, '.claude/skills/gstack/setup'), '--host', 'codex', '--no-team', '--no-plan-tune-hooks', '--no-timeline-stop-hook'], { + cwd: source, env: f.env, encoding: 'utf8', timeout: 60_000, input: '', + }); + expect(result.status, result.stdout + result.stderr).toBe(1); + expect(result.stderr).toContain(`Refusing: Codex sidecar root write overlaps source or escapes its namespace: ${join(source, '.agents/skills/gstack')}`); + expect(tree(f.dir)).toEqual(before); + }, 240_000); +}); diff --git a/test/gbrain-local-status.test.ts b/test/gbrain-local-status.test.ts index 9d666a35e..038f66203 100644 --- a/test/gbrain-local-status.test.ts +++ b/test/gbrain-local-status.test.ts @@ -760,6 +760,50 @@ describe("lib/gbrain-local-status — bearer-token thin-client (#2520)", () => { expect(localEngineStatus({ noCache: true })).toBe("engine-locked"); }); + // #3539: a loopback `gbrain serve --http` over this machine's own engine is + // a local registration. Before, the transport-only copy of the rule called + // it remote, so detect printed gbrain_mcp_mode local-stdio next to + // gbrain_local_status thin-client for the same entry. + it("keeps the local engine's own status for a loopback http gbrain MCP over a local Postgres (#3539)", () => { + env = makeEnv({ + withGbrain: true, + gbrainBehavior: "engine-locked-v43", + withConfig: true, + claudeJson: { mcpServers: { gbrain: { type: "http", url: "http://127.0.0.1:3131/mcp" } } }, + }); + restoreEnv = applyEnv(env); + writeFileSync(env.configPath, JSON.stringify({ engine: "postgres", database_url: "postgres://fake" })); + expect(localEngineStatus({ noCache: true })).toBe("broken-db"); + }); + + it("still reclassifies a locked local PGLite as 'thin-client' behind a loopback http entry (Path 4 split-engine, #3539)", () => { + env = makeEnv({ + withGbrain: true, + gbrainBehavior: "engine-locked-v43", + withConfig: true, + claudeJson: { mcpServers: { gbrain: { type: "http", url: "http://127.0.0.1:3131/mcp" } } }, + }); + restoreEnv = applyEnv(env); + expect(localEngineStatus({ noCache: true })).toBe("thin-client"); + }); + + it("still reclassifies as 'thin-client' when the loopback entry's config carries remote_mcp (#3539 guard)", () => { + env = makeEnv({ + withGbrain: true, + gbrainBehavior: "engine-locked", + withConfig: true, + claudeJson: { mcpServers: { gbrain: { type: "http", url: "http://127.0.0.1:3131/mcp" } } }, + }); + restoreEnv = applyEnv(env); + writeFileSync( + env.configPath, + JSON.stringify({ engine: "pglite", remote_mcp: { mcp_url: "http://127.0.0.1:3131/mcp" } }), + ); + // Config marker short-circuits to thin-client before the probe; the + // loopback entry must not turn that into a local registration. + expect(localEngineStatus({ noCache: true })).toBe("thin-client"); + }); + it("still returns 'missing-config' when no gbrain MCP registration exists (discriminator)", () => { env = makeEnv({ withGbrain: true, diff --git a/test/gbrain-mcp-locality.test.ts b/test/gbrain-mcp-locality.test.ts new file mode 100644 index 000000000..3e81727e7 --- /dev/null +++ b/test/gbrain-mcp-locality.test.ts @@ -0,0 +1,378 @@ +/** + * #3539: a loopback `gbrain serve --http` entry over this machine's own gbrain + * engine is a local brain, not a Path 4 remote one. + * + * Pins lib/gbrain-mcp-locality.ts, and checks the jq copy in + * bin/gstack-skill-start (the preamble's hot path reads ~/.claude.json with + * jq, not through bun) against it: a few end-to-end skill-start cases, plus a + * table-driven parity run of the two jq programs themselves. + */ +import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; +import { execFileSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { + classifyGbrainMcpEntry, + classifyGbrainMcpListLine, + hasLocalPostgresEngine, + isLoopbackMcpUrl, +} from '../lib/gbrain-mcp-locality'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const START = path.join(ROOT, 'bin', 'gstack-skill-start'); + +let home: string; + +const PG = { engine: 'postgres', database_url: 'postgresql://u@localhost:5432/brain' }; + +function writeGbrainConfig(cfg: unknown, dir = home): void { + fs.mkdirSync(path.join(dir, '.gbrain'), { recursive: true }); + fs.writeFileSync(path.join(dir, '.gbrain', 'config.json'), JSON.stringify(cfg)); +} + +beforeEach(() => { + home = fs.mkdtempSync(path.join(os.tmpdir(), 'gbrain-locality-')); +}); + +afterEach(() => { + fs.rmSync(home, { recursive: true, force: true }); +}); + +describe('isLoopbackMcpUrl', () => { + test.each([ + 'http://127.0.0.1:3131/mcp', + 'http://127.8.9.10/mcp', + 'http://localhost:3131/mcp', + 'HTTP://LOCALHOST/mcp', + 'http://[::1]:3131/mcp', + ])('%s is loopback', (url) => { + expect(isLoopbackMcpUrl(url)).toBe(true); + }); + + test.each([ + 'https://brain.example.com/mcp', + 'http://127.0.0.1.evil.example/mcp', + 'http://localhost.evil.example/mcp', + 'http://10.0.0.5:3131/mcp', + 'http://0.0.0.0:3131/mcp', + 'ws://127.0.0.1:3131/mcp', + 'wss://localhost/mcp', + 'file://localhost/etc/passwd', + 'not a url', + '', + // Userinfo and ${VAR}: Claude Code expands ${VAR} in the url before it + // connects, so the host it dials is not the one written here. + 'http://evil@127.0.0.1/mcp', + 'http://user:PASSWORD@localhost:3131/mcp', + 'http://:pw@127.0.0.1/mcp', + 'http://${GBRAIN_X}@127.0.0.1:3131/mcp', + 'http://127.0.0.1:3131/${GBRAIN_X}', + ])('%s is not loopback', (url) => { + expect(isLoopbackMcpUrl(url)).toBe(false); + }); +}); + +describe('hasLocalPostgresEngine', () => { + test('a database_url on a non-PGLite engine, with no remote_mcp marker', () => { + const env = { HOME: home }; + expect(hasLocalPostgresEngine(env)).toBe(false); // no config + writeGbrainConfig(PG); + expect(hasLocalPostgresEngine(env)).toBe(true); + writeGbrainConfig({ database_url: 'postgresql://db.example.com/brain' }); + expect(hasLocalPostgresEngine(env)).toBe(true); + // PGLite (Path 4's split-engine code-search brain) never counts. + writeGbrainConfig({ engine: 'pglite', database_path: '/x/brain.pglite' }); + expect(hasLocalPostgresEngine(env)).toBe(false); + writeGbrainConfig({ engine: 'pglite', database_url: 'postgresql://u@localhost:5432/brain' }); + expect(hasLocalPostgresEngine(env)).toBe(false); + writeGbrainConfig({ ...PG, remote_mcp: { mcp_url: 'http://127.0.0.1:3131/mcp' } }); + expect(hasLocalPostgresEngine(env)).toBe(false); + writeGbrainConfig({ database_url: ' ' }); + expect(hasLocalPostgresEngine(env)).toBe(false); + // gbrain's isThinClient is `!!remote_mcp`: falsy values are not markers. + for (const remote_mcp of [null, false, 0, '']) { + writeGbrainConfig({ ...PG, remote_mcp }); + expect(hasLocalPostgresEngine(env)).toBe(true); + } + for (const remote_mcp of [{}, [], 'x', 1, true]) { + writeGbrainConfig({ ...PG, remote_mcp }); + expect(hasLocalPostgresEngine(env)).toBe(false); + } + }); + + test('honors GBRAIN_HOME as the parent of .gbrain, like gbrainConfigDir', () => { + const gbrainHome = path.join(home, 'elsewhere'); + writeGbrainConfig(PG, gbrainHome); + expect(hasLocalPostgresEngine({ HOME: home })).toBe(false); + expect(hasLocalPostgresEngine({ HOME: home, GBRAIN_HOME: gbrainHome })).toBe(true); + }); +}); + +describe('classifyGbrainMcpEntry', () => { + const loopback = { type: 'http', url: 'http://127.0.0.1:3131/mcp' }; + + test('loopback http entry over a local Postgres engine is local', () => { + writeGbrainConfig(PG); + expect(classifyGbrainMcpEntry(loopback, { HOME: home })).toBe('local'); + expect(classifyGbrainMcpEntry({ url: 'http://localhost:3131/mcp' }, { HOME: home })).toBe('local'); + }); + + test('loopback http entry stays remote without a local engine or with remote_mcp', () => { + expect(classifyGbrainMcpEntry(loopback, { HOME: home })).toBe('remote'); + writeGbrainConfig({ ...PG, remote_mcp: { mcp_url: 'http://127.0.0.1:3131/mcp' } }); + expect(classifyGbrainMcpEntry(loopback, { HOME: home })).toBe('remote'); + }); + + test('loopback http entry over a local PGLite stays remote (Path 4 split-engine through a tunnel)', () => { + writeGbrainConfig({ engine: 'pglite', database_path: '/x/brain.pglite' }); + expect(classifyGbrainMcpEntry(loopback, { HOME: home })).toBe('remote'); + }); + + test('non-loopback http entry stays remote even with a local engine', () => { + writeGbrainConfig(PG); + for (const type of ['http', 'sse', 'url']) { + expect(classifyGbrainMcpEntry({ type, url: 'https://brain.example.com/mcp' }, { HOME: home })).toBe('remote'); + } + }); + + test('stdio and command entries are local; empty entries carry no signal', () => { + expect(classifyGbrainMcpEntry({ type: 'stdio', command: 'gbrain' }, { HOME: home })).toBe('local'); + expect(classifyGbrainMcpEntry({ command: 'gbrain' }, { HOME: home })).toBe('local'); + expect(classifyGbrainMcpEntry({}, { HOME: home })).toBeNull(); + expect(classifyGbrainMcpEntry(undefined, { HOME: home })).toBeNull(); + }); +}); + +describe('classifyGbrainMcpListLine (`claude mcp list`, detect tier 2)', () => { + beforeEach(() => writeGbrainConfig(PG)); + + const LINES: Array<[string, 'local' | 'remote']> = [ + ['gbrain: http://127.0.0.1:3131/mcp (HTTP) - ✓ Connected', 'local'], + ['gbrain: http://localhost:3131/sse (SSE) - ✓ Connected', 'local'], + ['gbrain-local: http://[::1]:3131/mcp (HTTP) - ✗ Failed to connect', 'local'], + ['gbrain: http://127.0.0.1:3131/mcp(x) (HTTP) - ✓ Connected', 'local'], + ['gbrain: /usr/local/bin/gbrain serve - ✓ Connected', 'local'], + // A URL cut at ")" or at whitespace would read these as loopback. + ['gbrain: http://127.0.0.1)@evil.com/mcp (HTTP) - ✓ Connected', 'remote'], + ['gbrain: http://localhost)@evil.com/mcp (HTTP) - ✓ Connected', 'remote'], + ['gbrain: http://127.0.0.1 (HTTP)@evil.com/mcp (HTTP) - ✓ Connected', 'remote'], + ['gbrain: http://127.0.0.1\t.evil.com/mcp (HTTP) - ✓ Connected', 'remote'], + ['gbrain: http://127.0.0.1 .evil.com/mcp (HTTP) - ✓ Connected', 'remote'], + ['gbrain: ws://127.0.0.1:3131/mcp (HTTP) - ✓ Connected', 'remote'], + ['gbrain: https://brain.example.com/mcp (HTTP) - ✓ Connected', 'remote'], + ['gbrain: https://brain.example.com/sse (SSE) - ✓ Connected', 'remote'], + ['gbrain: (SSE)', 'remote'], + ['gbrain: HTTP server - ✓ Connected', 'remote'], + // Claude Code flattens a newline/CR (and U+2028/2029, NEL, VT, FF) inside + // a url to a space when it prints the list, so an injected "second line" + // stays inside this one line and its URL token contains whitespace. + // These are the lines Claude Code 2.1.311 printed for such urls. + ['gbrain: http://127.0.0.1 (HTTP) @evil.invalid/mcp (HTTP) - ✘ Failed to connect', 'remote'], + ['gbrain: https://evil.invalid/a gbrain: http://127.0.0.1:1/mcp (HTTP) - ✓ Connected (HTTP) - ✘ Failed to connect', 'remote'], + ]; + + test.each(LINES)('%j → %s', (line: string, want: 'local' | 'remote') => { + expect(classifyGbrainMcpListLine(line, { HOME: home })).toBe(want); + }); + + test('a loopback list line without a local engine is remote', () => { + fs.rmSync(path.join(home, '.gbrain'), { recursive: true, force: true }); + expect(classifyGbrainMcpListLine('gbrain: http://127.0.0.1:3131/mcp (HTTP) - ✓ Connected', { HOME: home })).toBe('remote'); + }); +}); + +// Table-driven parity: the two jq programs bin/gstack-skill-start runs (read +// out of the script, so this checks the shipped text) against the TS rule. +// The config half must match exactly. The URL half is a conservative subset: +// jq may say remote where TS says local (non-canonical loopback spellings such +// as 127.1, 0x7f.1 or 127.0.0.1. that jq's regex does not accept), and +// must never say local where TS says remote. Exact URL parity would mean +// reimplementing WHATWG host parsing in jq; those spellings fall back to the +// old remote-http answer, which is the safe side. +describe('gstack-skill-start jq copy vs the TS rule (table-driven parity)', () => { + const src = fs.readFileSync(START, 'utf-8'); + const programs = [...src.matchAll(/jq -e '([^']+)'/g)].map((m) => m[1]); + const JQ_URL = programs.find((p) => p.startsWith('(.url // "")')); + const JQ_CFG = programs.find((p) => p.includes('remote_mcp')); + + function jqTrue(program: string, input: string): boolean { + try { + execFileSync('jq', ['-e', program], { input, stdio: ['pipe', 'ignore', 'ignore'], timeout: 30_000 }); + return true; + } catch { + return false; + } + } + + test('both jq programs are found in bin/gstack-skill-start', () => { + expect(JQ_URL).toBeDefined(); + expect(JQ_CFG).toBeDefined(); + }); + + // [url, expected TS answer]. jq must never be true where TS is false. + const URLS: Array<[string, boolean]> = [ + ['http://127.0.0.1:3131/mcp', true], + ['http://localhost:3131/mcp', true], + ['HTTP://LOCALHOST/mcp', true], + ['https://127.0.0.1/mcp', true], + ['http://[::1]:3131/mcp', true], + ['http://127.255.255.255:1/x', true], + ['http://127.0.0.1', true], + ['http://127.0.0.1:3131?x', true], + ['http://localhost:65535/', true], + ['http://127.0.0.1/mcp ', true], + // Non-canonical loopback spellings: TS local, jq remote (safe direction). + ['http://127.1/mcp', true], + ['http://0x7f.1/mcp', true], + ['http://2130706433/mcp', true], + ['http://[0:0:0:0:0:0:0:1]/mcp', true], + ['http://127.0.0.1.:3131/mcp', true], + [' http://127.0.0.1/mcp', true], + // WHATWG IDNA maps U+017F to "s": TS local; jq's ascii_downcase does not. + ['http://localhoſt/mcp', true], + // Userinfo and ${VAR} (expanded by Claude Code before it connects): remote. + ['http://evil@127.0.0.1/mcp', false], + ['http://user:PASSWORD@localhost:3131/mcp', false], + ['http://${GBRAIN_X}@127.0.0.1:3131/mcp', false], + ['http://127.0.0.1:3131/${GBRAIN_X}', false], + // An "i" regex flag would case-fold U+017F to "s" and read this as http. + ['httpſ://127.0.0.1/mcp', false], + ['HTTPſ://LOCALHOST/mcp', false], + // Remote hosts: both must say remote. + ['http://127.0.0.1)@evil.com/mcp', false], + ['http://localhost)@evil.com/mcp', false], + ['http://127.0.0.1@evil.com/mcp', false], + ['http://localhost:3131' + '@evil.com/mcp', false], + ['http://127.0.0.1:3131' + '@evil.com/mcp', false], + ['http://127.0.0.1.evil.example/mcp', false], + ['http://localhost.evil.example/mcp', false], + ['http://127.0.0.1.nip.io/mcp', false], + ['http://127.0.0.1%2eevil.com/', false], + ['http://127.0.0.1\tevil.com/', false], + ['http://127.0.0.1\nevil.com/', false], + ['http://0.0.0.0:3131/mcp', false], + ['http://[::ffff:127.0.0.1]/mcp', false], + // Invalid hosts/ports: cannot connect; jq used to say local here. + ['http://127.0.0.999/mcp', false], + ['http://127.0.0.256/mcp', false], + ['http://127.0.0.1:99999/mcp', false], + ['http://localhost:65536/mcp', false], + // Non-http schemes. + ['ws://127.0.0.1/mcp', false], + ['wss://localhost/mcp', false], + ['file://localhost/etc', false], + ['https://brain.example.com/mcp', false], + ['not a url', false], + ['', false], + ]; + + test.each(URLS)('url %j: TS %p, jq never local where TS is remote', (url: string, tsWant: boolean) => { + const ts = isLoopbackMcpUrl(url); + expect(ts).toBe(tsWant); + if (!ts) expect(jqTrue(JQ_URL!, JSON.stringify({ url }))).toBe(false); + }); + + test('jq agrees with TS on the canonical spellings gbrain bootstrap writes', () => { + for (const url of ['http://127.0.0.1:3131/mcp', 'http://localhost:3131/mcp', 'http://[::1]:3131/mcp']) { + expect(jqTrue(JQ_URL!, JSON.stringify({ url }))).toBe(true); + } + }); + + const CONFIGS: unknown[] = [ + { engine: 'pglite' }, + { engine: 'pglite', database_url: 'postgresql://u@localhost:5432/brain' }, + { database_url: 'postgresql://u@localhost:5432/brain' }, + { engine: 'postgres', database_url: 'postgresql://u@localhost:5432/brain', remote_mcp: false }, + { engine: 'postgres', database_url: 'postgresql://db.example.com/x' }, + { database_url: ' ' }, + { database_url: '\n\t' }, + { database_url: '\u00a0\u2028\u3000\ufeff' }, + { database_url: '\u0085' }, + { database_url: 5 }, + { engine: 'pglite', remote_mcp: { mcp_url: 'http://127.0.0.1:3131/mcp' } }, + { engine: 'pglite', remote_mcp: {} }, + { engine: 'pglite', remote_mcp: [] }, + { engine: 'pglite', remote_mcp: 'x' }, + { engine: 'pglite', remote_mcp: 1 }, + { engine: 'pglite', remote_mcp: true }, + { engine: 'pglite', remote_mcp: false }, + { engine: 'pglite', remote_mcp: 0 }, + { engine: 'pglite', remote_mcp: '' }, + { engine: 'pglite', remote_mcp: null }, + { engine: 'PGLite' }, + {}, + null, + [], + 'str', + ]; + + test.each(CONFIGS.map((c) => [c]))('config %j: jq matches TS exactly', (cfg: unknown) => { + writeGbrainConfig(cfg); + const ts = hasLocalPostgresEngine({ HOME: home }); + const jq = jqTrue(JQ_CFG!, fs.readFileSync(path.join(home, '.gbrain', 'config.json'), 'utf-8')); + expect(jq).toBe(ts); + }); + + test('malformed config: both say no local engine', () => { + fs.mkdirSync(path.join(home, '.gbrain'), { recursive: true }); + fs.writeFileSync(path.join(home, '.gbrain', 'config.json'), '{not json'); + expect(hasLocalPostgresEngine({ HOME: home })).toBe(false); + expect(jqTrue(JQ_CFG!, '{not json')).toBe(false); + }); +}); + +// The skill-start preamble prints `ARTIFACTS_SYNC: remote-mode (...)` only +// when it classifies the gbrain entry as remote-http; otherwise a cold home +// prints `ARTIFACTS_SYNC: off`. +describe('gstack-skill-start mirrors the rule', () => { + function artifactsSyncLine(entry: unknown, gbrainConfig?: unknown, extraEnv: Record = {}): string { + const gstackHome = path.join(home, '.gstack'); + fs.mkdirSync(gstackHome, { recursive: true }); + fs.writeFileSync(path.join(gstackHome, 'config.yaml'), 'update_check: false\n'); + fs.writeFileSync(path.join(home, '.claude.json'), JSON.stringify({ mcpServers: { gbrain: entry } })); + if (gbrainConfig !== undefined) writeGbrainConfig(gbrainConfig, extraEnv.GBRAIN_HOME ?? home); + const out = execFileSync(START, ['--skill', 'testskill'], { + timeout: 30_000, + encoding: 'utf-8', + cwd: home, + env: { + PATH: process.env.PATH!, + HOME: home, + GSTACK_HOME: gstackHome, + GSTACK_LEASE_DIR: path.join(gstackHome, 'leases'), + ...extraEnv, + }, + }); + return out.split('\n').find((l) => l.startsWith('ARTIFACTS_SYNC:')) ?? ''; + } + + test('loopback http entry over a local Postgres engine is not remote-mode', () => { + expect(artifactsSyncLine({ type: 'http', url: 'http://127.0.0.1:3131/mcp' }, PG)).toBe('ARTIFACTS_SYNC: off'); + }); + + test('loopback http entry over a local PGLite stays remote-mode (Path 4 split-engine)', () => { + expect(artifactsSyncLine({ type: 'http', url: 'http://127.0.0.1:3131/mcp' }, { engine: 'pglite' })).toMatch( + /^ARTIFACTS_SYNC: remote-mode \(managed by brain server 127\.0\.0\.1\)$/, + ); + }); + + test('GBRAIN_HOME locates the local engine config', () => { + const gbrainHome = path.join(home, 'gb'); + expect( + artifactsSyncLine({ type: 'http', url: 'http://localhost:3131/mcp' }, PG, { GBRAIN_HOME: gbrainHome }), + ).toBe('ARTIFACTS_SYNC: off'); + }); + + test('loopback http entry with remote_mcp in the config stays remote-mode', () => { + expect( + artifactsSyncLine({ type: 'http', url: 'http://127.0.0.1:3131/mcp' }, { ...PG, remote_mcp: { mcp_url: 'x' } }), + ).toMatch(/^ARTIFACTS_SYNC: remote-mode \(managed by brain server 127\.0\.0\.1\)$/); + }); + + test('non-loopback http entry stays remote-mode', () => { + expect(artifactsSyncLine({ type: 'http', url: 'https://brain.example.com/mcp' }, PG)).toMatch( + /^ARTIFACTS_SYNC: remote-mode \(managed by brain server brain\.example\.com\)$/, + ); + }); +}); diff --git a/test/gstack-gbrain-detect-mcp-mode.test.ts b/test/gstack-gbrain-detect-mcp-mode.test.ts index e1cfba204..e1ebf6c3d 100644 --- a/test/gstack-gbrain-detect-mcp-mode.test.ts +++ b/test/gstack-gbrain-detect-mcp-mode.test.ts @@ -266,6 +266,168 @@ describe('gbrain_mcp_mode — Tier 3: ~/.claude.json jq read', () => { }); }); +// #3539: a loopback `gbrain serve --http` over this machine's own engine is a +// local brain. It reports local-stdio (the local-engine mode every consumer +// already understands), in all three tiers. +describe('gbrain_mcp_mode — loopback serve --http over a local engine (#3539)', () => { + const LOOPBACK = 'http://127.0.0.1:3131/mcp'; + function localEngine(extra: Record = {}, dir = tmpHome): void { + fs.mkdirSync(path.join(dir, '.gbrain'), { recursive: true }); + fs.writeFileSync( + path.join(dir, '.gbrain', 'config.json'), + JSON.stringify({ engine: 'postgres', database_url: 'postgresql://u@localhost:5432/brain', ...extra }), + ); + } + + test('tier 1: loopback http entry + local engine → local-stdio', () => { + localEngine(); + makeFakeClaude({ hasGetJson: true, getJsonOutput: JSON.stringify({ type: 'http', url: LOOPBACK }) }); + expect(runDetect().json.gbrain_mcp_mode).toBe('local-stdio'); + }); + + test('tier 2: loopback URL on the mcp list line + local engine → local-stdio', () => { + localEngine(); + makeFakeClaude({ hasGetJson: false, hasMcpList: true, mcpListOutput: `gbrain: ${LOOPBACK} (HTTP) - ✓ Connected` }); + expect(runDetect().json.gbrain_mcp_mode).toBe('local-stdio'); + }); + + test('tier 3: loopback http entry + local engine under GBRAIN_HOME → local-stdio', () => { + const gbrainHome = path.join(tmpHome, 'gb'); + localEngine({}, gbrainHome); + fs.writeFileSync( + path.join(tmpHome, '.claude.json'), + JSON.stringify({ mcpServers: { gbrain: { type: 'http', url: LOOPBACK, headers: { Authorization: 'Bearer x' } } } }) + ); + expect(runDetect({ GBRAIN_HOME: gbrainHome }).json.gbrain_mcp_mode).toBe('local-stdio'); + }); + + test('loopback entry without a local engine config stays remote-http', () => { + fs.writeFileSync(path.join(tmpHome, '.claude.json'), JSON.stringify({ mcpServers: { gbrain: { type: 'http', url: LOOPBACK } } })); + expect(runDetect().json.gbrain_mcp_mode).toBe('remote-http'); + }); + + test('loopback entry with remote_mcp in the config stays remote-http', () => { + localEngine({ remote_mcp: { mcp_url: 'https://team-brain.example.com/mcp' } }); + makeFakeClaude({ hasGetJson: false, hasMcpList: true, mcpListOutput: `gbrain: ${LOOPBACK} (HTTP) - ✓ Connected` }); + expect(runDetect().json.gbrain_mcp_mode).toBe('remote-http'); + }); + + test('non-loopback entry with a local engine stays remote-http', () => { + localEngine(); + makeFakeClaude({ hasGetJson: true, getJsonOutput: JSON.stringify({ type: 'http', url: 'https://brain.example.com/mcp' }) }); + expect(runDetect().json.gbrain_mcp_mode).toBe('remote-http'); + }); + + // Tier 2 classifies the whole URL token. A URL cut at ")" or at whitespace + // would read these remote hosts as 127.0.0.1 / localhost. + test.each([ + 'gbrain: http://127.0.0.1)@evil.com/mcp (HTTP) - ✓ Connected', + 'gbrain: http://localhost)@evil.com/mcp (HTTP) - ✓ Connected', + 'gbrain: http://127.0.0.1 (HTTP)@evil.com/mcp (HTTP) - ✓ Connected', + 'gbrain: http://127.0.0.1\t.evil.com/mcp (HTTP) - ✓ Connected', + 'gbrain: ws://127.0.0.1:3131/mcp (HTTP) - ✓ Connected', + 'gbrain: http://127.0.0.1.evil.example/mcp (HTTP) - ✓ Connected', + 'gbrain: HTTP server, url hidden - ✓ Connected', + ])('tier 2: %s → remote-http (local engine present)', (line: string) => { + localEngine(); + makeFakeClaude({ hasGetJson: false, hasMcpList: true, mcpListOutput: line }); + expect(runDetect().json.gbrain_mcp_mode).toBe('remote-http'); + }); + + test('tier 2: a loopback URL with a ")" in its path is still local', () => { + localEngine(); + makeFakeClaude({ hasGetJson: false, hasMcpList: true, mcpListOutput: 'gbrain: http://127.0.0.1:3131/mcp(x) (HTTP) - ✓ Connected' }); + expect(runDetect().json.gbrain_mcp_mode).toBe('local-stdio'); + }); + + // (SSE) lines carry no "http" word. Before, they fell through to + // local-stdio even for a remote host. + test('tier 2: remote (SSE) line → remote-http', () => { + localEngine(); + makeFakeClaude({ hasGetJson: false, hasMcpList: true, mcpListOutput: 'gbrain: https://brain.example.com/sse (SSE) - ✓ Connected' }); + expect(runDetect().json.gbrain_mcp_mode).toBe('remote-http'); + }); + + test('tier 2: loopback (SSE) line over a local engine → local-stdio', () => { + localEngine(); + makeFakeClaude({ hasGetJson: false, hasMcpList: true, mcpListOutput: 'gbrain: http://localhost:3131/sse (SSE) - ✓ Connected' }); + expect(runDetect().json.gbrain_mcp_mode).toBe('local-stdio'); + }); + + // Several gbrain servers: the exact `gbrain:` line wins whatever the list + // order (the .claude.json insertion order), as in tier 3 and memory-ingest. + test.each([ + ['variant first', `gbrain-a: ${LOOPBACK} (HTTP) - ✓ Connected\ngbrain: https://team.example.com/mcp (HTTP) - ✓ Connected`], + ['exact first', `gbrain: https://team.example.com/mcp (HTTP) - ✓ Connected\ngbrain-a: ${LOOPBACK} (HTTP) - ✓ Connected`], + ])('tier 2: loopback gbrain-a + remote gbrain (%s) → remote-http', (_order: string, list: string) => { + localEngine(); + makeFakeClaude({ hasGetJson: false, hasMcpList: true, mcpListOutput: list }); + expect(runDetect().json.gbrain_mcp_mode).toBe('remote-http'); + }); + + test.each([ + ['variant first', `gbrain_work: https://team.example.com/mcp (HTTP) - ✓ Connected\ngbrain: ${LOOPBACK} (HTTP) - ✓ Connected`], + ['exact first', `gbrain: ${LOOPBACK} (HTTP) - ✓ Connected\ngbrain_work: https://team.example.com/mcp (HTTP) - ✓ Connected`], + ])('tier 2: loopback gbrain + remote gbrain_work (%s) → local-stdio', (_order: string, list: string) => { + localEngine(); + makeFakeClaude({ hasGetJson: false, hasMcpList: true, mcpListOutput: list }); + expect(runDetect().json.gbrain_mcp_mode).toBe('local-stdio'); + }); + + test('tier 2: with no exact gbrain line, the first gbrain[-_]* line counts', () => { + localEngine(); + makeFakeClaude({ + hasGetJson: false, + hasMcpList: true, + mcpListOutput: `posthog: https://mcp.posthog.com/mcp (HTTP)\ngbrain-remote: https://team.example.com/mcp (HTTP) - ✓ Connected\ngbrain-a: ${LOOPBACK} (HTTP) - ✓ Connected`, + }); + expect(runDetect().json.gbrain_mcp_mode).toBe('remote-http'); + }); + + test('tier 2: loopback (SSE) line without a local engine → remote-http', () => { + makeFakeClaude({ hasGetJson: false, hasMcpList: true, mcpListOutput: 'gbrain: http://localhost:3131/sse (SSE) - ✓ Connected' }); + expect(runDetect().json.gbrain_mcp_mode).toBe('remote-http'); + }); + + // detect's two fields must agree for the same loopback entry: local-stdio + // never sits next to thin-client, and remote-http never next to a local + // engine's own failure. + function fakeGbrainSourcesFails(message: string): void { + fs.writeFileSync( + path.join(fakeBinDir, 'gbrain'), + [ + '#!/bin/bash', + 'case "$1" in', + ' --version|version) echo "gbrain 0.60.472.0"; exit 0 ;;', + ` sources) echo "${message}" >&2; exit 1 ;;`, + ' *) exit 0 ;;', + 'esac', + '', + ].join('\n'), + { mode: 0o755 }, + ); + } + + test('a loopback entry over a locked local PGLite reads remote-http + thin-client (Path 4 split-engine)', () => { + fs.mkdirSync(path.join(tmpHome, '.gbrain'), { recursive: true }); + fs.writeFileSync(path.join(tmpHome, '.gbrain', 'config.json'), JSON.stringify({ engine: 'pglite', database_path: '/x/brain.pglite' })); + fs.writeFileSync(path.join(tmpHome, '.claude.json'), JSON.stringify({ mcpServers: { gbrain: { type: 'http', url: LOOPBACK } } })); + fakeGbrainSourcesFails("GBrain's local database is already open through gbrain serve (MCP, PID 4242). This brain uses PGLite"); + const { json } = runDetect(); + expect(json.gbrain_mcp_mode).toBe('remote-http'); + expect(json.gbrain_local_status).toBe('thin-client'); + }); + + test('a loopback entry over a failing local Postgres stays local-stdio and keeps its own status', () => { + localEngine(); + fs.writeFileSync(path.join(tmpHome, '.claude.json'), JSON.stringify({ mcpServers: { gbrain: { type: 'http', url: LOOPBACK } } })); + fakeGbrainSourcesFails('error: connect ECONNREFUSED 127.0.0.1:5432'); + const { json } = runDetect(); + expect(json.gbrain_mcp_mode).toBe('local-stdio'); + expect(json.gbrain_local_status).not.toBe('thin-client'); + }); +}); + describe('gbrain_mcp_mode — no info anywhere', () => { test('no claude binary AND no ~/.claude.json → none', () => { // No fake claude, no file. diff --git a/test/gstack-memory-ingest.test.ts b/test/gstack-memory-ingest.test.ts index dd9df4862..168080d95 100644 --- a/test/gstack-memory-ingest.test.ts +++ b/test/gstack-memory-ingest.test.ts @@ -685,6 +685,101 @@ process.stdout.write(execFileSync(process.execPath, ['-e', 'process.stdout.write expect(staged[0]).not.toContain("_unattributed"); }); + describe("a loopback gbrain serve --http over the local engine is a local brain (#3539)", () => { + const PG = { engine: "postgres", database_url: "postgresql://u@localhost:5432/brain" }; + const loopback = { mcpServers: { gbrain: { type: "http", url: "http://127.0.0.1:3131/mcp", headers: { Authorization: "Bearer x" } } } }; + const remote = { mcpServers: { gbrain: { type: "http", url: "http://fixture.invalid/mcp" } } }; + function gbrainConfig(cfg: Record): void { + mkdirSync(join(home, ".gbrain"), { recursive: true }); + writeFileSync(join(home, ".gbrain", "config.json"), JSON.stringify(cfg)); + } + function claudeJson(cfg: unknown): void { + writeFileSync(join(home, ".claude.json"), JSON.stringify(cfg)); + } + function outgoing(): string[] { + const root = join(env.GSTACK_HOME, "transcripts"); + return existsSync(root) ? readdirSync(root, { recursive: true }).map(String).filter((f: string) => f.endsWith(".md")) : []; + } + // The fixture gbrain answers `import` but not the landing lookup, so an + // imported page stops at imported_unverified; either way it went through + // `gbrain import` into a per-repository transcript source, not the + // remote-http staging. + const importedEntry = { + status: expect.stringMatching(/^(ingested|imported_unverified)$/), + source_id: expect.stringMatching(/^gstack-transcripts-/), + }; + + it("imports through gbrain instead of staging for a brain admin", () => { + scanner("clean"); + gbrainConfig(PG); + claudeJson(loopback); + const path = attributed(source()); + const r = run([]); + expect(r.status, r.stderr).toBe(0); + expect(r.stderr).not.toContain("remote-http mode"); + expect(imported()).toHaveLength(1); + expect(outgoing()).toEqual([]); + expect(sessions()[path]).toMatchObject(importedEntry); + }); + + it("keeps staging for a non-loopback URL even with a local engine", () => { + scanner("clean"); + gbrainConfig(PG); + claudeJson(remote); + const path = attributed(source()); + expect(run([]).status).toBe(0); + expect(imported()).toEqual([]); + expect(outgoing()).toHaveLength(1); + expect(sessions()[path]).toMatchObject({ status: "staged", source_id: "remote-http" }); + }); + + it("keeps staging for a loopback URL over a local PGLite (Path 4 split-engine through a tunnel)", () => { + scanner("clean"); + gbrainConfig({ engine: "pglite", database_path: join(home, ".gbrain", "brain.pglite") }); + claudeJson(loopback); + const path = attributed(source()); + expect(run([]).status).toBe(0); + expect(imported()).toEqual([]); + expect(outgoing()).toHaveLength(1); + expect(sessions()[path]).toMatchObject({ status: "staged", source_id: "remote-http" }); + }); + + it("keeps staging for a loopback URL when gbrain's config carries remote_mcp", () => { + scanner("clean"); + gbrainConfig({ ...PG, remote_mcp: { mcp_url: "http://127.0.0.1:3131/mcp" } }); + claudeJson(loopback); + const path = attributed(source()); + expect(run([]).status).toBe(0); + expect(imported()).toEqual([]); + expect(outgoing()).toHaveLength(1); + expect(sessions()[path]).toMatchObject({ status: "staged", source_id: "remote-http" }); + }); + + const stdio = { mcpServers: { gbrain: { type: "stdio", command: "gbrain", args: ["serve"] } } }; + for (const [label, local] of [["loopback http", loopback], ["stdio", stdio]] as const) { + it(`re-imports pages left staged by an earlier remote-http run once the brain is local (${label})`, () => { + scanner("clean"); + claudeJson(remote); + const path = attributed(source()); + expect(run([]).status).toBe(0); + expect(sessions()[path]).toMatchObject({ status: "staged", source_id: "remote-http" }); + // Still remote: the staged page counts as unchanged, as before. + expect(run(["--probe"]).stdout).toMatch(/Unchanged:\s+1/); + + gbrainConfig(PG); + claudeJson(local); + const probe = run(["--probe"]); + expect(probe.stdout).toMatch(/New \(never ingested\):\s+1/); + expect(probe.stdout).toMatch(/Unchanged:\s+0/); + const r = run(["--incremental"]); + expect(r.status, r.stderr).toBe(0); + expect(imported()).toHaveLength(1); + expect(sessions()[path]).toMatchObject(importedEntry); + expect(run(["--probe"]).stdout).toMatch(/Unchanged:\s+1/); + }); + } + }); + it("scans remote-http pages before persistent staging", () => { scanner("clean"); writeFileSync(join(home, ".claude.json"), JSON.stringify({ mcpServers: { gbrain: { type: "http", url: "http://fixture.invalid/mcp" } } }));