diff --git a/CHANGELOG.md b/CHANGELOG.md index b07bc2142..ef75b9b50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,55 @@ # Changelog +## [1.55.0.0] - 2026-05-30 + +## **`/sync-gbrain` can no longer be the trigger that lets gbrain delete your repo. The headed browser stops crash-looping, and gbrain installs the current release instead of a pin 23 versions stale.** + +gbrain can rm-rf a working tree when its autopilot daemon reclones mid-cycle. `/sync-gbrain` used to call gbrain's `sources remove` and `sync --strategy code` as if they were safe, so it could be the thing that set that race off. Now every destructive gbrain call sits behind feature-detected guards: the orchestrator refuses to run while autopilot is active, refuses to remove a user-managed source it can't storage-protect (it fails closed), canonicalizes paths with realpath so a symlink can't smuggle a delete outside gbrain's own clones, and requires an explicit `--allow-reclone` before a URL-managed source's code walk. Shipped in the same wave: the headed browser's self-inflicted crash-loop is gone, big-brain memory ingests stop getting killed at a fixed 30 minutes, and the gbrain installer moves off its frozen v0.18.2 pin onto the latest release behind a version floor and a `doctor` self-test. + +### The numbers that matter + +From the shipped diff and its regression suites (`bun test test/gbrain-*.test.ts browse/test/restart-env.test.ts test/memory-ingest-timeout.test.ts`): + +| Metric | Before | After | Δ | +|--------|--------|-------|---| +| Destructive gbrain ops behind guards | 0 | 4 | +4 | +| gbrain / brain-sync spawns that work on Windows | 0/8 | 8/8 | +8 | +| gbrain version installed | v0.18.2 (pinned, ~23 behind) | latest + min-version floor + doctor gate | — | +| Memory-ingest timeout | hardcoded 30 min | configurable, checkpoint preserved on timeout | — | +| Generated SKILL.md that parse under strict YAML | partial (colons broke Codex) | all (quoted) | — | + +The guard that matters most: a `sources remove` on a source whose files live outside `~/.gbrain/clones/` and can't be storage-protected now refuses instead of proceeding. The path that ate a repo no longer runs unattended. + +### What this means for you + +If you use `/sync-gbrain`, you are protected from the data-loss race even before gbrain ships its own root fix. "Don't run `/sync-gbrain` while `gbrain autopilot` is active" is now enforced, not just advised, and nothing gets deleted that can't be proven safe. Headed-browser QA against beacon-heavy pages (analytics, live extensions) no longer crash-loops, leaks Chromium, or silently drops to an invisible headless window. New gbrain installs track the current release. Codex and OpenAI can load every gstack skill again. + +### Itemized changes + +#### Added +- `/sync-gbrain` destructive-op guards (`lib/gbrain-guards.ts`): multi-signal autopilot detection, fail-closed `sources remove`, realpath `remote_url` pre-flight audit, and a `--allow-reclone` gate before URL-managed code walks. +- Install-time gbrain gate (`bin/gstack-gbrain-install`): a minimum-version floor and a `gbrain doctor --fast` self-test, both hard-fail with remediation. +- `GSTACK_INGEST_TIMEOUT_MS` to configure the memory-ingest timeout; on timeout the gbrain checkpoint is preserved so the next run resumes. + +#### Changed +- gbrain installs at the latest default-branch HEAD by default; pin a commit with `gstack-gbrain-install --pinned-commit ` for reproducibility. +- Generated SKILL.md descriptions with interior colons are now quoted, so strict YAML loaders (Codex/OpenAI) parse them. +- `/sync-gbrain` guidance: do not run during autopilot; prefer `gbrain sources add --path` over URL-managed sources. + +#### Fixed +- `/sync-gbrain` no longer races gbrain's autopilot into a destructive reclone or remove (#1734). Report by @mvanhorn. +- `gstack-jsonl-merge` resolves equal-timestamp entries deterministically across machines, so append-only logs converge instead of re-conflicting forever (#1769). Contributed by @jbetala7. +- Generated SKILL.md frontmatter parses under strict YAML loaders (#1778). Reported by @GilbertzzzZZ, @genisis0x, @cathrynlavery, and @sator-imaging. +- The headed browser daemon no longer crash-loops under load, leaks Chromium processes, or silently downgrades a headed session to headless (#1781). +- `/sync-gbrain --full` memory ingests on large brains are no longer killed at a fixed 30-minute timeout (#1611). +- The gbrain CLI and `gstack-brain-sync` spawn correctly on Windows (#1731). + +#### For contributors +- `lib/gbrain-guards.ts` with hermetic tests for every guard branch (autopilot signals, fail-closed remove, reclone gate, realpath containment). +- `parseSourcesList` centralizes `gbrain sources list --json` shape handling across all readers (#1576, whose crash was already fixed in v1.42.0.0 — this removes the last divergent reader). +- Static-grep tripwire (`test/gbrain-spawn-windows-shell.test.ts`) fails CI if a gbrain spawn drops the Windows shell flag. +- gbrain-side requirements for the root fixes (ungated reclone, `--keep-storage`, a cooperative remove-lease, a capability command, true ingest-resume, integration CI) are tracked for the gbrain repo. + ## [1.54.0.0] - 2026-05-30 ## **The heaviest skill stopped taxing every session. /ship's always-loaded cost dropped 59%, and its prose now loads only when a step needs it.** diff --git a/CLAUDE.md b/CLAUDE.md index 4e3c48a55..dc62ad561 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -938,4 +938,10 @@ file globs. Run `/sync-gbrain` after meaningful code changes; for ongoing auto-sync across all worktrees, run `gbrain autopilot --install` once per machine — gbrain's daemon handles incremental refresh on a schedule. +Safety: don't run `/sync-gbrain` while `gbrain autopilot` is active — the +orchestrator refuses destructive source ops when it detects a running autopilot +to avoid racing it (#1734). Prefer registering user repos with `gbrain sources +add --path ` (no `--url`): URL-managed sources can auto-reclone, and the +sync code walk for them requires an explicit `--allow-reclone` opt-in. + diff --git a/USING_GBRAIN_WITH_GSTACK.md b/USING_GBRAIN_WITH_GSTACK.md index f2b4a48ce..ec1144c9a 100644 --- a/USING_GBRAIN_WITH_GSTACK.md +++ b/USING_GBRAIN_WITH_GSTACK.md @@ -136,7 +136,7 @@ The skill runs three stages — code, memory, brain-sync — independently. A fa 1. **Pre-flight.** Checks `gbrain_local_status` (the local engine's health). If the engine is `broken-db` or `broken-config`, the skill STOPs with a remediation menu — it refuses to silently degrade. If the local engine is missing and you're in remote-MCP mode (Path 4), the code stage SKIPs cleanly and only brain-sync runs. 2. **Code stage.** Registers the cwd as a federated source via `gbrain sources add`, writes a `.gbrain-source` pin file in the repo root (kubectl-style context — every worktree gets its own pin, so Conductor sibling worktrees don't collide), runs `gbrain sync --strategy code`. -3. **Memory stage.** Stages your `~/.gstack/` transcripts + curated memory. In local-stdio MCP mode, ingests into the local engine. In remote-http MCP mode, persists staged markdown to `~/.gstack/transcripts/run--/` for the remote brain admin's pull pipeline. +3. **Memory stage.** Stages your `~/.gstack/` transcripts + curated memory. In local-stdio MCP mode, ingests into the local engine. In remote-http MCP mode, persists staged markdown to `~/.gstack/transcripts/run--/` for the remote brain admin's pull pipeline. The ingest timeout is 30 minutes by default; raise it for a big brain with `GSTACK_INGEST_TIMEOUT_MS` (accepts 1 min–24h). On timeout the gbrain import checkpoint is preserved, so the next `/sync-gbrain` resumes instead of starting over. 4. **Brain-sync stage.** Pushes curated artifacts (plans, designs, retros) to your private artifacts repo if you have one configured. 5. **CLAUDE.md guidance.** Capability-checks the round-trip (write a page → search → find it). If green, writes the `## GBrain Search Guidance` block to your project's CLAUDE.md. If red, REMOVES the block — the agent should never be told to use a tool that isn't installed. @@ -379,7 +379,7 @@ Another gstack session in a sibling Conductor workspace may be holding a lock on ## Related skills + next steps - `/health` — includes a GBrain dimension (doctor status, sync queue depth, last-push age) in its 0-10 composite score. The dimension is omitted when gbrain isn't installed; running `/health` on a non-gbrain machine doesn't penalize that choice. -- `/gstack-upgrade` — keeps gstack itself up to date. Does NOT upgrade gbrain independently. To bump gbrain, update `PINNED_COMMIT` in `bin/gstack-gbrain-install` and re-run `/setup-gbrain`. +- `/gstack-upgrade` — keeps gstack itself up to date. Does NOT upgrade gbrain independently. gbrain installs at the latest HEAD by default; to refresh it, `git pull` in your gbrain clone (default `~/gbrain`) and re-run `/setup-gbrain`. Pin a specific commit with `gstack-gbrain-install --pinned-commit ` if you need reproducibility. Installs below the minimum tested version are refused. - `/retro` — weekly retrospective pulls learnings and plans from your gbrain when memory sync is on, letting the retro reference cross-machine history. Run `/setup-gbrain` and see what sticks. diff --git a/VERSION b/VERSION index 1ffb2a6e0..7bd316a72 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.54.0.0 +1.55.0.0 diff --git a/bin/gstack-gbrain-install b/bin/gstack-gbrain-install index e7e029ce0..60c8f86b6 100755 --- a/bin/gstack-gbrain-install +++ b/bin/gstack-gbrain-install @@ -19,9 +19,14 @@ # - git # - network reachability to https://github.com # -# The pinned commit is declared here rather than resolved dynamically so -# upgrades are explicit and reviewable. Update PINNED_COMMIT when gstack -# verifies compatibility with a new gbrain release. +# gbrain installs at the latest default-branch HEAD by default — the hard pin +# was removed in #1744 (it had drifted ~23 versions behind). Pass +# --pinned-commit to install a specific commit for reproducibility. A +# minimum-version floor (MIN_GBRAIN_VERSION) hard-fails the install when the +# resulting gbrain is too old for gstack's sync integration, and a fast +# `gbrain doctor` self-test hard-fails a broken install when gbrain is already +# configured. This keeps the version gate that the pin used to provide without +# freezing users 23 releases behind. # # Env: # GBRAIN_INSTALL_DIR — override default install path (~/gbrain) @@ -33,8 +38,14 @@ set -euo pipefail # --- defaults --- -PINNED_COMMIT="08b3698e90532b7b66c445e6b1d8cdfe71822802" # gbrain v0.18.2 -PINNED_TAG="v0.18.2" +# No version pin by default — install the latest default-branch HEAD (#1744). +# --pinned-commit overrides for reproducibility. +PINNED_COMMIT="" +PINNED_TAG="" +# Minimum gbrain version gstack's integration is known to work with. The +# `sources list --json` wrapped-object shape + federated sources landed by 0.20; +# older predates the surface gstack drives. Hard-fail below this floor (#1744). +MIN_GBRAIN_VERSION="0.20.0" GBRAIN_REPO_URL="https://github.com/garrytan/gbrain.git" DEFAULT_INSTALL_DIR="${GBRAIN_INSTALL_DIR:-$HOME/gbrain}" INSTALL_DIR="$DEFAULT_INSTALL_DIR" @@ -113,7 +124,7 @@ elif [ -n "$DETECTED_CLONE" ]; then else # Fresh clone path. if $DRY_RUN; then - log "DRY RUN: would clone $GBRAIN_REPO_URL @ $PINNED_COMMIT → $INSTALL_DIR" + log "DRY RUN: would clone $GBRAIN_REPO_URL ${PINNED_COMMIT:+@ $PINNED_COMMIT }→ $INSTALL_DIR (latest HEAD unless --pinned-commit)" exit 0 fi if [ -d "$INSTALL_DIR" ]; then @@ -121,8 +132,12 @@ else fi log "cloning $GBRAIN_REPO_URL → $INSTALL_DIR" git clone --quiet "$GBRAIN_REPO_URL" "$INSTALL_DIR" - ( cd "$INSTALL_DIR" && git checkout --quiet "$PINNED_COMMIT" ) - log "pinned to $PINNED_COMMIT${PINNED_TAG:+ ($PINNED_TAG)}" + if [ -n "$PINNED_COMMIT" ]; then + ( cd "$INSTALL_DIR" && git checkout --quiet "$PINNED_COMMIT" ) + log "checked out pinned commit $PINNED_COMMIT${PINNED_TAG:+ ($PINNED_TAG)}" + else + log "installed latest gbrain (default-branch HEAD)" + fi fi if $DRY_RUN; then @@ -195,6 +210,44 @@ fi log "installed gbrain $actual_version from $INSTALL_DIR" +# --- minimum-version floor (#1744) --- +# Unpinning means new installs track gbrain HEAD. Hard-fail if the resulting +# version is below the floor gstack's sync integration needs — same exit-3 posture +# as the PATH-shadow / version-mismatch failures above. A warning here is exactly +# how the data-loss class slipped through, so this gate fails closed. +version_lt() { + # 0 (true) when $1 < $2 by version sort; equal versions are NOT less-than. + [ "$1" = "$2" ] && return 1 + [ "$(printf '%s\n%s\n' "$1" "$2" | sort -V | head -1)" = "$1" ] +} +if version_lt "$actual_norm" "$MIN_GBRAIN_VERSION"; then + echo "" >&2 + echo "gstack-gbrain-install: gbrain $actual_version is below the minimum gstack-tested version ($MIN_GBRAIN_VERSION)." >&2 + echo " gstack's sync integration needs the v0.20+ source/list surface." >&2 + echo " Fix: update the gbrain clone at $INSTALL_DIR to a newer release (git pull), then" >&2 + echo " re-run /setup-gbrain. Or pass --pinned-commit to install a specific newer commit." >&2 + echo "" >&2 + exit 3 +fi + +# --- functional self-test when gbrain is already configured (#1744) --- +# When a brain config exists (re-install / detected clone), run a fast doctor as +# a hard gate so a broken gbrain is caught at setup, not at data-loss time. +# Pre-init installs skip this (config not written yet); the full +# `/sync-gbrain --dry-run` self-test runs from /setup-gbrain after `gbrain init`. +_GBRAIN_HOME_CHECK="${GBRAIN_HOME:-$HOME/.gbrain}" +if [ -f "$_GBRAIN_HOME_CHECK/config.json" ]; then + if ! gbrain doctor --fast >/dev/null 2>&1; then + echo "" >&2 + echo "gstack-gbrain-install: gbrain $actual_version installed but 'gbrain doctor --fast' failed." >&2 + echo " Refusing to leave a broken gbrain in place. Run 'gbrain doctor' to see what's wrong," >&2 + echo " fix it, then re-run /setup-gbrain." >&2 + echo "" >&2 + exit 3 + fi + log "gbrain doctor --fast passed" +fi + # v1.40.0.0 post-install validation (T6 / codex review #19): --ignore-scripts # may skip artifacts gbrain needs at runtime, especially on Windows # MSYS/MINGW where we DID pass --ignore-scripts. `gbrain --version` above diff --git a/bin/gstack-gbrain-sync.ts b/bin/gstack-gbrain-sync.ts index c3708a090..d88fc51a4 100644 --- a/bin/gstack-gbrain-sync.ts +++ b/bin/gstack-gbrain-sync.ts @@ -37,9 +37,10 @@ import { createHash } from "crypto"; import "../lib/conductor-env-shim"; import { detectEngineTier, withErrorContext, canonicalizeRemote } from "../lib/gstack-memory-helpers"; -import { ensureSourceRegistered, sourcePageCount } from "../lib/gbrain-sources"; +import { ensureSourceRegistered, sourcePageCount, parseSourcesList } from "../lib/gbrain-sources"; +import { detectAutopilot, decideSourceRemove, decideCodeSync } from "../lib/gbrain-guards"; import { localEngineStatus, type LocalEngineStatus } from "../lib/gbrain-local-status"; -import { buildGbrainEnv, spawnGbrain, execGbrainJson } from "../lib/gbrain-exec"; +import { buildGbrainEnv, spawnGbrain, execGbrainJson, NEEDS_SHELL_ON_WINDOWS } from "../lib/gbrain-exec"; // ── Types ────────────────────────────────────────────────────────────────── @@ -52,6 +53,8 @@ interface CliArgs { noMemory: boolean; noBrainSync: boolean; codeOnly: boolean; + /** #1734: opt-in to sync a URL-managed source whose code walk may auto-reclone. */ + allowReclone: boolean; } interface CodeStageDetail { @@ -59,7 +62,7 @@ interface CodeStageDetail { source_path?: string; page_count?: number | null; last_imported?: string; - status?: "ok" | "skipped" | "failed"; + status?: "ok" | "skipped" | "failed" | "refused-autopilot" | "refused-reclone"; } interface StageResult { @@ -205,6 +208,8 @@ Options: --no-memory Skip the gstack-memory-ingest stage (transcripts + artifacts). --no-brain-sync Skip the gstack-brain-sync git pipeline stage. --code-only Only run the code-import stage (alias for --no-memory --no-brain-sync). + --allow-reclone Permit the code walk for URL-managed sources (remote_url set) + even though gbrain may auto-reclone the working tree (#1734). --help This text. Stages run in order: code → memory ingest → curated git push. @@ -220,6 +225,7 @@ function parseArgs(): CliArgs { let noMemory = false; let noBrainSync = false; let codeOnly = false; + let allowReclone = false; for (let i = 0; i < args.length; i++) { const a = args[i]; @@ -231,6 +237,7 @@ function parseArgs(): CliArgs { case "--no-code": noCode = true; break; case "--no-memory": noMemory = true; break; case "--no-brain-sync": noBrainSync = true; break; + case "--allow-reclone": allowReclone = true; break; case "--code-only": codeOnly = true; noMemory = true; @@ -247,7 +254,7 @@ function parseArgs(): CliArgs { } } - return { mode, quiet, noCode, noMemory, noBrainSync, codeOnly }; + return { mode, quiet, noCode, noMemory, noBrainSync, codeOnly, allowReclone }; } // ── Helpers ──────────────────────────────────────────────────────────────── @@ -407,10 +414,7 @@ export function sourceLocalPath(sourceId: string, env?: NodeJS.ProcessEnv): stri { baseEnv: env }, ); if (!raw) return null; - const list: Array<{ id?: string; local_path?: string }> = Array.isArray(raw) - ? (raw as Array<{ id?: string; local_path?: string }>) - : ((raw as { sources?: Array<{ id?: string; local_path?: string }> }).sources ?? []); - const found = list.find((s) => s.id === sourceId); + const found = parseSourcesList(raw).find((s) => s.id === sourceId); return found?.local_path ?? null; } @@ -469,20 +473,50 @@ export function planHostnameFoldMigration( return { kind: "pending-cleanup", oldId: legacyPathHashId }; } +export interface GuardedRemoveResult { + removed: boolean; + /** True when a guard refused the remove (autopilot active or unsafe source). */ + skipped: boolean; + reason: string; +} + +/** + * #1734: run `gbrain sources remove --confirm-destructive` only behind the + * data-loss guards. Checked immediately before the destructive op (E8: as late + * as possible) so the autopilot window is as small as we can make it without a + * gbrain-side lease. Refuses when autopilot is active or when the source is + * user-managed and gbrain can't keep its storage. Pure side-effect helper; the + * caller decides whether a skip is fatal (it never is today — removes are + * best-effort cleanup). + */ +export function safeSourcesRemove(sourceId: string, env?: NodeJS.ProcessEnv): GuardedRemoveResult { + const ap = detectAutopilot(env); + if (ap.active) { + return { + removed: false, + skipped: true, + reason: `autopilot active (${ap.signal}); refusing destructive remove of ${sourceId}. ` + + `Stop autopilot, then re-run /sync-gbrain.`, + }; + } + const decision = decideSourceRemove(sourceId, env); + if (!decision.allow) { + return { removed: false, skipped: true, reason: decision.reason }; + } + const r = spawnGbrain( + ["sources", "remove", sourceId, "--confirm-destructive", ...decision.extraArgs], + { baseEnv: env }, + ); + return { removed: r.status === 0, skipped: false, reason: decision.reason }; +} + /** * Remove an orphaned source. Called only after new-source sync verifies pages - * exist, so the old source is provably redundant before deletion. - * - * Flag note: existing call sites used `--confirm-destructive` here and - * `--yes` in `lib/gbrain-sources.ts` — gbrain 0.35.0.0 accepts neither - * deterministically (the subcommand surface help is generic). We pass - * `--confirm-destructive` to match the existing call site convention; the - * flag-helper centralization in commit 4 (lib/gbrain-exec.ts) will resolve - * the inconsistency across the codebase. + * exist, so the old source is provably redundant before deletion. Routed through + * safeSourcesRemove for the #1734 guards. */ export function removeOrphanedSource(oldId: string, env?: NodeJS.ProcessEnv): boolean { - const r = spawnGbrain(["sources", "remove", oldId, "--confirm-destructive"], { baseEnv: env }); - return r.status === 0; + return safeSourcesRemove(oldId, env).removed; } /** @@ -661,13 +695,12 @@ async function runCodeImport(args: CliArgs): Promise { const legacyId = deriveLegacyCodeSourceId(root); let legacyRemoved = false; if (legacyId !== sourceId) { - const rm = spawnGbrain(["sources", "remove", legacyId, "--confirm-destructive"], { - timeout: 30_000, - baseEnv: gbrainEnv, - }); - // Treat absent-source as success (clean state). gbrain emits "not found" on - // missing id; treat any non-zero exit without "not found" as a soft fail. - if (rm.status === 0) legacyRemoved = true; + // #1734: route through the data-loss guards (autopilot + source-safety). + const rm = safeSourcesRemove(legacyId, gbrainEnv); + if (rm.skipped && !args.quiet) { + console.error(`[sync:code] legacy-source cleanup skipped: ${rm.reason}`); + } + if (rm.removed) legacyRemoved = true; } // Step 0b: Hostname-fold migration (#1414). @@ -720,6 +753,29 @@ async function runCodeImport(args: CliArgs): Promise { process.env.GSTACK_SYNC_CODE_TIMEOUT_MS, "GSTACK_SYNC_CODE_TIMEOUT_MS", ); + + // #1734 guards, checked immediately before the destructive walk (E8): + // - autopilot active → refuse (the race that wiped a working tree). + // - URL-managed source → the walk can auto-reclone (rm-rf); require + // --allow-reclone. Both surface a visible reason and fail the stage so the + // verdict shows ERR rather than silently skipping protection. + const apBeforeWalk = detectAutopilot(gbrainEnv); + if (apBeforeWalk.active) { + return { + name: "code", ran: true, ok: false, duration_ms: Date.now() - t0, + summary: `refused: gbrain autopilot active (${apBeforeWalk.signal}). Stop autopilot, then re-run /sync-gbrain.`, + detail: { source_id: sourceId, source_path: root, status: "refused-autopilot" }, + }; + } + const reclone = decideCodeSync(sourceId, gbrainEnv, args.allowReclone); + if (!reclone.allow) { + return { + name: "code", ran: true, ok: false, duration_ms: Date.now() - t0, + summary: `refused: ${reclone.reason}`, + detail: { source_id: sourceId, source_path: root, status: "refused-reclone" }, + }; + } + const walkResult = spawnGbrain(["sync", "--strategy", "code", "--source", sourceId], { stdio: args.quiet ? ["ignore", "ignore", "ignore"] : ["ignore", "inherit", "inherit"], timeout: codeTimeoutMs, @@ -961,13 +1017,17 @@ function runBrainSyncPush(args: CliArgs): StageResult { return { name: "brain-sync", ran: false, ok: true, duration_ms: 0, summary: "skipped (gstack-brain-sync not installed)" }; } + // #1731: gstack-brain-sync is a bash shebang script; Windows can't spawn it + // without a shell, which surfaced as "brain-sync exited undefined". spawnSync(brainSyncPath, ["--discover-new"], { stdio: args.quiet ? ["ignore", "ignore", "ignore"] : ["ignore", "inherit", "inherit"], timeout: 60 * 1000, + shell: NEEDS_SHELL_ON_WINDOWS, }); const result = spawnSync(brainSyncPath, ["--once"], { stdio: args.quiet ? ["ignore", "ignore", "ignore"] : ["ignore", "inherit", "inherit"], timeout: 60 * 1000, + shell: NEEDS_SHELL_ON_WINDOWS, }); return { diff --git a/bin/gstack-jsonl-merge b/bin/gstack-jsonl-merge index c777612ac..d2fa5744c 100755 --- a/bin/gstack-jsonl-merge +++ b/bin/gstack-jsonl-merge @@ -53,18 +53,25 @@ for path in paths: continue if line in seen: continue - # Prefer ISO ts field for sort; fall back to SHA-256. + # Prefer ISO ts field for sort; fall back to SHA-256. The line + # content is the final tiebreaker so the order is total: two + # entries sharing a ts must resolve identically regardless of + # which side they arrive on. Without it, equal-ts entries fall + # back to insertion order (base, ours, theirs), and since ours + # and theirs are swapped depending on which machine runs the + # merge, the two sides produce divergent files that never + # converge. sort_key = None try: obj = json.loads(line) ts = obj.get('ts') or obj.get('timestamp') if isinstance(ts, str): - sort_key = (0, ts) + sort_key = (0, ts, line) except (json.JSONDecodeError, ValueError, TypeError): pass if sort_key is None: h = hashlib.sha256(line.encode('utf-8')).hexdigest() - sort_key = (1, h) + sort_key = (1, h, line) seen[line] = sort_key except FileNotFoundError: # Absent base / absent ours / absent theirs are all valid. diff --git a/bin/gstack-memory-ingest.ts b/bin/gstack-memory-ingest.ts index a7ff80d51..98907eeee 100644 --- a/bin/gstack-memory-ingest.ts +++ b/bin/gstack-memory-ingest.ts @@ -1349,10 +1349,32 @@ function installSignalForwarder(): void { * that kill the child on parent SIGTERM/SIGINT. Returns the same shape as * spawnSync's result so the caller doesn't care which mode was used. */ +/** + * #1611: the `gbrain import` is the long pole on big brains. Its timeout is + * configurable via GSTACK_INGEST_TIMEOUT_MS (default 30 min, 1min–24h) so large + * memory corpora aren't SIGTERM'd mid-import. On timeout we SIGTERM the child, + * which preserves gbrain's import-checkpoint.json (see installSignalForwarder) + * so the next run resumes instead of restarting from scratch. + */ +const DEFAULT_IMPORT_TIMEOUT_MS = 30 * 60 * 1000; +export function resolveImportTimeoutMs( + raw: string | undefined = process.env.GSTACK_INGEST_TIMEOUT_MS, +): number { + if (raw === undefined || raw === "") return DEFAULT_IMPORT_TIMEOUT_MS; + const n = Number.parseInt(raw, 10); + if (!Number.isFinite(n) || Number.isNaN(n) || n < 60_000 || n > 86_400_000) { + console.error( + `[memory-ingest] GSTACK_INGEST_TIMEOUT_MS="${raw}" invalid (need 60000–86400000ms); using ${DEFAULT_IMPORT_TIMEOUT_MS}ms`, + ); + return DEFAULT_IMPORT_TIMEOUT_MS; + } + return n; +} + function runGbrainImport( stagingDir: string, timeoutMs: number, -): Promise<{ status: number | null; stdout: string; stderr: string }> { +): Promise<{ status: number | null; stdout: string; stderr: string; timedOut: boolean }> { installSignalForwarder(); return new Promise((resolve) => { // Seed DATABASE_URL from gbrain's own config so this stage works @@ -1385,6 +1407,7 @@ function runGbrainImport( status: timedOut ? null : status, stdout, stderr, + timedOut, }); }); child.on("error", (err) => { @@ -1394,6 +1417,7 @@ function runGbrainImport( status: null, stdout, stderr: stderr + `\n[spawn-error] ${(err as Error).message}`, + timedOut, }); }); }); @@ -1608,13 +1632,33 @@ async function ingestPass(args: CliArgs): Promise { // spawn, parent termination orphans the gbrain process (observed // during 2026-05-10 cold-run testing — gbrain kept running 15 min // after the orchestrator timed out). - const importResult = await runGbrainImport(stagingDir, 30 * 60 * 1000); + const importResult = await runGbrainImport(stagingDir, resolveImportTimeoutMs()); const stdout = importResult.stdout || ""; const stderr = importResult.stderr || ""; const importJson = parseImportJson(stdout); if (importResult.status !== 0) { + // #1611: on timeout, gbrain's import-checkpoint.json is preserved (the + // SIGTERM forwarder keeps the staging dir), so the next /sync-gbrain + // resumes rather than restarting. Tell the user instead of looking failed. + if (importResult.timedOut) { + const mins = Math.round(resolveImportTimeoutMs() / 60000); + const msg = + `gbrain import timed out after ${mins}min; checkpoint preserved — re-run ` + + `/sync-gbrain to resume (raise GSTACK_INGEST_TIMEOUT_MS for big brains)`; + console.error(`[memory-ingest] ${msg}`); + return { + written: 0, + skipped_secret: prep.skippedSecret, + skipped_dedup: prep.skippedDedup, + skipped_unattributed: prep.skippedUnattributed, + failed, + duration_ms: Date.now() - t0, + partial_pages: prep.partialPages, + system_error: msg, + }; + } const tail = (stderr.trim().split("\n").pop() || "").slice(0, 300); const msg = `gbrain import exited ${importResult.status}: ${tail}`; console.error(`[memory-ingest] ERR: ${msg}`); @@ -1810,7 +1854,12 @@ async function main(): Promise { if (result.system_error) process.exit(1); } -main().catch((err) => { - console.error(`gstack-memory-ingest fatal: ${err instanceof Error ? err.message : String(err)}`); - process.exit(1); -}); +// Guard so the module is import-safe for unit tests (e.g. resolveImportTimeoutMs). +// The orchestrator runs it as `bun gstack-memory-ingest.ts ...`, where +// import.meta.main is true, so the CLI path is unaffected. +if (import.meta.main) { + main().catch((err) => { + console.error(`gstack-memory-ingest fatal: ${err instanceof Error ? err.message : String(err)}`); + process.exit(1); + }); +} diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 4df950190..59327b792 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -211,6 +211,86 @@ function cleanupLegacyState(): void { } } +// ─── Chromium profile lock helpers (#1781) ───────────────────── +/** Profile dir used by headed/connect Chromium sessions. */ +function chromiumProfileDir(): string { + return path.join(process.env.HOME || '/tmp', '.gstack', 'chromium-profile'); +} + +/** Remove Chromium SingletonLock/Socket/Cookie so a relaunch can acquire the + * profile. Safe to call when absent. */ +function cleanChromiumProfileLocks(profileDir: string = chromiumProfileDir()): void { + for (const lockFile of ['SingletonLock', 'SingletonSocket', 'SingletonCookie']) { + safeUnlinkQuiet(path.join(profileDir, lockFile)); + } +} + +/** Kill an orphaned Chromium that still holds the profile's SingletonLock. The + * lock symlink target is "hostname-PID"; killing that PID tears down its + * renderer tree so the next launch starts clean. No-op when absent/stale. */ +async function killOrphanChromium(profileDir: string = chromiumProfileDir()): Promise { + try { + const lockTarget = fs.readlinkSync(path.join(profileDir, 'SingletonLock')); // "hostname-12345" + const orphanPid = parseInt(lockTarget.split('-').pop() || '', 10); + if (orphanPid && isProcessAlive(orphanPid)) { + safeKill(orphanPid, 'SIGTERM'); + await new Promise(r => setTimeout(r, 1000)); + if (isProcessAlive(orphanPid)) { + safeKill(orphanPid, 'SIGKILL'); + await new Promise(r => setTimeout(r, 500)); + } + } + } catch (err: any) { + if (err?.code !== 'ENOENT' && err?.code !== 'EINVAL') throw err; + } +} + +/** Bounded /health probe. Returns true if the server answers within `attempts` + * tries spaced `backoffMs` apart — distinguishes a busy-but-alive daemon from a + * dead one (#1781) so a slow server isn't killed and restarted into a crash-loop. */ +async function probeHealthWithBackoff(port: number, attempts = 3, backoffMs = 250): Promise { + for (let i = 0; i < attempts; i++) { + if (await isServerHealthy(port)) return true; + if (i < attempts - 1) await Bun.sleep(backoffMs); + } + return false; +} + +/** + * Build the env for an auto-restart after a crash. headed/proxy/configHash are + * reapplied from THIS invocation OR the persisted server state, so a restart + * triggered by a plain command (goto/status, no --headed flag) never silently + * downgrades a headed session to headless (#1781). Pure + exported for tests. + */ +export function buildRestartEnv( + globalFlags: GlobalFlags | null | undefined, + oldState: ServerState | null, +): Record { + const env: Record = {}; + if (globalFlags?.proxyUrl) env.BROWSE_PROXY_URL = globalFlags.proxyUrl; + if (globalFlags?.headed || oldState?.mode === 'headed') env.BROWSE_HEADED = '1'; + const configHash = globalFlags?.configHash || oldState?.configHash; + if (configHash) env.BROWSE_CONFIG_HASH = configHash; + return env; +} + +/** macOS only: pull the headed Chromium window to the user's current Space. + * "Google Chrome for Testing" frequently opens behind the active window or on + * another Space — the first thing users read as "I can't see the browser" + * (#1781). Best-effort, fire-and-forget, never throws. The app name is a fixed + * literal (no interpolation). */ +function raiseHeadedWindowMacOS(): void { + if (process.platform !== 'darwin') return; + try { + nodeSpawn('osascript', ['-e', 'tell application "Google Chrome for Testing" to activate'], { + stdio: 'ignore', + detached: true, + }).unref(); + } catch { + // osascript missing or app not present — non-fatal + } +} + // ─── Server Lifecycle ────────────────────────────────────────── async function startServer(extraEnv?: Record): Promise { ensureStateDir(config); @@ -219,6 +299,13 @@ async function startServer(extraEnv?: Record): Promise= 1) throw new Error('[browse] Server unresponsive after retry — aborting'); + console.error('[browse] Server was briefly unresponsive (busy); retrying command...'); + return sendCommand(oldState, command, args, retries + 1); + } + // Truly dead (or health never recovered) → restart. if (retries >= 1) throw new Error('[browse] Server crashed twice in a row — aborting'); console.error('[browse] Server connection lost. Restarting...'); - // Kill the old server to avoid orphaned chromium processes - const oldState = readState(); if (oldState && oldState.pid) { await killServer(oldState.pid); } - // Reapply --proxy / --headed flags from this invocation when restarting - // after a crash. Without this, a proxied daemon that dies mid-command - // would silently restart in default direct/headless mode and bypass - // the SOCKS bridge. - const restartEnv: Record = {}; - if (_globalFlags?.proxyUrl) restartEnv.BROWSE_PROXY_URL = _globalFlags.proxyUrl; - if (_globalFlags?.headed) restartEnv.BROWSE_HEADED = '1'; - if (_globalFlags?.configHash) restartEnv.BROWSE_CONFIG_HASH = _globalFlags.configHash; + // startServer() now clears the Chromium SingletonLock + reaps the orphan, + // so the relaunch isn't blocked by the dead Chromium's profile lock (#1781). + // + // Reapply --proxy / --headed when restarting. headed comes from THIS + // invocation OR the persisted server mode, so a restart triggered by a + // plain command (goto/status, no --headed) never silently downgrades a + // headed session to headless (#1781). Same for proxy/configHash. + const restartEnv = buildRestartEnv(_globalFlags, oldState); const newState = await startServer(Object.keys(restartEnv).length ? restartEnv : undefined); return sendCommand(newState, command, args, retries + 1); } @@ -966,30 +1069,11 @@ Refs: After 'snapshot', use @e1, @e2... as selectors: } } - // Kill orphaned Chromium processes that may still hold the profile lock. - // The server PID is the Bun process; Chromium is a child that can outlive it - // if the server is killed abruptly (SIGKILL, crash, manual rm of state file). - const profileDir = path.join(process.env.HOME || '/tmp', '.gstack', 'chromium-profile'); - try { - const singletonLock = path.join(profileDir, 'SingletonLock'); - const lockTarget = fs.readlinkSync(singletonLock); // e.g. "hostname-12345" - const orphanPid = parseInt(lockTarget.split('-').pop() || '', 10); - if (orphanPid && isProcessAlive(orphanPid)) { - safeKill(orphanPid, 'SIGTERM'); - await new Promise(resolve => setTimeout(resolve, 1000)); - if (isProcessAlive(orphanPid)) { - safeKill(orphanPid, 'SIGKILL'); - await new Promise(resolve => setTimeout(resolve, 500)); - } - } - } catch (err: any) { - if (err?.code !== 'ENOENT' && err?.code !== 'EINVAL') throw err; - } - - // Clean up Chromium profile locks (can persist after crashes) - for (const lockFile of ['SingletonLock', 'SingletonSocket', 'SingletonCookie']) { - safeUnlinkQuiet(path.join(profileDir, lockFile)); - } + // Kill an orphaned Chromium still holding the profile lock (the Bun server + // PID's Chromium child can outlive an abrupt kill/crash), then clear the + // lock files so the launch is clean. Shared with the auto-restart path (#1781). + await killOrphanChromium(); + cleanChromiumProfileLocks(); // Delete stale state file safeUnlinkQuiet(config.stateFile); @@ -1027,6 +1111,11 @@ Refs: After 'snapshot', use @e1, @e2... as selectors: }); const status = await resp.text(); console.log(`Connected to real Chrome\n${status}`); + // #1781: surface the window — it often opens behind/on another Space. + raiseHeadedWindowMacOS(); + if (process.platform === 'darwin') { + console.log('(If you still don\'t see it, check Mission Control / other Spaces.)'); + } // sidebar-agent.ts spawn was here. Ripped alongside the chat queue — // the Terminal pane runs an interactive PTY now, no more one-shot @@ -1194,11 +1283,11 @@ Refs: After 'snapshot', use @e1, @e2... as selectors: safeKill(existingState.pid, 'SIGKILL'); } } - // Clean profile locks and state file - const profileDir = path.join(process.env.HOME || '/tmp', '.gstack', 'chromium-profile'); - for (const lockFile of ['SingletonLock', 'SingletonSocket', 'SingletonCookie']) { - safeUnlinkQuiet(path.join(profileDir, lockFile)); - } + // #1781: killing the daemon can orphan its Chromium child tree, which keeps + // holding the SingletonLock and makes the next `connect` fail to launch. + // Reap the orphan via the lock, then clear the lock files + state. + await killOrphanChromium(); + cleanChromiumProfileLocks(); // Xvfb orphan cleanup: if the recorded PID still matches our Xvfb (by // cmdline AND start-time), kill it. PID-only would risk killing a // recycled PID belonging to an unrelated process. @@ -1258,6 +1347,11 @@ Refs: After 'snapshot', use @e1, @e2... as selectors: } await sendCommand(state, command, commandArgs); + + // #1781: `focus` means "show me the window". The server-side focus activates + // the page via CDP, but on macOS the app can still sit on another Space — pull + // it to the user's current Space too. + if (command === 'focus') raiseHeadedWindowMacOS(); } if (import.meta.main) { diff --git a/browse/test/restart-env.test.ts b/browse/test/restart-env.test.ts new file mode 100644 index 000000000..5cf7502e1 --- /dev/null +++ b/browse/test/restart-env.test.ts @@ -0,0 +1,39 @@ +import { describe, test, expect } from "bun:test"; +import { buildRestartEnv } from "../src/cli"; + +// #1781: an auto-restart triggered by a plain command (no --headed flag) must +// NOT silently downgrade a headed session to headless. buildRestartEnv reapplies +// headed/proxy/configHash from this invocation OR the persisted server state. +describe("buildRestartEnv (#1781 headed persistence)", () => { + const headedState = { pid: 1, port: 9, token: "t", startedAt: "", serverPath: "", mode: "headed" as const }; + const launchedState = { pid: 1, port: 9, token: "t", startedAt: "", serverPath: "", mode: "launched" as const }; + + test("headed flag on this invocation → BROWSE_HEADED=1", () => { + expect(buildRestartEnv({ headed: true } as any, null).BROWSE_HEADED).toBe("1"); + }); + + test("plain command + persisted headed state → still BROWSE_HEADED=1 (the regression)", () => { + const env = buildRestartEnv({} as any, headedState as any); + expect(env.BROWSE_HEADED).toBe("1"); + }); + + test("plain command + headless state → no BROWSE_HEADED (no spurious headed)", () => { + const env = buildRestartEnv({} as any, launchedState as any); + expect(env.BROWSE_HEADED).toBeUndefined(); + }); + + test("nothing set → empty env", () => { + expect(buildRestartEnv(null, null)).toEqual({}); + }); + + test("proxy + configHash reapplied from flags", () => { + const env = buildRestartEnv({ proxyUrl: "socks5://x", configHash: "abc" } as any, null); + expect(env.BROWSE_PROXY_URL).toBe("socks5://x"); + expect(env.BROWSE_CONFIG_HASH).toBe("abc"); + }); + + test("configHash falls back to persisted state", () => { + const env = buildRestartEnv({} as any, { ...launchedState, configHash: "fromstate" } as any); + expect(env.BROWSE_CONFIG_HASH).toBe("fromstate"); + }); +}); diff --git a/design-consultation/SKILL.md b/design-consultation/SKILL.md index 1e8762964..9bab21e2d 100644 --- a/design-consultation/SKILL.md +++ b/design-consultation/SKILL.md @@ -2,7 +2,7 @@ name: design-consultation preamble-tier: 3 version: 1.0.0 -description: Design consultation: understands your product, researches the landscape, proposes a complete design system (aesthetic, typography, color, layout, spacing, motion), and generates font+color preview... (gstack) +description: "Design consultation: understands your product, researches the landscape, proposes a complete design system (aesthetic, typography, color, layout, spacing, motion), and generates font+color preview... (gstack)" allowed-tools: - Bash - Read diff --git a/design-html/SKILL.md b/design-html/SKILL.md index 2d1b3cfb5..f6e9e17f8 100644 --- a/design-html/SKILL.md +++ b/design-html/SKILL.md @@ -2,7 +2,7 @@ name: design-html preamble-tier: 2 version: 1.0.0 -description: Design finalization: generates production-quality Pretext-native HTML/CSS. (gstack) +description: "Design finalization: generates production-quality Pretext-native HTML/CSS. (gstack)" triggers: - build the design - code the mockup diff --git a/design-review/SKILL.md b/design-review/SKILL.md index 97f365f13..e874a94aa 100644 --- a/design-review/SKILL.md +++ b/design-review/SKILL.md @@ -2,7 +2,7 @@ name: design-review preamble-tier: 4 version: 2.0.0 -description: Designer's eye QA: finds visual inconsistency, spacing issues, hierarchy problems, AI slop patterns, and slow interactions — then fixes them. (gstack) +description: "Designer's eye QA: finds visual inconsistency, spacing issues, hierarchy problems, AI slop patterns, and slow interactions — then fixes them. (gstack)" allowed-tools: - Bash - Read diff --git a/design-shotgun/SKILL.md b/design-shotgun/SKILL.md index b504b79fe..9fd662ce6 100644 --- a/design-shotgun/SKILL.md +++ b/design-shotgun/SKILL.md @@ -2,7 +2,7 @@ name: design-shotgun preamble-tier: 2 version: 1.0.0 -description: Design shotgun: generate multiple AI design variants, open a comparison board, collect structured feedback, and iterate. (gstack) +description: "Design shotgun: generate multiple AI design variants, open a comparison board, collect structured feedback, and iterate. (gstack)" triggers: - explore design variants - show me design options diff --git a/guard/SKILL.md b/guard/SKILL.md index 8d4a57448..e4dff7936 100644 --- a/guard/SKILL.md +++ b/guard/SKILL.md @@ -1,7 +1,7 @@ --- name: guard version: 0.1.0 -description: Full safety mode: destructive command warnings + directory-scoped edits. (gstack) +description: "Full safety mode: destructive command warnings + directory-scoped edits. (gstack)" triggers: - full safety mode - guard against mistakes diff --git a/ios-clean/SKILL.md b/ios-clean/SKILL.md index 0a2ecd992..c9073b6d5 100644 --- a/ios-clean/SKILL.md +++ b/ios-clean/SKILL.md @@ -2,7 +2,7 @@ name: ios-clean preamble-tier: 3 version: 1.0.0 -description: Remove the DebugBridge SPM package and all #if DEBUG wiring from an iOS app. (gstack) +description: "Remove the DebugBridge SPM package and all #if DEBUG wiring from an iOS app. (gstack)" allowed-tools: - Bash - Read diff --git a/lib/gbrain-exec.ts b/lib/gbrain-exec.ts index 4568ef41a..12855d11d 100644 --- a/lib/gbrain-exec.ts +++ b/lib/gbrain-exec.ts @@ -137,6 +137,18 @@ export function buildGbrainEnv(opts: BuildGbrainEnvOptions = {}): NodeJS.Process return out; } +/** + * Windows can't directly spawn the `gbrain` launcher (bun/npm install it as a + * `gbrain.cmd`/`.ps1` shim) or a shebang script like the bash `gstack-brain-sync` + * — `spawnSync`/`spawn` resolve those only through a shell's PATHEXT + interpreter + * lookup. Without `shell: true` the child spawn fails ENOENT, which on the sync + * orchestrator surfaced as "brain-sync exited undefined" (#1731). Gate on platform + * so POSIX keeps the cheaper no-shell path. Exported so the static-grep tripwire + * (test/gbrain-spawn-windows-shell.test.ts) can assert every gbrain/brain-sync + * spawn carries it. + */ +export const NEEDS_SHELL_ON_WINDOWS = process.platform === "win32"; + export interface SpawnGbrainOptions { /** Timeout in milliseconds. Defaults to 30s. */ timeout?: number; @@ -166,6 +178,7 @@ export function spawnGbrain(args: string[], opts: SpawnGbrainOptions = {}): Spaw cwd: opts.cwd, stdio: opts.stdio || ["ignore", "pipe", "pipe"], env: buildGbrainEnv({ baseEnv: opts.baseEnv, announce: opts.announce }), + shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows }); } @@ -198,6 +211,7 @@ export function spawnGbrainAsync( stdio: opts.stdio || ["ignore", "pipe", "pipe"], cwd: opts.cwd, env: buildGbrainEnv({ baseEnv: opts.baseEnv, announce: false }), + shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows }); } @@ -212,5 +226,6 @@ export function execGbrainText(args: string[], opts: SpawnGbrainOptions = {}): s cwd: opts.cwd, stdio: opts.stdio || ["ignore", "pipe", "pipe"], env: buildGbrainEnv({ baseEnv: opts.baseEnv, announce: opts.announce }), + shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows }); } diff --git a/lib/gbrain-guards.ts b/lib/gbrain-guards.ts new file mode 100644 index 000000000..3a4edacba --- /dev/null +++ b/lib/gbrain-guards.ts @@ -0,0 +1,266 @@ +/** + * gbrain-guards — defense-in-depth against gbrain's destructive code paths (#1734). + * + * gbrain (the separate CLI gstack shells out to) can rm-rf a user's working tree + * during an autopilot race (its own bug, upstream gbrain #1526). gstack can't fix + * that, but it MUST stop treating gbrain's destructive subcommands as safe. These + * guards gate the two ways the orchestrator can reach destruction: + * + * 1. `sources remove --confirm-destructive` → decideSourceRemove() + * 2. `sync --strategy code` (can auto-reclone) → decideCodeSync() + * + * plus an autopilot-active check (detectAutopilot) that refuses to run destructive + * ops concurrently with the daemon. + * + * Design notes grounded in the real gbrain 0.41.x surface: + * - There is NO `--keep-storage` flag and NO structured capability command, and + * subcommand `--help` is generic — so capability detection is best-effort and + * defaults to "unsupported". When we can't protect a user-managed source's + * files, we FAIL CLOSED (refuse the remove) rather than delete unprotected. + * - The autopilot lock filename isn't documented and (gbrain #1226) ignores + * GBRAIN_HOME, so the live `gbrain autopilot` process is the PRIMARY signal; + * known lock paths under both the configured home and ~/.gbrain are secondary. + * - We refuse only on an AFFIRMATIVE autopilot signal — inability to introspect + * never blocks a normal sync (that would brick the tool). + * - Path containment uses realpath so a symlink inside ~/.gbrain/clones can't + * smuggle a delete out to a user repo. + * + * Pure decision functions; the orchestrator logs the reasons (observability). + */ + +import { spawnSync } from "child_process"; +import { existsSync, realpathSync } from "fs"; +import { homedir } from "os"; +import { join, resolve, sep } from "path"; +import { execGbrainJson, execGbrainText, NEEDS_SHELL_ON_WINDOWS } from "./gbrain-exec"; +import { parseSourcesList, type GbrainSourceRow } from "./gbrain-sources"; + +export function gbrainHome(env: NodeJS.ProcessEnv = process.env): string { + return env.GBRAIN_HOME || join(homedir(), ".gbrain"); +} + +/** + * Directories gbrain owns and may delete safely. A source whose local_path + * resolves inside one of these is gbrain-managed; outside = user-managed and + * must be protected. Both the configured home and the default ~/.gbrain are + * checked because gbrain #1226 shows home-resolution is inconsistent. + */ +function clonesDirs(env: NodeJS.ProcessEnv = process.env): string[] { + return [...new Set([join(gbrainHome(env), "clones"), join(homedir(), ".gbrain", "clones")])]; +} + +/** True if `p` resolves (symlinks + `..` collapsed) to a location inside `dir`. */ +export function isInside(p: string, dir: string): boolean { + let rp: string; + let rd: string; + try { rp = realpathSync(p); } catch { rp = resolve(p); } + try { rd = realpathSync(dir); } catch { rd = resolve(dir); } + const base = rd.endsWith(sep) ? rd : rd + sep; + return rp === rd || rp.startsWith(base); +} + +// ── Autopilot detection (E1: multi-signal, affirmative-only) ──────────────── + +export interface AutopilotStatus { + active: boolean; + /** Which signal fired (lock path or "process"), or null when inactive. */ + signal: string | null; +} + +export interface AutopilotProbe { + /** Override the lock-path list (tests). */ + lockPaths?: string[]; + /** Override the live-process check (tests). */ + processRunning?: () => boolean; +} + +/** + * Detect a running gbrain autopilot. Refuse the caller's destructive op only on + * an affirmative signal; absence of a confirmable mechanism returns inactive so + * normal syncs are never bricked. + */ +export function detectAutopilot( + env: NodeJS.ProcessEnv = process.env, + probe: AutopilotProbe = {}, +): AutopilotStatus { + // Secondary signal: known lock files. gbrain #1226 — the lock ignores + // GBRAIN_HOME, so check both the configured home and the default ~/.gbrain. + const lockPaths = probe.lockPaths ?? [ + join(gbrainHome(env), "autopilot.lock"), + join(homedir(), ".gbrain", "autopilot.lock"), + join(gbrainHome(env), "autopilot.pid"), + join(homedir(), ".gbrain", "autopilot.pid"), + ]; + for (const lp of lockPaths) { + if (existsSync(lp)) return { active: true, signal: `lock:${lp}` }; + } + // Primary signal: a live `gbrain autopilot` process. + const running = (probe.processRunning ?? defaultProcessRunning)(); + if (running) return { active: true, signal: "process:gbrain autopilot" }; + return { active: false, signal: null }; +} + +function defaultProcessRunning(): boolean { + // No reliable pgrep on Windows; rely on the lock-file signal there. + if (process.platform === "win32") return false; + const r = spawnSync("pgrep", ["-f", "gbrain autopilot"], { encoding: "utf-8", timeout: 3_000 }); + return r.status === 0 && (r.stdout || "").trim().length > 0; +} + +// ── Capability detection (E4 + Codex: per-process memo, no persistent cache) ─ +// +// No structured capability command exists and subcommand --help is generic, so +// --keep-storage support can't be probed reliably; default unsupported. Memoize +// per process (keyed to the resolved gbrain identity) rather than persisting a +// cross-run cache — Codex flagged stale persistent caches, and the probe is cheap. + +let _keepStorageMemo: { key: string; value: boolean } | undefined; + +function gbrainIdentity(env: NodeJS.ProcessEnv): string { + const r = spawnSync("gbrain", ["--version"], { + encoding: "utf-8", + timeout: 3_000, + shell: NEEDS_SHELL_ON_WINDOWS, + env, + }); + return (r.stdout || "").trim() || "unknown"; +} + +export function gbrainSupportsKeepStorage(env: NodeJS.ProcessEnv = process.env): boolean { + const key = gbrainIdentity(env); + if (_keepStorageMemo && _keepStorageMemo.key === key) return _keepStorageMemo.value; + let value = false; + for (const args of [["sources", "remove", "--help"], ["--help"]]) { + try { + if (/--keep-storage/.test(execGbrainText(args, { baseEnv: env, timeout: 5_000 }))) { + value = true; + break; + } + } catch { + // generic/empty help or non-zero exit → treat as unsupported + } + } + _keepStorageMemo = { key, value }; + return value; +} + +/** Test-only: reset the per-process capability memo. */ +export function _resetCapabilityMemo(): void { + _keepStorageMemo = undefined; +} + +// ── Destructive-op decisions ──────────────────────────────────────────────── + +/** + * Fetch + normalize the source list. Throws on read/parse failure so callers can + * distinguish "couldn't read" (fail closed) from "empty list" (source absent). + * Injectable for hermetic tests. + */ +export function fetchSources(env: NodeJS.ProcessEnv = process.env): GbrainSourceRow[] { + const raw = execGbrainJson(["sources", "list", "--json"], { baseEnv: env }); + if (raw === null) throw new Error("gbrain sources list returned no JSON"); + return parseSourcesList(raw); +} + +export interface RemoveDecision { + allow: boolean; + /** Extra args to append to `sources remove` (e.g. --keep-storage). */ + extraArgs: string[]; + reason: string; +} + +/** + * Decide whether `sources remove ` is safe, and with what flags. + * + * Fail-closed cases (allow=false): + * - sources list unreadable/unparseable (can't prove the row is safe). + * - the row is user-managed (remote_url set AND local_path outside gbrain's + * clones) and gbrain has no --keep-storage to protect the files. + * + * Allowed: absent row (no-op), gbrain-managed (inside clones), or path-managed + * without a remote_url (gbrain's remove won't touch an outside-clones path that + * it didn't clone). --keep-storage is appended whenever supported, as extra armor. + */ +export interface DecideRemoveOpts { + /** Override capability detection (tests / cached caps). */ + keepStorage?: boolean; + /** Override the source-list fetch (tests). Throwing simulates a read failure. */ + fetchRows?: (env: NodeJS.ProcessEnv) => GbrainSourceRow[]; +} + +export function decideSourceRemove( + sourceId: string, + env: NodeJS.ProcessEnv = process.env, + opts: DecideRemoveOpts = {}, +): RemoveDecision { + const keepStorage = opts.keepStorage ?? gbrainSupportsKeepStorage(env); + const extra = keepStorage ? ["--keep-storage"] : []; + + let rows: GbrainSourceRow[]; + try { + rows = (opts.fetchRows ?? fetchSources)(env); + } catch { + return { allow: false, extraArgs: [], reason: "could not read sources list; refusing remove (fail closed)" }; + } + + const row = rows.find((r) => r.id === sourceId); + if (!row) return { allow: true, extraArgs: extra, reason: "source absent (no-op)" }; + + const remoteUrl = row.config?.remote_url; + const userManaged = + !!remoteUrl && !!row.local_path && !clonesDirs(env).some((d) => isInside(row.local_path!, d)); + + if (userManaged) { + if (keepStorage) { + return { allow: true, extraArgs: ["--keep-storage"], reason: "user-managed; --keep-storage protects files" }; + } + return { + allow: false, + extraArgs: [], + reason: + `refusing remove of user-managed source "${sourceId}" (remote_url set, local_path ` + + `${row.local_path} outside gbrain clones) — this gbrain has no --keep-storage to ` + + `protect the working tree. Upgrade gbrain or remove the source manually.`, + }; + } + + return { allow: true, extraArgs: extra, reason: "gbrain-managed or path-managed without remote_url" }; +} + +export interface SyncDecision { + allow: boolean; + reason: string; +} + +/** + * Decide whether `sync --strategy code --source ` is safe to run. + * + * A source with a remote_url can trigger gbrain's auto-reclone, the ungated + * rm-rf path behind the data loss (gbrain #1526). Require an explicit + * --allow-reclone opt-in for URL-managed sources. Read failure here is NOT + * itself destructive, so it fails open (proceed) — the autopilot guard, checked + * first, is the primary protection against the race that caused the loss. + */ +export function decideCodeSync( + sourceId: string, + env: NodeJS.ProcessEnv = process.env, + allowReclone = false, + fetchRows: (env: NodeJS.ProcessEnv) => GbrainSourceRow[] = fetchSources, +): SyncDecision { + let rows: GbrainSourceRow[]; + try { + rows = fetchRows(env); + } catch { + return { allow: true, reason: "sources unreadable; proceeding (sync read is non-destructive)" }; + } + const row = rows.find((r) => r.id === sourceId); + if (row?.config?.remote_url && !allowReclone) { + return { + allow: false, + reason: + `source "${sourceId}" is URL-managed (remote_url set); sync may auto-reclone and ` + + `delete the working tree. Re-run /sync-gbrain with --allow-reclone to proceed.`, + }; + } + return { allow: true, reason: "no remote_url, or reclone explicitly allowed" }; +} diff --git a/lib/gbrain-local-status.ts b/lib/gbrain-local-status.ts index ae760067b..f6332cf6b 100644 --- a/lib/gbrain-local-status.ts +++ b/lib/gbrain-local-status.ts @@ -35,7 +35,7 @@ import { } from "fs"; import { homedir } from "os"; import { dirname, join } from "path"; -import { buildGbrainEnv } from "./gbrain-exec"; +import { buildGbrainEnv, NEEDS_SHELL_ON_WINDOWS } from "./gbrain-exec"; export type LocalEngineStatus = | "ok" @@ -113,6 +113,7 @@ export function resolveGbrainBin(env?: NodeJS.ProcessEnv): string | null { timeout: 2_000, stdio: ["ignore", "ignore", "ignore"], env: e, + shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows }); result = "gbrain"; } catch { @@ -135,6 +136,7 @@ export function readGbrainVersion(env?: NodeJS.ProcessEnv): string { timeout: 2_000, stdio: ["ignore", "pipe", "ignore"], env: e, + shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows }); result = out.trim().split("\n")[0] || ""; } catch { @@ -241,6 +243,7 @@ function freshClassify(env?: NodeJS.ProcessEnv): LocalEngineStatus { timeout: PROBE_TIMEOUT_MS, stdio: ["ignore", "pipe", "pipe"], env: buildGbrainEnv({ baseEnv: env ?? process.env }), + shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows }); return "ok"; } catch (err) { diff --git a/lib/gbrain-sources.ts b/lib/gbrain-sources.ts index c8ffbad5a..8856b5215 100644 --- a/lib/gbrain-sources.ts +++ b/lib/gbrain-sources.ts @@ -11,6 +11,7 @@ import { execFileSync, spawnSync } from "child_process"; import { withErrorContext } from "./gstack-memory-helpers"; +import { NEEDS_SHELL_ON_WINDOWS } from "./gbrain-exec"; export interface SourceState { /** "absent" — id not registered. "match" — id at expected path. "drift" — id at different path. */ @@ -26,6 +27,37 @@ export interface EnsureResult { state: SourceState; } +/** + * One row of `gbrain sources list --json`. `config.remote_url` distinguishes + * URL-managed sources (gbrain owns the clone, may auto-reclone) from + * path-managed ones (user owns the working tree) — load-bearing for the #1734 + * destructive-op guards. + */ +export interface GbrainSourceRow { + id?: string; + local_path?: string; + page_count?: number; + config?: { remote_url?: string | null } | null; +} + +/** + * Normalize `gbrain sources list --json` output to an array of source rows. + * + * gbrain has shipped two shapes: a wrapped `{ sources: [...] }` object (v0.20+) + * and, in older/other variants, a bare top-level array. #1576 was a crash when a + * reader assumed one shape; the parse is centralized here so every reader + * (probeSource, sourcePageCount, sourceLocalPath, the #1734 remote_url audit) + * agrees on the shape in ONE place. Returns [] for null/garbage rather than + * throwing — callers treat "no rows" as absent. + */ +export function parseSourcesList(raw: unknown): GbrainSourceRow[] { + if (Array.isArray(raw)) return raw as GbrainSourceRow[]; + if (raw && typeof raw === "object" && Array.isArray((raw as { sources?: unknown }).sources)) { + return (raw as { sources: GbrainSourceRow[] }).sources; + } + return []; +} + export interface EnsureOptions { /** Pass --federated to `gbrain sources add`. Default false. */ federated?: boolean; @@ -56,6 +88,7 @@ export function probeSource(id: string, env?: NodeJS.ProcessEnv): SourceState { timeout: 30_000, stdio: ["ignore", "pipe", "pipe"], env, + shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows }); } catch (err) { const e = err as NodeJS.ErrnoException & { stderr?: Buffer }; @@ -69,14 +102,14 @@ export function probeSource(id: string, env?: NodeJS.ProcessEnv): SourceState { throw err; } - let parsed: { sources?: Array<{ id?: string; local_path?: string }> }; + let parsed: unknown; try { parsed = JSON.parse(stdout); } catch (err) { throw new Error(`gbrain sources list returned non-JSON output: ${(err as Error).message}`); } - const sources = parsed.sources || []; + const sources = parseSourcesList(parsed); const match = sources.find((s) => s.id === id); if (!match) return { status: "absent" }; return { @@ -129,6 +162,7 @@ export async function ensureSourceRegistered( encoding: "utf-8", timeout: 30_000, env, + shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows }); if (rm.status !== 0) { throw new Error(`gbrain sources remove ${id} failed: ${rm.stderr || rm.stdout || `exit ${rm.status}`}`); @@ -142,6 +176,7 @@ export async function ensureSourceRegistered( encoding: "utf-8", timeout: 30_000, env, + shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows }); if (add.status !== 0) { throw new Error(`gbrain sources add ${id} failed: ${add.stderr || add.stdout || `exit ${add.status}`}`); @@ -167,14 +202,14 @@ export function sourcePageCount(id: string, env?: NodeJS.ProcessEnv): number | n timeout: 30_000, stdio: ["ignore", "pipe", "pipe"], env, + shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows }); } catch { return null; } try { - const parsed = JSON.parse(stdout) as { sources?: Array<{ id?: string; page_count?: number }> }; - const match = (parsed.sources || []).find((s) => s.id === id); + const match = parseSourcesList(JSON.parse(stdout)).find((s) => s.id === id); if (!match) return null; if (typeof match.page_count !== "number") return null; return match.page_count; diff --git a/package.json b/package.json index 80d437b98..280299e0c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.54.0.0", + "version": "1.55.0.0", "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/plan-tune/SKILL.md b/plan-tune/SKILL.md index 8e61abc58..41c45342e 100644 --- a/plan-tune/SKILL.md +++ b/plan-tune/SKILL.md @@ -2,7 +2,7 @@ name: plan-tune preamble-tier: 2 version: 1.0.0 -description: Self-tuning question sensitivity + developer psychographic for gstack (v1: observational). (gstack) +description: "Self-tuning question sensitivity + developer psychographic for gstack (v1: observational). (gstack)" triggers: - tune questions - stop asking me that diff --git a/scripts/gen-skill-docs.ts b/scripts/gen-skill-docs.ts index ac38357c1..5fea07713 100644 --- a/scripts/gen-skill-docs.ts +++ b/scripts/gen-skill-docs.ts @@ -356,6 +356,28 @@ export function buildWhenToInvokeSection(parts: CatalogParts): string { return lines.join('\n'); } +/** + * Render a string as a YAML inline scalar value (the text after `key: `), + * quoting only when a plain scalar would be invalid or ambiguous. + * + * The bug this guards (#1778): a description like "Ship workflow: detect..." + * emitted as a plain scalar has an interior ": " that a strict YAML parser + * (Codex/OpenAI skill loading) reads as a nested mapping and rejects with + * "mapping values are not allowed in this context". When quoting is needed we + * fall back to JSON.stringify, which produces a double-quoted scalar that YAML + * accepts verbatim (YAML is a superset of JSON for flow scalars). Strings that + * are already valid plain scalars pass through unchanged to keep regen diffs small. + */ +export function toYamlInlineScalar(s: string): string { + const needsQuote = + s.length === 0 || + s !== s.trim() || // leading/trailing whitespace + /:(\s|$)/.test(s) || // "foo: bar" / trailing colon → mapping ambiguity + /\s#/.test(s) || // " #" → inline comment + /^[\s>|&*!%@`"'#,\[\]{}?-]/.test(s); // leading YAML indicator char + return needsQuote ? JSON.stringify(s) : s; +} + /** * Apply catalog trim to a SKILL.md body: * - shorten frontmatter `description:` to lead + (gstack) @@ -397,8 +419,16 @@ export function applyCatalogTrim(content: string, skillName: string): { content: // Replace description in frontmatter — keep trailing newline so the next // YAML field doesn't collide on the same line as the description value. + // Quote the value when it would be an invalid YAML plain scalar (the common + // case: an interior ": " like "Ship workflow: detect..." which a strict YAML + // parser reads as a nested mapping and rejects — #1778). toYamlInlineScalar + // only quotes when needed, so descriptions without special chars stay plain. const newDesc = buildTrimmedDescription(parts); - const newFrontmatter = frontmatter.replace(descMatch[0], `description: ${newDesc}\n`); + // Function replacer (not a string) so a `$` in the description — e.g. a future + // skill referencing `$B`/`$D` — can't be interpreted as a `$&`/`$1` replacement + // pattern and silently corrupt the frontmatter. + const newDescLine = `description: ${toYamlInlineScalar(newDesc)}\n`; + const newFrontmatter = frontmatter.replace(descMatch[0], () => newDescLine); let newContent = '---\n' + newFrontmatter + content.slice(fmEnd); // Insert body section after frontmatter (after the closing ---\n and any diff --git a/setup-gbrain/SKILL.md b/setup-gbrain/SKILL.md index 2e2acd834..cad27fcec 100644 --- a/setup-gbrain/SKILL.md +++ b/setup-gbrain/SKILL.md @@ -2,7 +2,7 @@ name: setup-gbrain preamble-tier: 2 version: 1.0.0 -description: Set up gbrain for this coding agent: install the CLI, initialize a local PGLite or Supabase brain, register MCP, capture per-remote trust policy. (gstack) +description: "Set up gbrain for this coding agent: install the CLI, initialize a local PGLite or Supabase brain, register MCP, capture per-remote trust policy. (gstack)" triggers: - setup gbrain - install gbrain diff --git a/ship/SKILL.md b/ship/SKILL.md index 4f7aaf239..ecf203787 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -2,7 +2,7 @@ name: ship preamble-tier: 4 version: 1.0.0 -description: Ship workflow: detect + merge base branch, run tests, review diff, bump VERSION, update CHANGELOG, commit, push, create PR. (gstack) +description: "Ship workflow: detect + merge base branch, run tests, review diff, bump VERSION, update CHANGELOG, commit, push, create PR. (gstack)" allowed-tools: - Bash - Read diff --git a/sync-gbrain/SKILL.md b/sync-gbrain/SKILL.md index 0c21b8d5a..4a3a5bc1d 100644 --- a/sync-gbrain/SKILL.md +++ b/sync-gbrain/SKILL.md @@ -990,6 +990,12 @@ file globs. Run `/sync-gbrain` after meaningful code changes; for ongoing auto-sync across all worktrees, run `gbrain autopilot --install` once per machine — gbrain's daemon handles incremental refresh on a schedule. +Safety: don't run `/sync-gbrain` while `gbrain autopilot` is active — the +orchestrator refuses destructive source ops when it detects a running autopilot +to avoid racing it (#1734). Prefer registering user repos with `gbrain sources +add --path ` (no `--url`): URL-managed sources can auto-reclone, and the +sync code walk for them requires an explicit `--allow-reclone` opt-in. + ``` diff --git a/sync-gbrain/SKILL.md.tmpl b/sync-gbrain/SKILL.md.tmpl index 6d9700aac..6f9d47752 100644 --- a/sync-gbrain/SKILL.md.tmpl +++ b/sync-gbrain/SKILL.md.tmpl @@ -295,6 +295,12 @@ file globs. Run `/sync-gbrain` after meaningful code changes; for ongoing auto-sync across all worktrees, run `gbrain autopilot --install` once per machine — gbrain's daemon handles incremental refresh on a schedule. +Safety: don't run `/sync-gbrain` while `gbrain autopilot` is active — the +orchestrator refuses destructive source ops when it detects a running autopilot +to avoid racing it (#1734). Prefer registering user repos with `gbrain sources +add --path ` (no `--url`): URL-managed sources can auto-reclone, and the +sync code walk for them requires an explicit `--allow-reclone` opt-in. + ``` diff --git a/test/catalog-mode-full.test.ts b/test/catalog-mode-full.test.ts index 009db33ee..c964f35ab 100644 --- a/test/catalog-mode-full.test.ts +++ b/test/catalog-mode-full.test.ts @@ -60,7 +60,9 @@ describe('--catalog-mode=full opt-out behavior (smoke)', () => { test('--catalog-mode=full produces multi-line description in frontmatter', () => { // Save the trim'd state so we can restore it. const trimmedShip = fs.readFileSync(SHIP_SKILL, 'utf-8'); - expect(trimmedShip).toMatch(/^description: Ship workflow:[^\n]*\(gstack\)\n/m); + // #1778: the trimmed ship description has an interior colon ("Ship workflow:") + // and is now YAML-quoted — tolerate the optional surrounding quotes. + expect(trimmedShip).toMatch(/^description: "?Ship workflow:[^\n]*\(gstack\)"?\n/m); try { // Run with --catalog-mode=full. Mutates working tree. @@ -100,7 +102,8 @@ describe('--catalog-mode=full opt-out behavior (smoke)', () => { } // Sanity-check the restored state matches what we saw at the start. const restoredShip = fs.readFileSync(SHIP_SKILL, 'utf-8'); - expect(restoredShip).toMatch(/^description: Ship workflow:[^\n]*\(gstack\)\n/m); + // #1778: restored trim state has the YAML-quoted (interior-colon) description. + expect(restoredShip).toMatch(/^description: "?Ship workflow:[^\n]*\(gstack\)"?\n/m); } }, 180_000); diff --git a/test/catalog-trim.test.ts b/test/catalog-trim.test.ts index e58678603..82e46bdfe 100644 --- a/test/catalog-trim.test.ts +++ b/test/catalog-trim.test.ts @@ -227,8 +227,10 @@ Original body content here. const result = applyCatalogTrim(minimalSkill, 'example'); expect(result).not.toBeNull(); const { content, parts } = result!; - // Frontmatter description is now ONE line ending with (gstack) - expect(content).toMatch(/^description: Example skill:[^\n]*\(gstack\)\n/m); + // Frontmatter description is now ONE line ending with (gstack). #1778: a + // description with an interior colon ("Example skill:") is YAML-quoted, so + // the value is wrapped in double quotes — tolerate the optional quotes. + expect(content).toMatch(/^description: "?Example skill:[^\n]*\(gstack\)"?\n/m); // Body has the When to invoke section expect(content).toContain('## When to invoke this skill'); expect(content).toContain('Use when asked to do an example task.'); @@ -257,7 +259,8 @@ Original body content here. expect(result).not.toBeNull(); expect(result!.content).not.toMatch(/\(gstack\)preamble-tier/); expect(result!.content).not.toMatch(/\(gstack\)allowed-tools/); - expect(result!.content).toMatch(/\(gstack\)\n[a-z-]+:/); + // #1778: optional closing quote when the description was YAML-quoted. + expect(result!.content).toMatch(/\(gstack\)"?\n[a-z-]+:/); }); test('returns null on content without proper frontmatter', () => { diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index 4f7aaf239..ecf203787 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -2,7 +2,7 @@ name: ship preamble-tier: 4 version: 1.0.0 -description: Ship workflow: detect + merge base branch, run tests, review diff, bump VERSION, update CHANGELOG, commit, push, create PR. (gstack) +description: "Ship workflow: detect + merge base branch, run tests, review diff, bump VERSION, update CHANGELOG, commit, push, create PR. (gstack)" allowed-tools: - Bash - Read diff --git a/test/gbrain-detect-install.test.ts b/test/gbrain-detect-install.test.ts index 6eb7ce2db..b9c82c155 100644 --- a/test/gbrain-detect-install.test.ts +++ b/test/gbrain-detect-install.test.ts @@ -204,14 +204,30 @@ describe('gstack-gbrain-install D19 PATH-shadow validation', () => { } test('passes when install-dir version matches `gbrain --version` on PATH', () => { + // Version must be >= MIN_GBRAIN_VERSION (0.20.0) floor (#1744). + const installDir = seedInstallDir('0.41.29'); + const fakeBin = seedFakeGbrainBinary('0.41.29'); + try { + const r = run(INSTALL, ['--validate-only', '--install-dir', installDir], { + env: { PATH: `${fakeBin}:${SAFE_PATH}` }, + }); + expect(r.status).toBe(0); + expect(r.stdout).toContain('installed gbrain 0.41.29'); + } finally { + fs.rmSync(installDir, { recursive: true, force: true }); + fs.rmSync(fakeBin, { recursive: true, force: true }); + } + }); + + test('hard-fails (exit 3) when the installed gbrain is below the version floor (#1744)', () => { const installDir = seedInstallDir('0.18.2'); const fakeBin = seedFakeGbrainBinary('0.18.2'); try { const r = run(INSTALL, ['--validate-only', '--install-dir', installDir], { env: { PATH: `${fakeBin}:${SAFE_PATH}` }, }); - expect(r.status).toBe(0); - expect(r.stdout).toContain('installed gbrain 0.18.2'); + expect(r.status).toBe(3); + expect(r.stderr).toContain('below the minimum gstack-tested version'); } finally { fs.rmSync(installDir, { recursive: true, force: true }); fs.rmSync(fakeBin, { recursive: true, force: true }); @@ -219,8 +235,8 @@ describe('gstack-gbrain-install D19 PATH-shadow validation', () => { }); test('tolerates a leading "v" in `gbrain --version` output', () => { - const installDir = seedInstallDir('0.18.2'); - const fakeBin = seedFakeGbrainBinary('v0.18.2'); + const installDir = seedInstallDir('0.41.29'); + const fakeBin = seedFakeGbrainBinary('v0.41.29'); try { const r = run(INSTALL, ['--validate-only', '--install-dir', installDir], { env: { PATH: `${fakeBin}:${SAFE_PATH}` }, diff --git a/test/gbrain-guards.test.ts b/test/gbrain-guards.test.ts new file mode 100644 index 000000000..0740148f9 --- /dev/null +++ b/test/gbrain-guards.test.ts @@ -0,0 +1,140 @@ +import { describe, test, expect, afterEach } from "bun:test"; +import * as fs from "fs"; +import * as os from "os"; +import { join } from "path"; +import { + detectAutopilot, + decideSourceRemove, + decideCodeSync, + isInside, + _resetCapabilityMemo, + type GbrainSourceRow, +} from "../lib/gbrain-guards"; + +const HOME = os.homedir(); +const clonesPath = (name: string) => join(HOME, ".gbrain", "clones", name); + +afterEach(() => _resetCapabilityMemo()); + +// ── #1734 autopilot detection (E1: affirmative multi-signal) ──────────────── +describe("detectAutopilot", () => { + test("refuses on a present lock file (secondary signal)", () => { + const tmp = fs.mkdtempSync(join(os.tmpdir(), "ap-")); + const lock = join(tmp, "autopilot.lock"); + fs.writeFileSync(lock, ""); + const r = detectAutopilot(process.env, { lockPaths: [lock], processRunning: () => false }); + expect(r.active).toBe(true); + expect(r.signal).toContain("lock:"); + }); + + test("refuses on a live autopilot process (primary signal)", () => { + const r = detectAutopilot(process.env, { lockPaths: [], processRunning: () => true }); + expect(r.active).toBe(true); + expect(r.signal).toBe("process:gbrain autopilot"); + }); + + test("proceeds when no signal fires (never blanket-refuses)", () => { + const r = detectAutopilot(process.env, { lockPaths: [], processRunning: () => false }); + expect(r.active).toBe(false); + expect(r.signal).toBeNull(); + }); +}); + +// ── #1734 remove safety (E7: fail closed on user-managed without keep-storage) ─ +describe("decideSourceRemove", () => { + const rows = (extra: GbrainSourceRow[] = []): GbrainSourceRow[] => [ + { id: "gbrain-managed", local_path: clonesPath("repo"), config: { remote_url: "https://x/r.git" } }, + { id: "user-managed", local_path: "/tmp/user-repo", config: { remote_url: "https://x/r.git" } }, + { id: "path-managed", local_path: "/tmp/path-repo" }, // no remote_url + ...extra, + ]; + const fetchRows = (extra?: GbrainSourceRow[]) => () => rows(extra); + + test("absent source → allow (no-op)", () => { + const d = decideSourceRemove("nope", process.env, { keepStorage: false, fetchRows: fetchRows() }); + expect(d.allow).toBe(true); + expect(d.reason).toContain("absent"); + }); + + test("user-managed + no --keep-storage → FAIL CLOSED", () => { + const d = decideSourceRemove("user-managed", process.env, { keepStorage: false, fetchRows: fetchRows() }); + expect(d.allow).toBe(false); + expect(d.reason).toContain("user-managed"); + }); + + test("user-managed + --keep-storage supported → allow with flag", () => { + const d = decideSourceRemove("user-managed", process.env, { keepStorage: true, fetchRows: fetchRows() }); + expect(d.allow).toBe(true); + expect(d.extraArgs).toContain("--keep-storage"); + }); + + test("gbrain-managed (inside clones) → allow even without keep-storage", () => { + const d = decideSourceRemove("gbrain-managed", process.env, { keepStorage: false, fetchRows: fetchRows() }); + expect(d.allow).toBe(true); + }); + + test("path-managed without remote_url → allow (normal --path case)", () => { + const d = decideSourceRemove("path-managed", process.env, { keepStorage: false, fetchRows: fetchRows() }); + expect(d.allow).toBe(true); + }); + + test("sources unreadable → FAIL CLOSED", () => { + const d = decideSourceRemove("user-managed", process.env, { + keepStorage: false, + fetchRows: () => { throw new Error("boom"); }, + }); + expect(d.allow).toBe(false); + expect(d.reason).toContain("fail closed"); + }); +}); + +// ── #1734 reclone guard (E-level: require --allow-reclone for URL-managed) ─── +describe("decideCodeSync", () => { + const rows: GbrainSourceRow[] = [ + { id: "url-managed", local_path: "/tmp/u", config: { remote_url: "https://x/r.git" } }, + { id: "plain", local_path: "/tmp/p" }, + ]; + const fetch = () => rows; + + test("URL-managed + no --allow-reclone → refuse", () => { + const d = decideCodeSync("url-managed", process.env, false, fetch); + expect(d.allow).toBe(false); + expect(d.reason).toContain("auto-reclone"); + }); + + test("URL-managed + --allow-reclone → allow", () => { + const d = decideCodeSync("url-managed", process.env, true, fetch); + expect(d.allow).toBe(true); + }); + + test("no remote_url → allow", () => { + const d = decideCodeSync("plain", process.env, false, fetch); + expect(d.allow).toBe(true); + }); + + test("sources unreadable → fail OPEN (sync read is non-destructive)", () => { + const d = decideCodeSync("url-managed", process.env, false, () => { throw new Error("boom"); }); + expect(d.allow).toBe(true); + }); +}); + +// ── path containment uses realpath (symlink can't smuggle a delete out) ────── +describe("isInside", () => { + test("plain path inside dir", () => { + expect(isInside("/a/b/c", "/a/b")).toBe(true); + expect(isInside("/a/x", "/a/b")).toBe(false); + }); + + test("sibling-prefix is not 'inside' (clonesX vs clones)", () => { + expect(isInside("/a/clones-evil/x", "/a/clones")).toBe(false); + }); + + test("symlink pointing outside resolves outside", () => { + const base = fs.mkdtempSync(join(os.tmpdir(), "clones-")); + const outside = fs.mkdtempSync(join(os.tmpdir(), "outside-")); + const link = join(base, "sneaky"); + fs.symlinkSync(outside, link); + // link lives under base, but realpath resolves to `outside` → not inside base. + expect(isInside(link, base)).toBe(false); + }); +}); diff --git a/test/gbrain-sources-parse.test.ts b/test/gbrain-sources-parse.test.ts new file mode 100644 index 000000000..ce198b8e5 --- /dev/null +++ b/test/gbrain-sources-parse.test.ts @@ -0,0 +1,49 @@ +import { describe, test, expect } from "bun:test"; +import { parseSourcesList } from "../lib/gbrain-sources"; + +// #1576 hardening: `gbrain sources list --json` has shipped two shapes — a +// wrapped `{ sources: [...] }` object (v0.20+) and a bare top-level array. +// parseSourcesList is the single place that normalizes both, so every reader +// (probeSource, sourcePageCount, sourceLocalPath, the #1734 remote_url audit) +// agrees on the shape. These tests pin both shapes plus the garbage paths. +describe("parseSourcesList", () => { + const rows = [ + { id: "a", local_path: "/x", page_count: 3 }, + { id: "b", local_path: "/y", config: { remote_url: "https://example.com/r.git" } }, + ]; + + test("wrapped { sources: [...] } shape", () => { + expect(parseSourcesList({ sources: rows })).toEqual(rows); + }); + + test("bare top-level array shape", () => { + expect(parseSourcesList(rows)).toEqual(rows); + }); + + test("both shapes yield identical rows (shape-independent)", () => { + expect(parseSourcesList({ sources: rows })).toEqual(parseSourcesList(rows)); + }); + + test("null / undefined → empty array (no throw)", () => { + expect(parseSourcesList(null)).toEqual([]); + expect(parseSourcesList(undefined)).toEqual([]); + }); + + test("object without sources key → empty array", () => { + expect(parseSourcesList({ pages: [] })).toEqual([]); + }); + + test("sources key present but not an array → empty array", () => { + expect(parseSourcesList({ sources: "oops" })).toEqual([]); + }); + + test("scalar garbage → empty array", () => { + expect(parseSourcesList("nope")).toEqual([]); + expect(parseSourcesList(42)).toEqual([]); + }); + + test("preserves config.remote_url for the #1734 audit", () => { + const parsed = parseSourcesList({ sources: rows }); + expect(parsed.find((r) => r.id === "b")?.config?.remote_url).toBe("https://example.com/r.git"); + }); +}); diff --git a/test/gbrain-spawn-windows-shell.test.ts b/test/gbrain-spawn-windows-shell.test.ts new file mode 100644 index 000000000..d968d2f68 --- /dev/null +++ b/test/gbrain-spawn-windows-shell.test.ts @@ -0,0 +1,45 @@ +import { describe, test, expect } from "bun:test"; +import * as fs from "fs"; +import * as path from "path"; + +const ROOT = path.resolve(import.meta.dir, ".."); +const read = (rel: string) => fs.readFileSync(path.join(ROOT, rel), "utf-8"); + +// #1731 tripwire. Windows can't spawn the `gbrain` shim (gbrain.cmd) or the bash +// shebang script gstack-brain-sync without a shell; the fix gates `shell: true` +// behind NEEDS_SHELL_ON_WINDOWS. These static checks fail CI if a refactor adds +// a gbrain/brain-sync child spawn without the Windows shell flag, since macOS/ +// Linux CI can't exercise the Windows path at runtime. +describe("#1731 gbrain spawns carry the Windows shell flag", () => { + test("NEEDS_SHELL_ON_WINDOWS is platform-gated in gbrain-exec.ts", () => { + const src = read("lib/gbrain-exec.ts"); + expect(src).toMatch(/export const NEEDS_SHELL_ON_WINDOWS\s*=\s*process\.platform === "win32"/); + }); + + // Every direct `gbrain` child spawn in these files must be matched by a + // shell:NEEDS_SHELL_ON_WINDOWS flag. Count openers vs flags as a cheap, + // refactor-resistant invariant. + const gbrainSpawnFiles = [ + "lib/gbrain-exec.ts", + "lib/gbrain-sources.ts", + "lib/gbrain-local-status.ts", + ]; + for (const rel of gbrainSpawnFiles) { + test(`${rel}: every gbrain spawn has shell:NEEDS_SHELL_ON_WINDOWS`, () => { + const src = read(rel); + const spawnOpeners = src.match(/(spawnSync|spawn|execFileSync)\("gbrain"/g)?.length ?? 0; + const shellFlags = src.match(/shell:\s*NEEDS_SHELL_ON_WINDOWS/g)?.length ?? 0; + expect(spawnOpeners).toBeGreaterThan(0); + expect(shellFlags).toBeGreaterThanOrEqual(spawnOpeners); + }); + } + + test("orchestrator brain-sync spawns carry the Windows shell flag", () => { + const src = read("bin/gstack-gbrain-sync.ts"); + const brainSyncSpawns = src.match(/spawnSync\(brainSyncPath,/g)?.length ?? 0; + expect(brainSyncSpawns).toBe(2); + // Both spawnSync(brainSyncPath, ...) blocks must include the shell flag. + const withShell = src.match(/spawnSync\(brainSyncPath,[\s\S]*?shell:\s*NEEDS_SHELL_ON_WINDOWS/g)?.length ?? 0; + expect(withShell).toBe(2); + }); +}); diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index 7e3f43c9b..ffe6ed7d6 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -173,12 +173,39 @@ describe('gen-skill-docs', () => { } }); - test('every generated SKILL.md has valid YAML frontmatter', () => { + // #1778: strict YAML parsers (Codex/OpenAI skill loading) reject frontmatter + // whose plain `description:` scalar contains an interior ": " (read as a nested + // mapping). Parse EVERY generated frontmatter block with a strict YAML parser, + // not just string-check that name:/description: exist. + function frontmatterBlock(content: string): string { + expect(content.startsWith('---\n')).toBe(true); + const end = content.indexOf('\n---', 4); + expect(end).toBeGreaterThan(0); + return content.slice(4, end); + } + + test('every generated SKILL.md frontmatter parses as strict YAML', () => { for (const skill of CLAUDE_GENERATED_SKILLS) { const content = fs.readFileSync(path.join(ROOT, skill.dir, 'SKILL.md'), 'utf-8'); - expect(content.startsWith('---\n')).toBe(true); - expect(content).toContain('name:'); - expect(content).toContain('description:'); + const fm = frontmatterBlock(content); + let parsed: any; + expect(() => { parsed = Bun.YAML.parse(fm); }, + `frontmatter for ${skill.dir} must be valid YAML`).not.toThrow(); + expect(typeof parsed?.name).toBe('string'); + expect(typeof parsed?.description).toBe('string'); + } + }); + + test('every generated Codex (.agents/skills) frontmatter parses as strict YAML', () => { + const agentsDir = path.join(ROOT, '.agents', 'skills'); + if (!fs.existsSync(agentsDir)) return; // skip if external hosts not generated + for (const entry of fs.readdirSync(agentsDir, { withFileTypes: true })) { + if (!entry.isDirectory()) continue; + const mdPath = path.join(agentsDir, entry.name, 'SKILL.md'); + if (!fs.existsSync(mdPath)) continue; + const fm = frontmatterBlock(fs.readFileSync(mdPath, 'utf-8')); + expect(() => Bun.YAML.parse(fm), + `Codex frontmatter for ${entry.name} must be valid YAML`).not.toThrow(); } }); diff --git a/test/jsonl-merge.test.ts b/test/jsonl-merge.test.ts new file mode 100644 index 000000000..20bb7d877 --- /dev/null +++ b/test/jsonl-merge.test.ts @@ -0,0 +1,96 @@ +import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; +import { execFileSync } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const DRIVER = path.join(ROOT, 'bin', 'gstack-jsonl-merge'); + +let tmpDir: string; + +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-jsonl-merge-')); +}); + +afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +/** + * Run the merge driver the way git does: `driver `. + * The driver writes the merged result back to the file. Returns that + * file's content. `base`/`ours`/`theirs` are arrays of JSONL lines (the file + * is created from them); pass `null` to omit a file entirely (git passes an + * absent path for an added file, which the driver must tolerate). + */ +function runMerge( + base: string[] | null, + ours: string[] | null, + theirs: string[] | null, +): string { + const write = (name: string, lines: string[] | null): string => { + const p = path.join(tmpDir, name); + if (lines === null) return path.join(tmpDir, `${name}.absent`); + fs.writeFileSync(p, lines.length ? lines.join('\n') + '\n' : ''); + return p; + }; + const basePath = write('base', base); + const oursPath = write('ours', ours); + const theirsPath = write('theirs', theirs); + execFileSync(DRIVER, [basePath, oursPath, theirsPath], { + encoding: 'utf-8', + timeout: 15000, + }); + return fs.readFileSync(oursPath, 'utf-8'); +} + +describe('gstack-jsonl-merge', () => { + test('equal-ts entries resolve identically regardless of side (convergence)', () => { + // Two machines append a different event in the same second, then each + // merges the other's push. Machine A sees its own line as "ours"; machine + // B sees the same line as "theirs". The merge must produce the same file + // on both, or the repos diverge and never reconcile. + const a = '{"ts":"2026-05-28T10:00:00Z","event":"a"}'; + const b = '{"ts":"2026-05-28T10:00:00Z","event":"b"}'; + + const machineA = runMerge([], [a], [b]); // a = ours, b = theirs + const machineB = runMerge([], [b], [a]); // b = ours, a = theirs + + expect(machineA).toBe(machineB); + // Both lines survive. + expect(machineA).toContain('"event":"a"'); + expect(machineA).toContain('"event":"b"'); + }); + + test('non-timestamped lines also resolve identically regardless of side', () => { + const a = '{"event":"a"}'; // no ts -> hash-ordered + const b = '{"event":"b"}'; + expect(runMerge([], [a], [b])).toBe(runMerge([], [b], [a])); + }); + + test('plain (non-JSON) lines resolve identically regardless of side', () => { + expect(runMerge([], ['zebra'], ['apple'])).toBe( + runMerge([], ['apple'], ['zebra']), + ); + }); + + test('exact-duplicate lines are deduped', () => { + const line = '{"ts":"2026-05-28T10:00:00Z","event":"a"}'; + const out = runMerge([line], [line], [line]); + expect(out.trimEnd().split('\n')).toEqual([line]); + }); + + test('timestamped entries sort ascending by ts', () => { + const early = '{"ts":"2026-05-28T09:00:00Z","event":"early"}'; + const late = '{"ts":"2026-05-28T11:00:00Z","event":"late"}'; + const out = runMerge([], [late], [early]).trimEnd().split('\n'); + expect(out).toEqual([early, late]); + }); + + test('absent ours/theirs files are tolerated (added-file merge)', () => { + const a = '{"ts":"2026-05-28T10:00:00Z","event":"a"}'; + const out = runMerge(null, [a], null); + expect(out.trimEnd()).toBe(a); + }); +}); diff --git a/test/memory-ingest-timeout.test.ts b/test/memory-ingest-timeout.test.ts new file mode 100644 index 000000000..f4713fafb --- /dev/null +++ b/test/memory-ingest-timeout.test.ts @@ -0,0 +1,27 @@ +import { describe, test, expect } from "bun:test"; +import { resolveImportTimeoutMs } from "../bin/gstack-memory-ingest"; + +// #1611: the gbrain import timeout is configurable via GSTACK_INGEST_TIMEOUT_MS +// (default 30 min) so big-brain --full ingests aren't SIGTERM'd mid-import. +const DEFAULT = 30 * 60 * 1000; + +describe("resolveImportTimeoutMs", () => { + test("unset → 30 min default", () => { + expect(resolveImportTimeoutMs(undefined)).toBe(DEFAULT); + expect(resolveImportTimeoutMs("")).toBe(DEFAULT); + }); + + test("valid override is honored", () => { + expect(resolveImportTimeoutMs("3600000")).toBe(3_600_000); // 1h + expect(resolveImportTimeoutMs("60000")).toBe(60_000); // floor + expect(resolveImportTimeoutMs("86400000")).toBe(86_400_000); // ceiling + }); + + test("invalid / out-of-range → default (no SIGTERM-too-soon footgun)", () => { + expect(resolveImportTimeoutMs("nope")).toBe(DEFAULT); + expect(resolveImportTimeoutMs("0")).toBe(DEFAULT); + expect(resolveImportTimeoutMs("59999")).toBe(DEFAULT); // below 1min floor + expect(resolveImportTimeoutMs("86400001")).toBe(DEFAULT); // above 24h ceiling + expect(resolveImportTimeoutMs("-5")).toBe(DEFAULT); + }); +});