diff --git a/.github/workflows/windows-free-tests.yml b/.github/workflows/windows-free-tests.yml index 67fefcbe9..69e24c7e3 100644 --- a/.github/workflows/windows-free-tests.yml +++ b/.github/workflows/windows-free-tests.yml @@ -116,6 +116,7 @@ jobs: test/setup-windows-fallback.test.ts \ test/build-script-shell-compat.test.ts \ test/docs-config-keys.test.ts \ + test/brain-sync-windows-paths.test.ts \ make-pdf/test/browseClient.test.ts \ make-pdf/test/pdftotext.test.ts shell: bash diff --git a/CHANGELOG.md b/CHANGELOG.md index cc8b86170..b32ceed4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,48 @@ # Changelog +## [1.44.1.0] - 2026-05-24 + +## **Nine community fixes ship in one bundle.** Office-hours session counter works again, iOS QA tunnels survive macOS 26.x, Windows brain-sync stops dropping artifacts, browse server tells you whether the bind failure was a port collision or a sandbox block. + +The fix wave pattern runs its second pass after v1.43.2.0's 15-PR Daegu wave. Nine contributor PRs land in eleven commits plus a merge from new main. Each cherry-pick routes through `git cherry-pick` per-commit so contributor authorship survives in `git log --author`, with `Co-Authored-By` trailers for GitHub's contribution UI. Wave-meta files (VERSION, CHANGELOG, version-only `package.json` bumps) stripped per cherry-pick so the wave owns its own bump cleanly. + +The triage caught a real failure mode mid-flight. An initial scope of 18 PRs went through Codex review as outside voice; Codex flagged that 9 of the 18 had already shipped via v1.43.2.0 or sibling commits. Verified against current main (`bin/gstack-gbrain-sync.ts:404` already wraps `{sources:[...]}`, `browser-manager.ts:30` already has `isCustomChromium`, `server.ts:209` already has `ownsTerminalAgent`). Recompute trimmed the wave from 18 to 9, saving nine empty cherry-picks and nine misleading "landed in" close comments to contributors whose work had already merged via another route. + +### The numbers that matter + +Source: `git log origin/main..HEAD` and `gh pr view --json closingIssuesReferences` per wave PR. + +| Metric | Value | +|----------------------------------------------|------------| +| Community PRs landed | 9 | +| Distinct contributors credited | 9 | +| Issues auto-closed by merge | 4 | +| Files changed | 26 | +| Lines added | 1,651 | +| Lines removed | 114 | +| Wave commits (excluding merge) | 11 | +| Already-shipped PRs caught + politely closed | 9 | +| Paid eval suites that ran (all PASS) | 6 | + +### What this means for contributors + +Your fix lands as a commit with your name in `git log --author=`. If your PR had multiple commits, each lands separately so dates and trailers survive. If your fix was the same as something that shipped via another route in v1.43.2.0, you get a close comment pointing at the CHANGELOG line that credits you by name. The recompute step that catches duplicates is now part of every future fix wave. + +### Itemized changes + +**Added** +- `/investigate` freeze hook resolves on standalone marketplace installs. Falls back through both bundled and standalone freeze-bin paths instead of crashing on a hardcoded `../freeze/` lookup. Closes #1647. Contributed by @Gujiassh via PR #1648. +- `gstack-next-version --version-path` flag plus `.gstack/version-path` config: monorepo VERSION layouts now work. Contributed by @cfeddersen via PR #1627. + +**Fixed** +- `/office-hours` SESSION_COUNT stuck at 0 since v1.0. Writer wrote to legacy `builder-profile.jsonl`, reader read from new `developer-profile.json`. Reader-path auto-migrates existing legacy data on first call; existing users keep their session history. 33 regression tests plus a static-grep invariant pinning the no-legacy-writes contract. Closes #1671, #1677. Contributed by @pryow via PR #1676. +- `gstack-timeline-read --branch "feature/o'hare"` no longer breaks on single-quoted branch names. Filters passed as data, not interpolated into a shell command. Closes #1634. Contributed by @jbetala7 via PR #1635. +- `browse` server localhost bind: distinguishes `EADDRINUSE` (real port collision) from sandbox `EPERM` (Codex/Conductor shell sandbox blocking the bind syscall). Tells the user which one happened. Contributed by @spacegeologist via PR #1664. +- `v1.40.0.0` migration on jq-less machines: defers done-marker until every repair succeeds, instead of writing it unconditionally. Re-runs the migration on next upgrade for users who hit the pre-fix path. 8-case regression test. Closes #1581. Contributed by @stedfn via PR #1589. +- Three Windows brain-sync bugs: backslash vs forward-slash globs, bash-shebang subprocess fail on `cmd.exe`, CRLF on stdout breaking `git add`. Static-invariant tests added to `windows-free-tests.yml`. Contributed by @daveowenatl via PR #1672. +- `gstack-diff-scope` detects `bun.lock` (Bun v1.2+ text lockfile) alongside `bun.lockb`. Without this, eval-select skipped lockfile changes on Bun 1.2+. Contributed by @hiSandog via PR #1649. +- iOS QA on macOS 26.x: `coredevice.local` resolution falls through `xcrun devicectl` → `dns.lookup` → `dns.resolve6` so the tunnel comes up even when mDNSResponder is bypassed. Tunnel keepalive added so long-running QA sessions survive. Contributed by @sternryan via PR #1673. + ## [1.44.0.0] - 2026-05-23 ## **Sidebar Claude Code now survives the day.** WebSocket keepalive, transparent re-attach across network blips with scrollback intact, and a restart button that actually kills the old claude before spawning the new one. Outer supervisor opt-in so the browse server itself can crash and recover without you noticing. diff --git a/VERSION b/VERSION index 5b692d29a..fc2278642 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.44.0.0 +1.44.1.0 diff --git a/bin/gstack-brain-sync b/bin/gstack-brain-sync index 939aa0d65..2ad0fe773 100755 --- a/bin/gstack-brain-sync +++ b/bin/gstack-brain-sync @@ -136,7 +136,11 @@ def load_privacy_map(path): allowlist_globs = load_lines(allowlist_path) privacy_map = load_privacy_map(privacy_path) -skip_lines = set(load_lines(skip_path)) +# Normalize skip entries to the POSIX form queued paths use, so a backslash +# entry in .brain-skip.txt still matches on Windows. The drain is the safety +# boundary that actually stages files, so it must normalize identically to +# discover_new — otherwise an explicitly-skipped file gets committed. +skip_lines = {s.replace(os.sep, "/") for s in load_lines(skip_path)} # Read queue; collect unique file paths. queue_paths = set() @@ -253,6 +257,8 @@ subcmd_once() { # Stage with git add -f (forces past .gitignore=*) explicit paths only. while IFS= read -r p; do + p="${p%$'\r'}" # Windows: compute_paths_to_stage's python print() emits CRLF; + # a trailing CR makes the pathspec match nothing (silent no-stage). [ -z "$p" ] && continue git -C "$GSTACK_HOME" add -f -- "$p" 2>/dev/null || true done < "$paths_file" @@ -376,10 +382,13 @@ subcmd_discover_new() { exit 0 fi # Walk allowlist globs; enqueue any file where mtime+size differs from cursor. - python3 - "$GSTACK_HOME" "$ALLOWLIST" "$DISCOVER_CURSOR" "$SCRIPT_DIR/gstack-brain-enqueue" <<'PYEOF' 2>/dev/null || true -import sys, os, json, glob, fnmatch, subprocess, hashlib + python3 - "$GSTACK_HOME" "$ALLOWLIST" "$DISCOVER_CURSOR" <<'PYEOF' 2>/dev/null || true +import sys, os, json, fnmatch +from datetime import datetime, timezone -gstack_home, allowlist_path, cursor_path, enqueue_bin = sys.argv[1:5] +gstack_home, allowlist_path, cursor_path = sys.argv[1:4] +queue_path = os.path.join(gstack_home, ".brain-queue.jsonl") +skip_path = os.path.join(gstack_home, ".brain-skip.txt") def load_lines(path): try: @@ -403,8 +412,12 @@ def save_cursor(path, data): pass allowlist = load_lines(allowlist_path) +# Normalize skip entries to the same POSIX form as `rel` below, so a +# backslash entry in .brain-skip.txt still matches a normalized path on Windows. +skip = {s.replace(os.sep, "/") for s in load_lines(skip_path)} cursor = load_cursor(cursor_path) new_cursor = dict(cursor) +to_enqueue = [] # Walk all files under gstack_home, match against allowlist. for root, dirs, files in os.walk(gstack_home): @@ -413,22 +426,54 @@ for root, dirs, files in os.walk(gstack_home): continue for name in files: full = os.path.join(root, name) - rel = os.path.relpath(full, gstack_home) + # Repo paths are POSIX-relative. os.path.relpath yields backslash + # separators on Windows, which never match the forward-slash allowlist + # globs (e.g. "projects/*/learnings.jsonl"), so discovery silently + # enqueued nothing under projects/ on Windows. Normalize to "/". + rel = os.path.relpath(full, gstack_home).replace(os.sep, "/") if rel.startswith(".brain-"): continue - matched = any(fnmatch.fnmatchcase(rel, pat) for pat in allowlist) - if not matched: + if not any(fnmatch.fnmatchcase(rel, pat) for pat in allowlist): + continue + if rel in skip: continue try: st = os.stat(full) key = f"{int(st.st_mtime)}:{st.st_size}" except OSError: continue - prev = cursor.get(rel) - if prev != key: - # Enqueue via the shim (respects sync mode + skip list). - subprocess.run([enqueue_bin, rel], check=False) - new_cursor[rel] = key + if cursor.get(rel) != key: + to_enqueue.append((rel, key)) + +# Append to the queue directly. The previous implementation shelled out to +# gstack-brain-enqueue once per file, but Windows Python cannot exec a +# bash-shebang script (the spawn fails with a fork error), so discovery +# enqueued nothing on Windows even after the path-match fix above. +# Writing the queue line here is platform-agnostic; the drain step +# (compute_paths_to_stage) still re-applies the skip-list + privacy filters. +if to_enqueue: + ts = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + try: + # One atomic append per record (O_APPEND, each line < PIPE_BUF), matching + # gstack-brain-enqueue's concurrency contract so a writer-shim append + # running in parallel can't interleave mid-record. Buffered text writes + # don't guarantee that. Compact separators match the shim's JSON shape. + fd = os.open(queue_path, os.O_WRONLY | os.O_CREAT | os.O_APPEND, 0o644) + try: + for rel, key in to_enqueue: + rec = json.dumps({"file": rel, "ts": ts}, separators=(",", ":")) + os.write(fd, (rec + "\n").encode("utf-8")) + finally: + os.close(fd) + except OSError: + # Queue write failed (disk full, AV file lock). Leave the cursor + # unadvanced so these files are retried on the next discover instead of + # being silently recorded as synced (which loses the change until the + # file next changes). + to_enqueue = [] + # Advance the cursor only for records actually written. + for rel, key in to_enqueue: + new_cursor[rel] = key save_cursor(cursor_path, new_cursor) PYEOF diff --git a/bin/gstack-developer-profile b/bin/gstack-developer-profile index 3e8ed0bd4..3bd397040 100755 --- a/bin/gstack-developer-profile +++ b/bin/gstack-developer-profile @@ -17,6 +17,9 @@ # --check-mismatch detect meaningful gaps between declared and observed. # --migrate migrate builder-profile.jsonl → developer-profile.json. # Idempotent; archives the source file on success. +# --log-session append a session entry (from /office-hours) to +# sessions[] and update aggregates. Required fields: +# date, mode. Silent skip on invalid input. # # Profile file: ~/.gstack/developer-profile.json (unified schema — see # docs/designs/PLAN_TUNING_V0.md). Event file: ~/.gstack/projects/{SLUG}/ @@ -154,6 +157,65 @@ ensure_profile() { EOF } +# ----------------------------------------------------------------------- +# Record session: append a session entry from /office-hours to sessions[] +# and update aggregates (signals_accumulated, resources_shown, topics). +# Fix for #1671: the writer side of the v1.0.0.0 migration. Reader and +# writer now share the same file. +# Silent skip on invalid input (matches gstack-timeline-log:22-26 pattern). +# ----------------------------------------------------------------------- +do_log_session() { + local INPUT="${1:-}" + if [ -z "$INPUT" ]; then + return 0 + fi + + # Validate: input must be parseable JSON with required fields (date, mode). + if ! printf '%s' "$INPUT" | bun -e " + const j = JSON.parse(await Bun.stdin.text()); + if (!j.date || !j.mode) process.exit(1); + " 2>/dev/null; then + return 0 + fi + + ensure_profile + + local TMPOUT + TMPOUT=$(mktemp "$GSTACK_HOME/developer-profile.json.XXXXXX.tmp") + trap 'rm -f "$TMPOUT"' EXIT + + PROFILE_FILE_PATH="$PROFILE_FILE" RECORD_INPUT="$INPUT" TMPOUT_PATH="$TMPOUT" bun -e " + const fs = require('fs'); + const entry = JSON.parse(process.env.RECORD_INPUT); + if (!entry.ts) entry.ts = new Date().toISOString(); + + const profile = JSON.parse(fs.readFileSync(process.env.PROFILE_FILE_PATH, 'utf-8')); + profile.sessions = profile.sessions || []; + profile.sessions.push(entry); + + profile.signals_accumulated = profile.signals_accumulated || {}; + for (const s of (entry.signals || [])) { + profile.signals_accumulated[s] = (profile.signals_accumulated[s] || 0) + 1; + } + + profile.resources_shown = profile.resources_shown || []; + const resSet = new Set(profile.resources_shown); + for (const r of (entry.resources_shown || [])) resSet.add(r); + profile.resources_shown = Array.from(resSet); + + profile.topics = profile.topics || []; + const topicSet = new Set(profile.topics); + for (const t of (entry.topics || [])) topicSet.add(t); + profile.topics = Array.from(topicSet); + + fs.writeFileSync(process.env.TMPOUT_PATH, JSON.stringify(profile, null, 2)); + " + + mv "$TMPOUT" "$PROFILE_FILE" + trap - EXIT + "$SCRIPT_DIR/gstack-brain-enqueue" "developer-profile.json" 2>/dev/null & +} + # ----------------------------------------------------------------------- # Read: emit legacy KEY: VALUE output for /office-hours compat. # ----------------------------------------------------------------------- @@ -168,14 +230,19 @@ do_read() { else if (count >= 4) tier = 'regular'; else if (count >= 1) tier = 'welcome_back'; - const last = sessions[count - 1] || {}; - const prev = sessions[count - 2] || {}; + // LAST_* / CROSS_PROJECT must reflect real sessions, not resource-tracking + // events (the Phase 6 auto-append). Without this filter, a session's + // resources entry written immediately after the real session would clobber + // LAST_PROJECT/LAST_ASSIGNMENT/LAST_DESIGN_TITLE. + const realSessions = sessions.filter(e => e.mode !== 'resources'); + const last = realSessions[realSessions.length - 1] || {}; + const prev = realSessions[realSessions.length - 2] || {}; const crossProject = prev.project_slug && last.project_slug ? prev.project_slug !== last.project_slug : false; - const designs = sessions.map(e => e.design_doc || '').filter(Boolean); - const designTitles = sessions + const designs = realSessions.map(e => e.design_doc || '').filter(Boolean); + const designTitles = realSessions .map(e => (e.design_doc ? (e.project_slug || 'unknown') : '')) .filter(Boolean); @@ -441,6 +508,7 @@ case "$CMD" in --vibe) do_vibe ;; --check-mismatch) do_check_mismatch ;; --migrate) do_migrate ;; + --log-session) do_log_session "$@" ;; --help|-h) sed -n '1,/^set -euo/p' "$0" | sed 's|^# \?||' ;; *) echo "gstack-developer-profile: unknown subcommand '$CMD'" >&2 diff --git a/bin/gstack-diff-scope b/bin/gstack-diff-scope index 2cff90c70..36918381c 100755 --- a/bin/gstack-diff-scope +++ b/bin/gstack-diff-scope @@ -57,7 +57,7 @@ while IFS= read -r f; do *.md) DOCS=true ;; # Config - package.json|package-lock.json|yarn.lock|bun.lockb) CONFIG=true ;; + package.json|package-lock.json|yarn.lock|bun.lock|bun.lockb) CONFIG=true ;; Gemfile|Gemfile.lock) CONFIG=true ;; *.yml|*.yaml) CONFIG=true ;; .github/*) CONFIG=true ;; diff --git a/bin/gstack-next-version b/bin/gstack-next-version index e10485d96..455dd72f2 100755 --- a/bin/gstack-next-version +++ b/bin/gstack-next-version @@ -10,7 +10,14 @@ // // Usage: // gstack-next-version --base --bump \ -// --current-version [--workspace-root |null] [--json] +// --current-version [--workspace-root |null] \ +// [--version-path ] [--json] +// +// VERSION path resolution (monorepo support): +// 1. --version-path CLI flag (highest priority) +// 2. .gstack/version-path file at the repo root (single-line relative path, +// committed so all collaborators benefit) +// 3. "VERSION" at the repo root (default, backward-compatible) // // Exit codes: // 0 — emitted JSON successfully (may include "offline":true or "host":"unknown") @@ -45,6 +52,7 @@ type Output = { version: string; current_version: string; base_version: string; + version_path: string; bump: Bump; host: "github" | "gitlab" | "unknown"; offline: boolean; @@ -114,6 +122,28 @@ function runCommand(cmd: string, args: string[], timeoutMs = 15000): { ok: boole }; } +// VERSION-path resolution for monorepos. Priority: CLI flag > .gstack/version-path +// at repo root > "VERSION". Pure function; takes the repo root as an argument so +// tests can drive it with a fixture dir without mocking git. +function resolveVersionPath(override: string | undefined, repoRoot: string): string { + if (override) return override.trim(); + const configFile = join(repoRoot, ".gstack", "version-path"); + if (existsSync(configFile)) { + try { + const firstLine = readFileSync(configFile, "utf8").split("\n")[0]?.trim() ?? ""; + if (firstLine) return firstLine; + } catch { + // fall through to default + } + } + return "VERSION"; +} + +function repoToplevel(): string { + const r = runCommand("git", ["rev-parse", "--show-toplevel"]); + return r.ok ? r.stdout.trim() : process.cwd(); +} + function detectHost(): "github" | "gitlab" | "unknown" { const remote = runCommand("git", ["remote", "get-url", "origin"]); if (remote.ok) { @@ -128,19 +158,19 @@ function detectHost(): "github" | "gitlab" | "unknown" { return "unknown"; } -function readBaseVersion(base: string, warnings: string[]): string { +function readBaseVersion(base: string, versionPath: string, warnings: string[]): string { // git fetch is best-effort; we tolerate failure and fall back to whatever // origin/ currently points at. runCommand("git", ["fetch", "origin", base, "--quiet"], 10000); - const r = runCommand("git", ["show", `origin/${base}:VERSION`]); + const r = runCommand("git", ["show", `origin/${base}:${versionPath}`]); if (!r.ok) { - warnings.push(`could not read VERSION at origin/${base}; assuming 0.0.0.0`); + warnings.push(`could not read ${versionPath} at origin/${base}; assuming 0.0.0.0`); return "0.0.0.0"; } return r.stdout.trim(); } -async function fetchGithubClaimed(base: string, excludePR: number | null, warnings: string[]): Promise<{ claimed: ClaimedPR[]; offline: boolean }> { +async function fetchGithubClaimed(base: string, versionPath: string, excludePR: number | null, warnings: string[]): Promise<{ claimed: ClaimedPR[]; offline: boolean }> { const list = runCommand("gh", [ "pr", "list", @@ -187,14 +217,18 @@ async function fetchGithubClaimed(base: string, excludePR: number | null, warnin const pr = queue.shift(); if (!pr) return; // gh passes branch name via argv, not shell — safe. + // encodeURI handles spaces in subproject paths (e.g. "Tinas Second Brain/...") + // while leaving "/" untouched so the GitHub Contents API gets the path intact. const content = runCommand("gh", [ "api", - `repos/{owner}/{repo}/contents/VERSION?ref=${encodeURIComponent(pr.headRefName)}`, + `repos/{owner}/{repo}/contents/${encodeURI(versionPath)}?ref=${encodeURIComponent(pr.headRefName)}`, "-q", ".content", ]); if (!content.ok) { - warnings.push(`PR #${pr.number}: could not fetch VERSION (fork or private)`); + warnings.push( + `PR #${pr.number}: could not fetch ${versionPath} (fork, private, or wrong path — try --version-path or .gstack/version-path)`, + ); continue; } let versionStr: string; @@ -215,7 +249,7 @@ async function fetchGithubClaimed(base: string, excludePR: number | null, warnin return { claimed: results, offline: false }; } -async function fetchGitlabClaimed(base: string, excludePR: number | null, warnings: string[]): Promise<{ claimed: ClaimedPR[]; offline: boolean }> { +async function fetchGitlabClaimed(base: string, versionPath: string, excludePR: number | null, warnings: string[]): Promise<{ claimed: ClaimedPR[]; offline: boolean }> { const list = runCommand("glab", [ "mr", "list", @@ -243,12 +277,15 @@ async function fetchGitlabClaimed(base: string, excludePR: number | null, warnin } const results: ClaimedPR[] = []; for (const mr of mrs) { + // GitLab files API takes the full path URL-encoded (slashes become %2F). const content = runCommand("glab", [ "api", - `projects/:id/repository/files/VERSION?ref=${encodeURIComponent(mr.source_branch)}`, + `projects/:id/repository/files/${encodeURIComponent(versionPath)}?ref=${encodeURIComponent(mr.source_branch)}`, ]); if (!content.ok) { - warnings.push(`MR !${mr.iid}: could not fetch VERSION`); + warnings.push( + `MR !${mr.iid}: could not fetch ${versionPath} (wrong path? — try --version-path or .gstack/version-path)`, + ); continue; } try { @@ -285,7 +322,7 @@ function currentRepoSlug(): string { return m ? m[1] : ""; } -function scanSiblings(root: string | null, claimed: ClaimedPR[], warnings: string[]): Sibling[] { +function scanSiblings(root: string | null, versionPath: string, claimed: ClaimedPR[], warnings: string[]): Sibling[] { if (!root || !existsSync(root)) return []; const mySlug = currentRepoSlug(); if (!mySlug) { @@ -308,7 +345,7 @@ function scanSiblings(root: string | null, claimed: ClaimedPR[], warnings: strin continue; } if (!existsSync(join(p, ".git")) && !existsSync(join(p, ".git/HEAD"))) continue; - const versionFile = join(p, "VERSION"); + const versionFile = join(p, versionPath); if (!existsSync(versionFile)) continue; let version: string; try { @@ -346,12 +383,13 @@ function markActiveSiblings(siblings: Sibling[], baseVersion: Version): Sibling[ }); } -function parseArgs(argv: string[]): { base: string; bump: Bump; current: string; workspaceRoot?: string; excludePR: number | null; help: boolean } { +function parseArgs(argv: string[]): { base: string; bump: Bump; current: string; workspaceRoot?: string; excludePR: number | null; versionPath?: string; help: boolean } { let base = ""; let bump: Bump | "" = ""; let current = ""; let workspaceRoot: string | undefined; let excludePR: number | null = null; + let versionPath: string | undefined; let help = false; for (let i = 0; i < argv.length; i++) { const a = argv[i]; @@ -359,6 +397,7 @@ function parseArgs(argv: string[]): { base: string; bump: Bump; current: string; else if (a === "--bump") bump = (argv[++i] ?? "") as Bump; else if (a === "--current-version") current = argv[++i] ?? ""; else if (a === "--workspace-root") workspaceRoot = argv[++i]; + else if (a === "--version-path") versionPath = argv[++i]; else if (a === "--exclude-pr") { const n = Number(argv[++i]); excludePR = Number.isFinite(n) && n > 0 ? n : null; @@ -375,7 +414,7 @@ function parseArgs(argv: string[]): { base: string; bump: Bump; current: string; console.error(`Error: --bump must be major|minor|patch|micro (got ${bump})`); process.exit(2); } - return { base, bump: bump as Bump, current, workspaceRoot, excludePR, help: false }; + return { base, bump: bump as Bump, current, workspaceRoot, excludePR, versionPath, help: false }; } // Auto-detect: if --exclude-pr wasn't passed, check whether the current branch @@ -392,13 +431,14 @@ async function main() { const args = parseArgs(process.argv.slice(2)); if (args.help) { console.log( - "Usage: gstack-next-version --base --bump --current-version [--workspace-root ]", + "Usage: gstack-next-version --base --bump --current-version [--workspace-root ] [--version-path ]", ); process.exit(0); } const warnings: string[] = []; const host = detectHost(); - const baseVersion = args.current || readBaseVersion(args.base, warnings); + const versionPath = resolveVersionPath(args.versionPath, repoToplevel()); + const baseVersion = args.current || readBaseVersion(args.base, versionPath, warnings); const baseParsed = parseVersion(baseVersion); if (!baseParsed) { console.error(`Error: could not parse base version '${baseVersion}'`); @@ -413,9 +453,9 @@ async function main() { let claimed: ClaimedPR[] = []; let offline = false; if (host === "github") { - ({ claimed, offline } = await fetchGithubClaimed(args.base, excludePR, warnings)); + ({ claimed, offline } = await fetchGithubClaimed(args.base, versionPath, excludePR, warnings)); } else if (host === "gitlab") { - ({ claimed, offline } = await fetchGitlabClaimed(args.base, excludePR, warnings)); + ({ claimed, offline } = await fetchGitlabClaimed(args.base, versionPath, excludePR, warnings)); } else { warnings.push("host unknown; queue-awareness unavailable"); } @@ -433,7 +473,7 @@ async function main() { const { version: picked, reason } = pickNextSlot(baseParsed, claimedVersions, args.bump); const workspaceRoot = resolveWorkspaceRoot(args.workspaceRoot); - const siblings = markActiveSiblings(scanSiblings(workspaceRoot, claimed, warnings), baseParsed); + const siblings = markActiveSiblings(scanSiblings(workspaceRoot, versionPath, claimed, warnings), baseParsed); const activeSiblings = siblings.filter((s) => s.is_active); // If an active sibling outranks our pick, bump past it (same bump level). @@ -453,6 +493,7 @@ async function main() { version: fmtVersion(finalVersion), current_version: args.current || baseVersion, base_version: baseVersion, + version_path: versionPath, bump: args.bump, host, offline, @@ -466,7 +507,7 @@ async function main() { } // Pure-function exports for testing -export { parseVersion, fmtVersion, bumpVersion, cmpVersion, pickNextSlot, markActiveSiblings }; +export { parseVersion, fmtVersion, bumpVersion, cmpVersion, pickNextSlot, markActiveSiblings, resolveVersionPath }; // Only run main() when invoked as a script, not when imported by tests. if (import.meta.main) { diff --git a/bin/gstack-timeline-read b/bin/gstack-timeline-read index f11d5b40e..5c1b6bb6f 100755 --- a/bin/gstack-timeline-read +++ b/bin/gstack-timeline-read @@ -29,11 +29,13 @@ if [ ! -f "$TIMELINE_FILE" ]; then exit 0 fi -cat "$TIMELINE_FILE" 2>/dev/null | bun -e " +cat "$TIMELINE_FILE" 2>/dev/null | GSTACK_TIMELINE_SINCE="$SINCE" GSTACK_TIMELINE_BRANCH="$BRANCH" GSTACK_TIMELINE_LIMIT="$LIMIT" bun -e " const lines = (await Bun.stdin.text()).trim().split('\n').filter(Boolean); -const since = '${SINCE}'; -const branch = '${BRANCH}'; -const limit = ${LIMIT}; +const since = process.env.GSTACK_TIMELINE_SINCE || ''; +const branch = process.env.GSTACK_TIMELINE_BRANCH || ''; +const limitRaw = process.env.GSTACK_TIMELINE_LIMIT || '20'; +const parsedLimit = Number.parseInt(limitRaw, 10); +const limit = Number.isSafeInteger(parsedLimit) && parsedLimit > 0 ? parsedLimit : 20; let sinceMs = 0; if (since) { diff --git a/browse/src/server.ts b/browse/src/server.ts index 9483602b4..bc0b378cb 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -650,6 +650,8 @@ export const __testInternals__ = { idleCheckTick, setTunnelActive: (v: boolean) => { tunnelActive = v; }, setLastActivity: (t: number) => { lastActivity = t; }, + formatExplicitPortUnavailableError, + formatRandomPortUnavailableError, // Reset the module-level shutdown latch so tests that drive shutdown to // completion (process.exit-stubbed) can be followed by tests that also // need shutdown to fire. Without this, the second test's shutdown @@ -752,41 +754,124 @@ let activeBrowserManager: BrowserManager = browserManager; browserManager.onDisconnect = (code) => activeShutdown?.(code ?? 2); let isShuttingDown = false; +type PortCheckResult = + | { available: true } + | { available: false; code?: string; message: string }; + +type FailedPortAttempt = { + port: number; + result: Extract; +}; + +const RANDOM_PORT_MIN = 10000; +const RANDOM_PORT_MAX = 60000; +const RANDOM_PORT_RETRIES = 5; + +function normalizePortError(err: unknown): Extract { + const maybeNodeError = err as NodeJS.ErrnoException | undefined; + return { + available: false, + code: maybeNodeError?.code, + message: maybeNodeError?.message || String(err), + }; +} + +function isOccupiedPort(result: Extract): boolean { + return result.code === 'EADDRINUSE'; +} + +function formatPortFailureDetail(attempt: FailedPortAttempt): string { + const { code, message } = attempt.result; + return code ? `${attempt.port} (${code}: ${message})` : `${attempt.port} (${message})`; +} + +function formatExplicitPortUnavailableError( + port: number, + result: Extract +): Error { + if (isOccupiedPort(result)) { + return new Error(`[browse] Port ${port} (from BROWSE_PORT env) is in use`); + } + + const detail = result.code ? `${result.code}: ${result.message}` : result.message; + return new Error( + `[browse] Cannot bind BROWSE_PORT=${port} on 127.0.0.1 (${detail}). ` + + `This usually means localhost port binding is blocked by the current sandbox or OS permissions, ` + + `not that the port is occupied. Allow localhost binding, or run browse from an unrestricted terminal.` + ); +} + +function formatRandomPortUnavailableError(attempts: FailedPortAttempt[]): Error { + const blockingAttempts = attempts.filter((attempt) => !isOccupiedPort(attempt.result)); + + if (blockingAttempts.length > 0) { + const last = blockingAttempts[blockingAttempts.length - 1]; + return new Error( + `[browse] Cannot bind localhost ports after ${attempts.length} attempts in range ` + + `${RANDOM_PORT_MIN}-${RANDOM_PORT_MAX}. Last error: ${formatPortFailureDetail(last)}. ` + + `This usually means the current sandbox or OS permissions are blocking localhost port binding, ` + + `not that every sampled port is occupied. Allow localhost binding, set BROWSE_PORT to an approved ` + + `port, or run browse from an unrestricted terminal.` + ); + } + + return new Error( + `[browse] No available port after ${RANDOM_PORT_RETRIES} attempts in range ` + + `${RANDOM_PORT_MIN}-${RANDOM_PORT_MAX}; every sampled port was already in use` + ); +} + // Test if a port is available by binding and immediately releasing. // Uses net.createServer instead of Bun.serve to avoid a race condition // in the Node.js polyfill where listen/close are async but the caller // expects synchronous bind semantics. See: #486 -function isPortAvailable(port: number, hostname: string = '127.0.0.1'): Promise { +function checkPortAvailable(port: number, hostname: string = '127.0.0.1'): Promise { return new Promise((resolve) => { const srv = net.createServer(); - srv.once('error', () => resolve(false)); - srv.listen(port, hostname, () => { - srv.close(() => resolve(true)); - }); + let settled = false; + const finish = (result: PortCheckResult) => { + if (settled) return; + settled = true; + resolve(result); + }; + + srv.once('error', (err) => finish(normalizePortError(err))); + try { + srv.listen(port, hostname, () => { + srv.close(() => finish({ available: true })); + }); + } catch (err) { + finish(normalizePortError(err)); + } }); } +function isPortAvailable(port: number, hostname: string = '127.0.0.1'): Promise { + return checkPortAvailable(port, hostname).then((result) => result.available); +} + // Find port: explicit BROWSE_PORT, or random in 10000-60000 async function findPort(): Promise { // Explicit port override (for debugging) if (BROWSE_PORT) { - if (await isPortAvailable(BROWSE_PORT)) { + const result = await checkPortAvailable(BROWSE_PORT); + if (result.available) { return BROWSE_PORT; } - throw new Error(`[browse] Port ${BROWSE_PORT} (from BROWSE_PORT env) is in use`); + throw formatExplicitPortUnavailableError(BROWSE_PORT, result); } // Random port with retry - const MIN_PORT = 10000; - const MAX_PORT = 60000; - const MAX_RETRIES = 5; - for (let attempt = 0; attempt < MAX_RETRIES; attempt++) { - const port = MIN_PORT + Math.floor(Math.random() * (MAX_PORT - MIN_PORT)); - if (await isPortAvailable(port)) { + const attempts: FailedPortAttempt[] = []; + for (let attempt = 0; attempt < RANDOM_PORT_RETRIES; attempt++) { + const port = RANDOM_PORT_MIN + Math.floor(Math.random() * (RANDOM_PORT_MAX - RANDOM_PORT_MIN)); + const result = await checkPortAvailable(port); + if (result.available) { return port; } + attempts.push({ port, result }); } - throw new Error(`[browse] No available port after ${MAX_RETRIES} attempts in range ${MIN_PORT}-${MAX_PORT}`); + throw formatRandomPortUnavailableError(attempts); } /** diff --git a/browse/test/findport.test.ts b/browse/test/findport.test.ts index fb3a9cb09..3255d240c 100644 --- a/browse/test/findport.test.ts +++ b/browse/test/findport.test.ts @@ -1,6 +1,7 @@ import { describe, test, expect } from 'bun:test'; import * as net from 'net'; import * as path from 'path'; +import { __testInternals__ } from '../src/server'; const polyfillPath = path.resolve(import.meta.dir, '../src/bun-polyfill.cjs'); @@ -28,6 +29,47 @@ function getFreePort(): Promise { } describe('findPort / isPortAvailable', () => { + test('explicit BROWSE_PORT diagnostic distinguishes bind denial from occupied port', () => { + const blocked = __testInternals__.formatExplicitPortUnavailableError(34567, { + available: false, + code: 'EPERM', + message: 'operation not permitted', + }).message; + + expect(blocked).toContain('Cannot bind BROWSE_PORT=34567'); + expect(blocked).toContain('localhost port binding is blocked'); + expect(blocked).toContain('not that the port is occupied'); + + const occupied = __testInternals__.formatExplicitPortUnavailableError(34567, { + available: false, + code: 'EADDRINUSE', + message: 'address already in use', + }).message; + + expect(occupied).toBe('[browse] Port 34567 (from BROWSE_PORT env) is in use'); + }); + + test('random port diagnostic calls out sandbox-style bind denial', () => { + const message = __testInternals__.formatRandomPortUnavailableError([ + { port: 11001, result: { available: false, code: 'EADDRINUSE', message: 'address already in use' } }, + { port: 12002, result: { available: false, code: 'EPERM', message: 'operation not permitted' } }, + ]).message; + + expect(message).toContain('Cannot bind localhost ports after 2 attempts'); + expect(message).toContain('Last error: 12002 (EPERM: operation not permitted)'); + expect(message).toContain('not that every sampled port is occupied'); + expect(message).toContain('set BROWSE_PORT to an approved port'); + }); + + test('random port diagnostic preserves old busy-port meaning when all attempts are occupied', () => { + const message = __testInternals__.formatRandomPortUnavailableError([ + { port: 11001, result: { available: false, code: 'EADDRINUSE', message: 'address already in use' } }, + { port: 12002, result: { available: false, code: 'EADDRINUSE', message: 'address already in use' } }, + ]).message; + + expect(message).toContain('No available port after 5 attempts'); + expect(message).toContain('every sampled port was already in use'); + }); test('isPortAvailable returns true for a free port', async () => { // Use the same isPortAvailable logic from server.ts diff --git a/docs/designs/FIX_1671_PROFILE_MIGRATION.md b/docs/designs/FIX_1671_PROFILE_MIGRATION.md new file mode 100644 index 000000000..96842535d --- /dev/null +++ b/docs/designs/FIX_1671_PROFILE_MIGRATION.md @@ -0,0 +1,81 @@ +# Fix #1671: `/office-hours` always reports SESSION_COUNT: 0 + +**Status:** SHIPPED +**Branch:** fix-1671-profile-migration +**Date:** 2026-05-23 +**Issue:** https://github.com/garrytan/gstack/issues/1671 +**Original PR that introduced the bug:** garrytan/gstack#1039 / commit `0a803f9` / v1.0.0.0 / 2026-04-18 + +## The problem + +`/office-hours` reports `SESSION_COUNT: 0` and `TIER: introduction` on every invocation, even for users who have run the skill many times. The `welcome_back` tier (`bin/gstack-developer-profile:165-169`) that exists to skip the closing pitch for returning users is unreachable. Live ~5 weeks on every fresh-`$HOME` user since v1.0.0.0. + +## Root cause + +The v1.0.0.0 migration moved the read path to `~/.gstack/developer-profile.json` but left the writer in `office-hours/SKILL.md.tmpl` writing to the legacy `~/.gstack/builder-profile.jsonl`. The `ensure_profile` stub created on first read has `sessions: []`; subsequent writes go to a file the reader never re-reads. Reader and writer disagree on storage. + +Full root-cause analysis (including RC2/RC3 follow-ups): https://github.com/garrytan/gstack/issues/1671 + +## The fix + +Make the writer use the same file the reader does. + +### Changes + +1. **`bin/gstack-developer-profile`** — add `--log-session ''` subcommand: + - Validates required fields (`date`, `mode`), silent-skip on invalid input (matches `bin/gstack-timeline-log:22-26`). + - Reads existing `developer-profile.json` via `bun -e`. + - Appends entry to `sessions[]`. Updates `signals_accumulated` (per-signal-string increment, same as `do_migrate:67-69`), unions `resources_shown` and `topics`. + - Atomic mktemp+mv write (matches existing pattern at line 54). + - Calls `gstack-brain-enqueue "developer-profile.json"` after write, mirroring `bin/gstack-timeline-log:40`. + +2. **`bin/gstack-developer-profile:do_read`** — filter `mode:"resources"` entries when picking LAST_PROJECT / LAST_ASSIGNMENT / LAST_DESIGN_TITLE / CROSS_PROJECT / DESIGN_*. The Phase 6 resources auto-append happens after the real session in the same /office-hours invocation; without the filter, that resources entry clobbers real-session state for the user's next session. Latent bug that was masked by the broken writer; activated by the fix. + +3. **`office-hours/SKILL.md.tmpl`** — swap writers at lines 490 and 893: + - From: `echo '{...}' >> "$GSTACK_STATE_ROOT/builder-profile.jsonl"` + - To: `~/.claude/skills/gstack/bin/gstack-developer-profile --log-session '{...}' 2>/dev/null || true` + - Run `bun run gen:skill-docs` to regenerate `office-hours/SKILL.md`. + +### What's NOT in the fix (intentionally) + +- **No new binary.** The owner binary for `developer-profile.json` is `gstack-developer-profile`; the writer belongs there as a subcommand. `--log-session` joins the binary's existing `--migrate` / `--derive` write-side subcommand boundary, not the `gstack-*-log` event-writer family. Verb name still matches `gstack-*-log`. +- **No mkdir-locks.** Concurrent /office-hours calls have a read-modify-write race on `developer-profile.json`. The codebase accepts the same race in `gstack-config` (r-m-w on YAML, no lock). Not introduced by this fix; out of scope. +- **No schema bump.** Schema stays at `schema_version: 1`. The fix doesn't change the schema, just makes the writer use it. +- **No auto-reconcile for affected users.** Existing users with stranded `builder-profile.jsonl` entries don't get their past history auto-merged into `developer-profile.json`. On their next /office-hours run, the first new session lands in `welcome_back`; past data stays in the legacy file (still readable by other tools during deprecation). Most affected users have only a handful of stranded sessions so the loss is mostly aesthetic. Dropped the one-release-only reconcile pathway as net noise — Garry's "right-sized diff" voice. +- **No autoplan timeline rollup (RC2).** Separate concern, separate PR. +- **No project-scope opt-in (RC3).** Separate concern, separate PR. +- **No gbrain glob change.** The office-hours manifest still globs `~/.gstack/builder-profile.jsonl` for context; once new writes stop landing there, the snapshot goes cold. Update in a follow-up if it becomes a UX issue. + +### Tests (all gate-tier, free, deterministic) + +1. **Regression test** in `test/gstack-developer-profile.test.ts`: + - Fresh `$HOME`. + - Run /office-hours preamble: gstack-developer-profile creates empty stub. + - Call `--log-session` with a startup-mode JSON. + - Run `--read` again. Assert `SESSION_COUNT: 1`, `TIER: welcome_back`. + - Fails on current main (subcommand doesn't exist). Passes with fix. + +2. **`do_read` mode filter test:** after recording a startup session followed by a resources entry, `--read` returns LAST_PROJECT / LAST_ASSIGNMENT / LAST_DESIGN_TITLE from the real session, not from the resources entry. RESOURCES_SHOWN still aggregates correctly. + +3. **Validation + aggregation tests:** `--log-session` silently skips invalid JSON / missing required fields, injects `ts` if missing, preserves user-set `ts`, correctly aggregates signals/resources/topics across multiple sessions. + +4. **Static-grep invariant** in `test/static-no-legacy-writes.test.ts` (new): walks every skill dir, asserts no production code path writes to `builder-profile.jsonl` except allowlisted readers (`gstack-developer-profile`, `gstack-memory-ingest.ts`, `gstack-artifacts-init`, doc files). Prevents future writers from regressing onto the legacy file. + +### Acceptance criteria + +- Second `/office-hours` invocation on a fresh `$HOME` returns `TIER: welcome_back`. +- `bun test` passes on the touched files in isolation. +- `bun run gen:skill-docs` produces clean diff matching the `.tmpl` edits. + +### Rollout + +- One commit. PATCH version bump per CHANGELOG style guide. +- CHANGELOG entry written by `/ship`. User-facing voice: lead with what users experience now that they didn't before (welcome_back tier kicks in on second visit). + +## Follow-up TODOs + +- Deprecate `builder-profile.jsonl` entirely (writer + shim + memory-ingest type) after one release. +- Fix RC2 (autoplan inlines sub-skills, bypassing their timeline-log preambles). +- Add `GSTACK_PROFILE_SCOPE` opt-in for power users with multiple agent identities (RC3). +- /plan-tune doesn't currently call `--derive`, so `inferred`/`gap` can drift (pre-existing, unrelated to #1671). +- `mode:"resources"` entries inflate SESSION_COUNT under the existing tier aggregator (pre-existing, unrelated to #1671 root cause). diff --git a/gstack-upgrade/migrations/v1.40.0.0.sh b/gstack-upgrade/migrations/v1.40.0.0.sh index d21c18ba3..e482d57a6 100755 --- a/gstack-upgrade/migrations/v1.40.0.0.sh +++ b/gstack-upgrade/migrations/v1.40.0.0.sh @@ -13,6 +13,12 @@ # # Idempotent: each insertion is gated on `not already present` so re-running # the migration is a no-op. +# +# Done-marker discipline (#1581): the marker is only written when every +# required repair either succeeded or was provably unnecessary. Tracking +# happens via the `incomplete` flag; on any failure path (missing jq, broken +# JSON, append failure, mv failure) we set `incomplete=1` and skip the touch +# so the migration runner retries on the next /gstack-upgrade. set -u @@ -34,19 +40,30 @@ NEW_PATTERNS=( ) added_any=0 +incomplete=0 # ----- .brain-allowlist --------------------------------------------------- if [ -f "${ALLOWLIST}" ]; then for PATTERN in "${NEW_PATTERNS[@]}"; do if ! grep -Fq -- "${PATTERN}" "${ALLOWLIST}" 2>/dev/null; then if grep -q '^# ---- USER ADDITIONS BELOW' "${ALLOWLIST}" 2>/dev/null; then - sed -i.bak "/^# ---- USER ADDITIONS BELOW/i\\ + if sed -i.bak "/^# ---- USER ADDITIONS BELOW/i\\ ${PATTERN} -" "${ALLOWLIST}" && rm -f "${ALLOWLIST}.bak" - added_any=1 +" "${ALLOWLIST}" 2>/dev/null; then + rm -f "${ALLOWLIST}.bak" + added_any=1 + else + echo " [v1.40.0.0] WARN: failed to insert ${PATTERN} into ${ALLOWLIST}; will retry on next upgrade." >&2 + rm -f "${ALLOWLIST}.bak" 2>/dev/null || true + incomplete=1 + fi else - printf '%s\n' "${PATTERN}" >> "${ALLOWLIST}" - added_any=1 + if printf '%s\n' "${PATTERN}" >> "${ALLOWLIST}" 2>/dev/null; then + added_any=1 + else + echo " [v1.40.0.0] WARN: failed to append ${PATTERN} to ${ALLOWLIST}; will retry on next upgrade." >&2 + incomplete=1 + fi fi fi done @@ -55,19 +72,39 @@ fi # ----- .brain-privacy-map.json ------------------------------------------- if [ -f "${PRIVACY}" ]; then if command -v jq >/dev/null 2>&1; then - for PATTERN in "${NEW_PATTERNS[@]}"; do - if ! jq -e --arg p "${PATTERN}" 'map(select(.pattern == $p)) | length > 0' "${PRIVACY}" >/dev/null 2>&1; then - if jq --arg p "${PATTERN}" '. += [{"pattern": $p, "class": "artifact"}]' "${PRIVACY}" > "${PRIVACY}.tmp" 2>/dev/null; then - mv "${PRIVACY}.tmp" "${PRIVACY}" - added_any=1 - else - rm -f "${PRIVACY}.tmp" - echo " [v1.40.0.0] WARN: jq failed to patch ${PRIVACY}; skipping pattern ${PATTERN}." >&2 + # Validate JSON shape up front. We won't try to repair a corrupt file — + # bail out and leave for manual fix. + if ! jq -e . "${PRIVACY}" >/dev/null 2>&1; then + echo " [v1.40.0.0] WARN: ${PRIVACY} is not valid JSON; skipping privacy-map repair. Fix manually or run gstack-artifacts-init." >&2 + incomplete=1 + else + for PATTERN in "${NEW_PATTERNS[@]}"; do + if ! jq -e --arg p "${PATTERN}" 'map(select(.pattern == $p)) | length > 0' "${PRIVACY}" >/dev/null 2>&1; then + tmp=$(mktemp "${PRIVACY}.tmp.XXXXXX" 2>/dev/null) + if [ -z "${tmp}" ] || [ ! -f "${tmp}" ]; then + echo " [v1.40.0.0] WARN: failed to create tempfile for ${PRIVACY}; skipping pattern ${PATTERN}." >&2 + incomplete=1 + continue + fi + if jq --arg p "${PATTERN}" '. += [{"pattern": $p, "class": "artifact"}]' "${PRIVACY}" > "${tmp}" 2>/dev/null; then + if mv "${tmp}" "${PRIVACY}" 2>/dev/null; then + added_any=1 + else + echo " [v1.40.0.0] WARN: failed to rewrite ${PRIVACY}; skipping pattern ${PATTERN}." >&2 + rm -f "${tmp}" + incomplete=1 + fi + else + echo " [v1.40.0.0] WARN: jq mutation failed for ${PRIVACY}; skipping pattern ${PATTERN}." >&2 + rm -f "${tmp}" + incomplete=1 + fi fi - fi - done + done + fi else echo " [v1.40.0.0] WARN: jq not found; skipping privacy-map repair. Install jq and re-run gstack-upgrade, or run gstack-artifacts-init manually." >&2 + incomplete=1 fi fi @@ -76,19 +113,27 @@ if [ -f "${GITATTRS}" ]; then for PATTERN in "${NEW_PATTERNS[@]}"; do RULE="${PATTERN} merge=union" if ! grep -Fq -- "${RULE}" "${GITATTRS}" 2>/dev/null; then - printf '%s\n' "${RULE}" >> "${GITATTRS}" - added_any=1 + if printf '%s\n' "${RULE}" >> "${GITATTRS}" 2>/dev/null; then + added_any=1 + else + echo " [v1.40.0.0] WARN: failed to append rule to ${GITATTRS}; will retry on next upgrade." >&2 + incomplete=1 + fi fi done fi -# Mark done even if no patches needed — a fresh-init user's -# bin/gstack-artifacts-init now writes the pattern directly, so re-runs -# should no-op. The touchfile keeps the migration runner from looping. -touch "${DONE}" - -if [ "${added_any}" = "1" ]; then - echo " [v1.40.0.0] allowlist/privacy-map/gitattributes patched for /plan-eng-review test plans (idempotent)" >&2 +if [ "${incomplete}" = "0" ]; then + # Mark done — every required repair either succeeded or was provably + # unnecessary. A fresh-init user's bin/gstack-artifacts-init now writes the + # pattern directly, so re-runs no-op. The touchfile keeps the migration + # runner from looping. + touch "${DONE}" + if [ "${added_any}" = "1" ]; then + echo " [v1.40.0.0] allowlist/privacy-map/gitattributes patched for /plan-eng-review test plans (idempotent)" >&2 + fi +else + echo " [v1.40.0.0] INFO: marker not written; gstack-upgrade will retry once prerequisites are met." >&2 fi # NEVER `git commit + push` from this migration. The user controls when the diff --git a/investigate/SKILL.md b/investigate/SKILL.md index b7780c1c4..40525d63a 100644 --- a/investigate/SKILL.md +++ b/investigate/SKILL.md @@ -30,12 +30,12 @@ hooks: - matcher: "Edit" hooks: - type: command - command: "bash ${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh" + command: 'bash -c ''S="${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh"; [ -x "$S" ] || S="${CLAUDE_SKILL_DIR}/../gstack-freeze/bin/check-freeze.sh"; [ -x "$S" ] && bash "$S" || exit 0''' statusMessage: "Checking debug scope boundary..." - matcher: "Write" hooks: - type: command - command: "bash ${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh" + command: 'bash -c ''S="${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh"; [ -x "$S" ] || S="${CLAUDE_SKILL_DIR}/../gstack-freeze/bin/check-freeze.sh"; [ -x "$S" ] && bash "$S" || exit 0''' statusMessage: "Checking debug scope boundary..." gbrain: schema: 1 @@ -874,7 +874,9 @@ If any learnings come back, name which one applies to your investigation in one After forming your root cause hypothesis, lock edits to the affected module to prevent scope creep. ```bash -[ -x "${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh" ] && echo "FREEZE_AVAILABLE" || echo "FREEZE_UNAVAILABLE" +_FREEZE_SCRIPT="${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh" +[ -x "$_FREEZE_SCRIPT" ] || _FREEZE_SCRIPT="${CLAUDE_SKILL_DIR}/../gstack-freeze/bin/check-freeze.sh" +[ -x "$_FREEZE_SCRIPT" ] && echo "FREEZE_AVAILABLE" || echo "FREEZE_UNAVAILABLE" ``` **If FREEZE_AVAILABLE:** Identify the narrowest directory containing the affected files. Write it to the freeze state file: diff --git a/investigate/SKILL.md.tmpl b/investigate/SKILL.md.tmpl index 20cc2e26d..67e254d74 100644 --- a/investigate/SKILL.md.tmpl +++ b/investigate/SKILL.md.tmpl @@ -30,12 +30,12 @@ hooks: - matcher: "Edit" hooks: - type: command - command: "bash ${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh" + command: 'bash -c ''S="${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh"; [ -x "$S" ] || S="${CLAUDE_SKILL_DIR}/../gstack-freeze/bin/check-freeze.sh"; [ -x "$S" ] && bash "$S" || exit 0''' statusMessage: "Checking debug scope boundary..." - matcher: "Write" hooks: - type: command - command: "bash ${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh" + command: 'bash -c ''S="${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh"; [ -x "$S" ] || S="${CLAUDE_SKILL_DIR}/../gstack-freeze/bin/check-freeze.sh"; [ -x "$S" ] && bash "$S" || exit 0''' statusMessage: "Checking debug scope boundary..." gbrain: schema: 1 @@ -118,7 +118,9 @@ If any learnings come back, name which one applies to your investigation in one After forming your root cause hypothesis, lock edits to the affected module to prevent scope creep. ```bash -[ -x "${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh" ] && echo "FREEZE_AVAILABLE" || echo "FREEZE_UNAVAILABLE" +_FREEZE_SCRIPT="${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh" +[ -x "$_FREEZE_SCRIPT" ] || _FREEZE_SCRIPT="${CLAUDE_SKILL_DIR}/../gstack-freeze/bin/check-freeze.sh" +[ -x "$_FREEZE_SCRIPT" ] && echo "FREEZE_AVAILABLE" || echo "FREEZE_UNAVAILABLE" ``` **If FREEZE_AVAILABLE:** Identify the narrowest directory containing the affected files. Write it to the freeze state file: diff --git a/ios-qa/daemon/src/devicectl.ts b/ios-qa/daemon/src/devicectl.ts index a3cab48ad..ee1696eb9 100644 --- a/ios-qa/daemon/src/devicectl.ts +++ b/ios-qa/daemon/src/devicectl.ts @@ -27,7 +27,32 @@ export interface ResolveImpl { const defaultSpawn: SpawnImpl = (cmd, args) => spawnSync(cmd, args, { stdio: 'pipe', timeout: 60_000 }); +/** + * Default resolver. Uses `dns.lookup` (getaddrinfo, goes through mDNSResponder + * on macOS) instead of `dns.resolve6` (libresolv, does NOT consult mDNS on + * recent macOS — returns ESERVFAIL for `*.coredevice.local`). + * + * Prefer the IPv6 record but fall back to whatever getaddrinfo returns. + */ const defaultResolve: ResolveImpl = async (hostname) => { + const dns = await import('dns'); + return new Promise((resolve, reject) => { + dns.lookup(hostname, { family: 6, all: true }, (err, addrs) => { + if (err) { reject(err); return; } + const ipv6 = (addrs ?? []).filter((a) => a.family === 6).map((a) => a.address); + if (ipv6.length === 0) { reject(new Error(`no IPv6 records for ${hostname}`)); return; } + resolve(ipv6); + }); + }); +}; + +/** + * Last-resort resolver using `dns.resolve6`. Kept for backwards compatibility + * and for environments where mDNSResponder is not in the resolver chain. On + * macOS 26.x (Darwin 25.x) this typically fails with ESERVFAIL — see comment + * on `defaultResolve` above. + */ +const legacyResolve6: ResolveImpl = async (hostname) => { const dns = await import('dns'); return new Promise((resolve, reject) => { dns.resolve6(hostname, (err, addrs) => { @@ -69,6 +94,89 @@ export function listDevices(spawn: SpawnImpl = defaultSpawn): DeviceEntry[] { } } +/** + * Resolve the CoreDevice tunnel's IPv6 address from `devicectl device info + * details --json-output`. This is the most reliable path on macOS 26.x: the + * tunnel IPv6 lives in `result.connectionProperties.tunnelIPAddress` and is + * authoritative (it's what CoreDevice itself uses to route). + * + * A side effect of running `devicectl device info details` is that it forces + * CoreDevice to bring up / refresh the tunnel session, which is why we prefer + * this over mDNS even on machines where mDNS works. + * + * Returns null when the device isn't found, isn't tunneled, or devicectl + * fails — callers should fall through to mDNS resolution. + */ +export function getDeviceTunnelIPv6FromDevicectl( + udid: string, + spawn: SpawnImpl = defaultSpawn, +): string | null { + const tmp = join(tmpdir(), `devicectl-details-${process.pid}-${Date.now()}.json`); + try { + const r = spawn('xcrun', ['devicectl', 'device', 'info', 'details', '--device', udid, '--json-output', tmp]); + if (r.status !== 0) return null; + const raw = readFileSync(tmp, 'utf-8'); + const obj = JSON.parse(raw); + // `result.connectionProperties.tunnelIPAddress` is the canonical location. + // Some Xcode/CoreDevice versions also surface it under `result.tunnel.ipAddress` + // — accept either. + const conn = obj?.result?.connectionProperties as Record | undefined; + const tunnel = obj?.result?.tunnel as Record | undefined; + const addr = (conn?.tunnelIPAddress ?? tunnel?.ipAddress) as string | undefined; + if (typeof addr === 'string' && addr.includes(':')) return addr; + return null; + } catch { + return null; + } finally { + try { rmSync(tmp, { force: true }); } catch { /* ignore */ } + } +} + +/** + * Start a periodic devicectl `info details` poll that keeps the CoreDevice + * tunnel session alive. Xcode 26's CoreDevice only holds the tunnel up while + * a devicectl command is in-flight or Xcode itself is debugging. Without + * something poking it, the tunnel IPv6 becomes unroutable within seconds — + * `curl` to the address times out even though the address looks valid. + * + * Implementation note: we chose `device info details` (cheap, ~10ms of CPU + * per tick, no persistent child process) over `device console` (which would + * keep the tunnel up continuously but spams stdout, can wedge on backpressure, + * and is harder to kill cleanly). The 5-second interval is comfortably under + * the empirically-observed tunnel teardown timeout (~10-15s of idle). + * + * Returns a `stop()` function that cancels the timer. Safe to call multiple + * times. + */ +export function startTunnelKeepalive( + udid: string, + opts: { intervalMs?: number; spawn?: SpawnImpl } = {}, +): { stop: () => void } { + const intervalMs = opts.intervalMs ?? 5_000; + const spawn = opts.spawn ?? defaultSpawn; + let stopped = false; + const tick = () => { + if (stopped) return; + // Fire-and-forget: ignore result, the side-effect of the spawn is what + // keeps the tunnel up. We deliberately do not use the JSON output here. + try { + const tmp = join(tmpdir(), `devicectl-keepalive-${process.pid}-${Date.now()}.json`); + spawn('xcrun', ['devicectl', 'device', 'info', 'details', '--device', udid, '--json-output', tmp]); + try { rmSync(tmp, { force: true }); } catch { /* ignore */ } + } catch { /* ignore — next tick will retry */ } + }; + const handle = setInterval(tick, intervalMs); + // Don't keep the event loop alive just for this — daemon owns the lifecycle. + if (typeof handle.unref === 'function') handle.unref(); + return { + stop: () => { + if (stopped) return; + stopped = true; + clearInterval(handle); + }, + }; +} + /** * Resolve the CoreDevice tunnel's IPv6 address for a device. The hostname is * derived from the device name as printed by `devicectl list devices`. The @@ -95,6 +203,43 @@ export async function getDeviceTunnelIPv6( } } +/** + * Resolve a device's tunnel IPv6 using every strategy we know, in order of + * decreasing reliability: + * + * 1. `devicectl device info details --json-output` (most reliable on + * macOS 26.x; also has the useful side-effect of bumping the tunnel). + * 2. mDNS via `dns.lookup` (getaddrinfo path — does consult mDNSResponder + * on macOS, unlike `dns.resolve6`). + * 3. mDNS via `dns.resolve6` (legacy path — kept for backwards + * compatibility; will ESERVFAIL on recent macOS). + * + * Returns the first address that any strategy yields, or null. + */ +export async function resolveTunnelIPv6(opts: { + udid: string; + deviceName: string; + spawn?: SpawnImpl; + resolve?: ResolveImpl; + legacyResolve?: ResolveImpl; +}): Promise { + const spawn = opts.spawn ?? defaultSpawn; + const resolveLookup = opts.resolve ?? defaultResolve; + const resolveLegacy = opts.legacyResolve ?? legacyResolve6; + + // 1. devicectl-based + const fromDevicectl = getDeviceTunnelIPv6FromDevicectl(opts.udid, spawn); + if (fromDevicectl) return fromDevicectl; + + // 2. mDNS via dns.lookup + const fromLookup = await getDeviceTunnelIPv6(opts.deviceName, resolveLookup); + if (fromLookup) return fromLookup; + + // 3. last-resort: legacy dns.resolve6 + const fromLegacy = await getDeviceTunnelIPv6(opts.deviceName, resolveLegacy); + return fromLegacy; +} + /** * Check whether a specific bundle ID has a running process on the device. */ diff --git a/ios-qa/daemon/src/index.ts b/ios-qa/daemon/src/index.ts index e89507e1c..abfe38be3 100644 --- a/ios-qa/daemon/src/index.ts +++ b/ios-qa/daemon/src/index.ts @@ -21,6 +21,7 @@ import { mintForCaller } from './auth-mint'; import { classifyRoute, proxyToDevice, type DeviceTunnel } from './proxy'; import { writeAudit, writeAttempt, sanitizeReplacer } from './audit'; import { bootstrapTunnel } from './tunnel-bootstrap'; +import { startTunnelKeepalive } from './devicectl'; import type { Capability } from './types'; interface DaemonOptions { @@ -402,6 +403,12 @@ if (import.meta.main) { // Default tunnelProvider: when GSTACK_IOS_TARGET_UDID (or a default with // any connected paired device) is set, bootstrap a real CoreDevice tunnel. // Otherwise return null (proxy will return 503 device_not_connected). + // + // After a successful bootstrap we spawn a periodic devicectl `info details` + // call to keep the CoreDevice tunnel session alive — Xcode 26's CoreDevice + // only holds the tunnel up while a devicectl command is in-flight, so + // without a poke every few seconds the IPv6 becomes unroutable. + let keepalive: { stop: () => void } | null = null; const realTunnelProvider = async () => { const result = await bootstrapTunnel({ udid: targetUDID, @@ -411,9 +418,18 @@ if (import.meta.main) { process.stderr.write(`bootstrap error: ${result.error}${result.detail ? ' — ' + result.detail : ''}\n`); return null; } + if (keepalive) keepalive.stop(); + keepalive = startTunnelKeepalive(result.tunnel.udid); return result.tunnel; }; + const shutdown = () => { + if (keepalive) { keepalive.stop(); keepalive = null; } + }; + process.on('SIGINT', shutdown); + process.on('SIGTERM', shutdown); + process.on('exit', shutdown); + startDaemon({ loopbackPort: port, tailnetEnabled: tailnet, diff --git a/ios-qa/daemon/src/tunnel-bootstrap.ts b/ios-qa/daemon/src/tunnel-bootstrap.ts index aeea9532d..aa6636938 100644 --- a/ios-qa/daemon/src/tunnel-bootstrap.ts +++ b/ios-qa/daemon/src/tunnel-bootstrap.ts @@ -17,7 +17,7 @@ import { randomBytes } from 'crypto'; import type { DeviceTunnel } from './proxy'; import { listDevices, - getDeviceTunnelIPv6, + resolveTunnelIPv6, isAppRunning, launchApp, copyFileFromAppContainer, @@ -97,8 +97,21 @@ export async function bootstrapTunnel(opts: BootstrapOptions): Promise { jsonOutput: { result: { runningProcesses: [{ executable: 'file:///private/var/containers/Bundle/Application/.../com.test.app/com.test', processIdentifier: 1234 }] } }, stdout: 'com.test', }, + { + // devicectl device info details (devicectl-based IPv6 resolution). + // Return no tunnelIPAddress so we fall through to the injected resolver. + argsMatch: /devicectl device info details/, + jsonOutput: { result: { connectionProperties: {} } }, + }, ]); const r = await bootstrapTunnel({ bundleId: 'com.test', @@ -173,6 +184,12 @@ describe('bootstrapTunnel', () => { jsonOutput: { result: { runningProcesses: [{ executable: 'file:///var/containers/Bundle/Application/X/com.test.app/com.test', processIdentifier: 5678 }] } }, stdout: '/com.test.app/', }, + { + // devicectl-based IPv6 resolution succeeds — returns the tunnel + // address directly, so the injected resolveImpl is never called. + argsMatch: /devicectl device info details/, + jsonOutput: { result: { connectionProperties: { tunnelIPAddress: 'fd99::beef' } } }, + }, { argsMatch: /devicectl device copy from/, destOutput: 'BOOT-TOKEN-XYZ-123\n', @@ -233,6 +250,11 @@ describe('bootstrapTunnel', () => { // jsonOutput body contains the bundle id path, so isAppRunning() returns true. jsonOutput: { result: { runningProcesses: [{ executable: 'file:///var/containers/Bundle/Application/X/com.test.app/com.test' }] } }, }, + { + // devicectl device info details returns no tunnel address. + argsMatch: /devicectl device info details/, + jsonOutput: { result: { connectionProperties: {} } }, + }, ]); const r = await bootstrapTunnel({ bundleId: 'com.test', @@ -258,6 +280,10 @@ describe('bootstrapTunnel', () => { argsMatch: /devicectl device info processes -d B/, jsonOutput: { result: { runningProcesses: [{ executable: 'file:///var/containers/Bundle/Application/X/com.test.app/com.test' }] } }, }, + { + argsMatch: /devicectl device info details --device B/, + jsonOutput: { result: { connectionProperties: { tunnelIPAddress: 'fd00::b' } } }, + }, { argsMatch: /devicectl device copy from --device B/, destOutput: 'TOKEN\n', @@ -274,3 +300,132 @@ describe('bootstrapTunnel', () => { if (r.ok) expect(r.tunnel.udid).toBe('B'); }); }); + +describe('getDeviceTunnelIPv6FromDevicectl', () => { + test('extracts tunnelIPAddress from connectionProperties', () => { + const spawn = makeSpawn([ + { + argsMatch: /devicectl device info details --device TEST-UDID/, + jsonOutput: { result: { connectionProperties: { tunnelIPAddress: 'fde4:2827:528e::1' } } }, + }, + ]); + expect(getDeviceTunnelIPv6FromDevicectl('TEST-UDID', spawn)).toBe('fde4:2827:528e::1'); + }); + + test('falls back to result.tunnel.ipAddress when connectionProperties absent', () => { + const spawn = makeSpawn([ + { + argsMatch: /devicectl device info details/, + jsonOutput: { result: { tunnel: { ipAddress: 'fd00::dead:beef' } } }, + }, + ]); + expect(getDeviceTunnelIPv6FromDevicectl('UDID', spawn)).toBe('fd00::dead:beef'); + }); + + test('returns null when devicectl exits non-zero', () => { + const spawn = makeSpawn([ + { argsMatch: /devicectl device info details/, exitCode: 1, stderr: 'no such device' }, + ]); + expect(getDeviceTunnelIPv6FromDevicectl('UDID', spawn)).toBeNull(); + }); + + test('returns null when tunnelIPAddress missing or non-string', () => { + const spawn = makeSpawn([ + { argsMatch: /devicectl device info details/, jsonOutput: { result: { connectionProperties: {} } } }, + ]); + expect(getDeviceTunnelIPv6FromDevicectl('UDID', spawn)).toBeNull(); + }); +}); + +describe('resolveTunnelIPv6 fallback chain', () => { + test('prefers devicectl-based resolution', async () => { + const spawn = makeSpawn([ + { + argsMatch: /devicectl device info details/, + jsonOutput: { result: { connectionProperties: { tunnelIPAddress: 'fd11::1' } } }, + }, + ]); + let resolveCalled = false; + const addr = await resolveTunnelIPv6({ + udid: 'U', + deviceName: 'Test', + spawn, + resolve: async () => { resolveCalled = true; return ['fd99::99']; }, + legacyResolve: async () => { resolveCalled = true; return ['fdAA::AA']; }, + }); + expect(addr).toBe('fd11::1'); + expect(resolveCalled).toBe(false); + }); + + test('falls through to dns.lookup when devicectl yields no address', async () => { + const spawn = makeSpawn([ + { argsMatch: /devicectl device info details/, jsonOutput: { result: { connectionProperties: {} } } }, + ]); + let legacyCalled = false; + const addr = await resolveTunnelIPv6({ + udid: 'U', + deviceName: 'Test', + spawn, + resolve: async () => ['fd22::2'], + legacyResolve: async () => { legacyCalled = true; return ['fdAA::AA']; }, + }); + expect(addr).toBe('fd22::2'); + expect(legacyCalled).toBe(false); + }); + + test('falls through to legacy resolve6 when both devicectl and dns.lookup fail', async () => { + const spawn = makeSpawn([ + { argsMatch: /devicectl device info details/, exitCode: 1 }, + ]); + const addr = await resolveTunnelIPv6({ + udid: 'U', + deviceName: 'Test', + spawn, + resolve: async () => { throw new Error('ESERVFAIL'); }, + legacyResolve: async () => ['fd33::3'], + }); + expect(addr).toBe('fd33::3'); + }); + + test('returns null when all three strategies fail', async () => { + const spawn = makeSpawn([ + { argsMatch: /devicectl device info details/, exitCode: 1 }, + ]); + const addr = await resolveTunnelIPv6({ + udid: 'U', + deviceName: 'Test', + spawn, + resolve: async () => { throw new Error('ESERVFAIL'); }, + legacyResolve: async () => { throw new Error('ESERVFAIL'); }, + }); + expect(addr).toBeNull(); + }); +}); + +describe('startTunnelKeepalive', () => { + test('invokes devicectl on each interval tick', async () => { + const calls: string[] = []; + const spawn: SpawnImpl = ((cmd: string, args: string[]) => { + calls.push(`${cmd} ${args.slice(0, 4).join(' ')}`); + return makeReturn(0, '{}', ''); + }) as SpawnImpl; + const ka = startTunnelKeepalive('UDID-X', { intervalMs: 20, spawn }); + await new Promise((res) => setTimeout(res, 75)); + ka.stop(); + const before = calls.length; + // After stop, no more calls. + await new Promise((res) => setTimeout(res, 50)); + expect(calls.length).toBe(before); + expect(before).toBeGreaterThanOrEqual(2); + expect(calls[0]).toContain('devicectl'); + expect(calls[0]).toContain('device info details'); + }); + + test('stop() is idempotent', () => { + const spawn: SpawnImpl = (() => makeReturn(0, '', '')) as SpawnImpl; + const ka = startTunnelKeepalive('U', { intervalMs: 1_000, spawn }); + ka.stop(); + ka.stop(); + // no throw + }); +}); diff --git a/office-hours/SKILL.md b/office-hours/SKILL.md index b8b6fe1f9..c13b87766 100644 --- a/office-hours/SKILL.md +++ b/office-hours/SKILL.md @@ -1537,12 +1537,9 @@ Count the signals. You'll use this count in Phase 6 to determine which tier of c ### Builder Profile Append After counting signals, append a session entry to the builder profile. This is the single -source of truth for all closing state (tier, resource dedup, journey tracking). - -```bash -eval "$(~/.claude/skills/gstack/bin/gstack-paths)" -mkdir -p "$GSTACK_STATE_ROOT" -``` +source of truth for all closing state (tier, resource dedup, journey tracking). The +`gstack-developer-profile --log-session` binary handles its own directory creation +and writes via atomic mktemp+mv to `~/.gstack/developer-profile.json`. Append one JSON line with these fields (substitute actual values from this session): - `date`: current ISO 8601 timestamp @@ -1556,12 +1553,12 @@ Append one JSON line with these fields (substitute actual values from this sessi - `topics`: array of 2-3 topic keywords that describe what this session was about ```bash -eval "$(~/.claude/skills/gstack/bin/gstack-paths)" -echo '{"date":"TIMESTAMP","mode":"MODE","project_slug":"SLUG","signal_count":N,"signals":SIGNALS_ARRAY,"design_doc":"DOC_PATH","assignment":"ASSIGNMENT_TEXT","resources_shown":[],"topics":TOPICS_ARRAY}' >> "$GSTACK_STATE_ROOT/builder-profile.jsonl" +~/.claude/skills/gstack/bin/gstack-developer-profile --log-session '{"date":"TIMESTAMP","mode":"MODE","project_slug":"SLUG","signal_count":N,"signals":SIGNALS_ARRAY,"design_doc":"DOC_PATH","assignment":"ASSIGNMENT_TEXT","resources_shown":[],"topics":TOPICS_ARRAY}' 2>/dev/null || true ``` -This entry is append-only. The `resources_shown` field will be updated via a second append -after resource selection in Phase 6 Beat 3.5. +The session entry is appended to `developer-profile.json`'s `sessions[]` array. A second +session entry with `mode: "resources"` is appended via `--log-session` after resource +selection in Phase 6 Beat 3.5. --- @@ -2018,8 +2015,8 @@ PAUL GRAHAM ESSAYS: 1. Log the selected resource URLs to the builder profile (single source of truth). Append a resource-tracking entry: ```bash -eval "$(~/.claude/skills/gstack/bin/gstack-paths)" -echo '{"date":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","mode":"resources","project_slug":"'"${SLUG:-unknown}"'","signal_count":0,"signals":[],"design_doc":"","assignment":"","resources_shown":["URL1","URL2","URL3"],"topics":[]}' >> "$GSTACK_STATE_ROOT/builder-profile.jsonl" +eval "$(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null || true)" +~/.claude/skills/gstack/bin/gstack-developer-profile --log-session '{"date":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","mode":"resources","project_slug":"'"${SLUG:-unknown}"'","signal_count":0,"signals":[],"design_doc":"","assignment":"","resources_shown":["URL1","URL2","URL3"],"topics":[]}' 2>/dev/null || true ``` 2. Log the selection to analytics: diff --git a/office-hours/SKILL.md.tmpl b/office-hours/SKILL.md.tmpl index 8546040ec..abb337549 100644 --- a/office-hours/SKILL.md.tmpl +++ b/office-hours/SKILL.md.tmpl @@ -471,12 +471,9 @@ Count the signals. You'll use this count in Phase 6 to determine which tier of c ### Builder Profile Append After counting signals, append a session entry to the builder profile. This is the single -source of truth for all closing state (tier, resource dedup, journey tracking). - -```bash -eval "$(~/.claude/skills/gstack/bin/gstack-paths)" -mkdir -p "$GSTACK_STATE_ROOT" -``` +source of truth for all closing state (tier, resource dedup, journey tracking). The +`gstack-developer-profile --log-session` binary handles its own directory creation +and writes via atomic mktemp+mv to `~/.gstack/developer-profile.json`. Append one JSON line with these fields (substitute actual values from this session): - `date`: current ISO 8601 timestamp @@ -490,12 +487,12 @@ Append one JSON line with these fields (substitute actual values from this sessi - `topics`: array of 2-3 topic keywords that describe what this session was about ```bash -eval "$(~/.claude/skills/gstack/bin/gstack-paths)" -echo '{"date":"TIMESTAMP","mode":"MODE","project_slug":"SLUG","signal_count":N,"signals":SIGNALS_ARRAY,"design_doc":"DOC_PATH","assignment":"ASSIGNMENT_TEXT","resources_shown":[],"topics":TOPICS_ARRAY}' >> "$GSTACK_STATE_ROOT/builder-profile.jsonl" +~/.claude/skills/gstack/bin/gstack-developer-profile --log-session '{"date":"TIMESTAMP","mode":"MODE","project_slug":"SLUG","signal_count":N,"signals":SIGNALS_ARRAY,"design_doc":"DOC_PATH","assignment":"ASSIGNMENT_TEXT","resources_shown":[],"topics":TOPICS_ARRAY}' 2>/dev/null || true ``` -This entry is append-only. The `resources_shown` field will be updated via a second append -after resource selection in Phase 6 Beat 3.5. +The session entry is appended to `developer-profile.json`'s `sessions[]` array. A second +session entry with `mode: "resources"` is appended via `--log-session` after resource +selection in Phase 6 Beat 3.5. --- @@ -892,8 +889,8 @@ PAUL GRAHAM ESSAYS: 1. Log the selected resource URLs to the builder profile (single source of truth). Append a resource-tracking entry: ```bash -eval "$(~/.claude/skills/gstack/bin/gstack-paths)" -echo '{"date":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","mode":"resources","project_slug":"'"${SLUG:-unknown}"'","signal_count":0,"signals":[],"design_doc":"","assignment":"","resources_shown":["URL1","URL2","URL3"],"topics":[]}' >> "$GSTACK_STATE_ROOT/builder-profile.jsonl" +eval "$(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null || true)" +~/.claude/skills/gstack/bin/gstack-developer-profile --log-session '{"date":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","mode":"resources","project_slug":"'"${SLUG:-unknown}"'","signal_count":0,"signals":[],"design_doc":"","assignment":"","resources_shown":["URL1","URL2","URL3"],"topics":[]}' 2>/dev/null || true ``` 2. Log the selection to analytics: diff --git a/package.json b/package.json index 2c39a80b7..3bcaa0f77 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.44.0.0", + "version": "1.44.1.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/test/brain-sync-windows-paths.test.ts b/test/brain-sync-windows-paths.test.ts new file mode 100644 index 000000000..021532d07 --- /dev/null +++ b/test/brain-sync-windows-paths.test.ts @@ -0,0 +1,66 @@ +import { describe, test, expect } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; + +// Static invariants guarding Windows artifact-sync (bin/gstack-brain-sync). +// +// These are deliberately static, not behavioral. The brain-sync integration +// suite (test/brain-sync.test.ts) spawns the bin/ scripts directly, which +// Node/Bun cannot exec on Windows (they are bash-shebang scripts), so that +// suite is excluded from the Windows CI lane. Instead we assert the source +// keeps the properties that make `--discover-new` and the `--once` drain work +// on Windows. Each maps to a confirmed, separately-reproduced failure: +// +// 1. os.path.relpath yields BACKSLASH separators on Windows, which never +// match the forward-slash allowlist globs (e.g. "projects/*/learnings.jsonl"), +// so nested artifacts were silently never discovered. +// 2. discover-new enqueued via subprocess.run([bash-shim]); Windows Python +// cannot exec a shebang script, so it enqueued nothing even once matched. +// 3. compute_paths_to_stage's python print() emits CRLF on Windows; the bash +// `read -r` keeps the trailing \r, so `git add -- "path\r"` matches +// nothing and the drain silently stages/commits nothing. +// +// Plus two robustness properties (independent codex review, both [P2]): +// 4. the inline enqueue must append one atomic record at a time (O_APPEND), +// or a concurrent writer-shim append can interleave mid-record and produce +// a malformed queue line that the drain silently drops. +// 5. the skip-list must be normalized to the same separator form as `rel`, +// or a backslash entry in .brain-skip.txt stops matching and a file the +// user explicitly skipped gets synced. +const ROOT = path.resolve(import.meta.dir, '..'); +const SRC = fs.readFileSync(path.join(ROOT, 'bin', 'gstack-brain-sync'), 'utf-8'); + +describe('gstack-brain-sync — Windows path/exec invariants', () => { + test('discover-new normalizes relpath separators before fnmatch (bug 1)', () => { + expect(SRC).toContain('os.path.relpath(full, gstack_home).replace(os.sep, "/")'); + }); + + test('no python subprocess exec — Windows cannot exec the bash shims (bug 2)', () => { + // The whole script must never shell out to a bin/ bash script from Python; + // that is the exec failure that left discover enqueuing nothing on Windows. + expect(SRC).not.toContain('subprocess'); + }); + + test('drain loop strips trailing CR before git add (bug 3)', () => { + const CR_STRIP = "p=\"${p%$'\\r'}\""; + expect(SRC).toContain(CR_STRIP); + // The strip must precede the staging call, or the pathspec still carries \r. + expect(SRC.indexOf(CR_STRIP)).toBeLessThan(SRC.indexOf('add -f -- "$p"')); + }); + + test('inline enqueue appends one atomic record at a time (codex P2 #1)', () => { + expect(SRC).toContain('os.O_APPEND'); + expect(SRC).toContain('os.write(fd'); + // No buffered batch write to the queue (the interleave-corruption shape). + expect(SRC).not.toContain('open(queue_path, "a"'); + }); + + test('skip-list is normalized on BOTH discover and drain sides (codex P2 #2)', () => { + // The drain (compute_paths_to_stage) is the real staging boundary, so it + // must normalize skip entries identically to discover_new — otherwise a + // backslash .brain-skip.txt entry is honored at discovery but bypassed at + // commit, syncing a file the user explicitly skipped. + const NORM = 's.replace(os.sep, "/") for s in load_lines(skip_path)'; + expect(SRC.split(NORM).length - 1).toBeGreaterThanOrEqual(2); + }); +}); diff --git a/test/diff-scope.test.ts b/test/diff-scope.test.ts index 44cfe03f3..2130a3e57 100644 --- a/test/diff-scope.test.ts +++ b/test/diff-scope.test.ts @@ -140,6 +140,12 @@ describe('gstack-diff-scope', () => { expect(scope.SCOPE_AUTH).toBe('true'); }); + test('detects config via bun.lock (Bun v1.2+ text lockfile)', () => { + const dir = createRepo(['bun.lock']); + const scope = runScope(dir); + expect(scope.SCOPE_CONFIG).toBe('true'); + }); + test('returns false for all new signals when no matching files', () => { const dir = createRepo(['docs/readme.md', 'config.yml']); const scope = runScope(dir); diff --git a/test/gstack-developer-profile.test.ts b/test/gstack-developer-profile.test.ts index 90cac8a7b..ed683bf34 100644 --- a/test/gstack-developer-profile.test.ts +++ b/test/gstack-developer-profile.test.ts @@ -439,3 +439,120 @@ describe('gstack-developer-profile errors', () => { expect(r.stderr).toContain('unknown subcommand'); }); }); + +// ----------------------------------------------------------------------- +// --log-session — the #1671 fix: writer that matches the reader. +// ----------------------------------------------------------------------- + +describe('gstack-developer-profile --log-session (#1671 fix)', () => { + test('regression: read-write-read sequence on fresh $HOME promotes to welcome_back', () => { + // First --read creates an empty stub (this is the bug-shape on current main). + const r1 = runDev('--read'); + expect(r1.stdout).toContain('SESSION_COUNT: 0'); + expect(r1.stdout).toContain('TIER: introduction'); + + // Office-hours writes a session via the new subcommand. + const r2 = runDev('--log-session', JSON.stringify({ + date: '2026-05-23T00:00:00Z', + mode: 'startup', + project_slug: 'test', + signal_count: 2, + signals: ['s1', 's2'], + })); + expect(r2.status).toBe(0); + + // Second --read sees the session — this is what was broken. + const r3 = runDev('--read'); + expect(r3.stdout).toContain('SESSION_COUNT: 1'); + expect(r3.stdout).toContain('TIER: welcome_back'); + expect(r3.stdout).toContain('LAST_PROJECT: test'); + expect(r3.stdout).toContain('TOTAL_SIGNAL_COUNT: 2'); + }); + + test('aggregates signals across multiple sessions', () => { + runDev('--log-session', JSON.stringify({ + date: '2026-05-20T00:00:00Z', mode: 'startup', project_slug: 'p', signals: ['a', 'b'], + })); + runDev('--log-session', JSON.stringify({ + date: '2026-05-21T00:00:00Z', mode: 'startup', project_slug: 'p', signals: ['a', 'c'], + })); + const p = readProfile() as { sessions: unknown[]; signals_accumulated: Record }; + expect(p.sessions.length).toBe(2); + expect(p.signals_accumulated).toEqual({ a: 2, b: 1, c: 1 }); + }); + + test('aggregates resources_shown and topics as deduped unions', () => { + runDev('--log-session', JSON.stringify({ + date: '2026-05-20T00:00:00Z', mode: 'resources', project_slug: 'p', + resources_shown: ['url1', 'url2'], topics: ['ai'], + })); + runDev('--log-session', JSON.stringify({ + date: '2026-05-21T00:00:00Z', mode: 'resources', project_slug: 'p', + resources_shown: ['url2', 'url3'], topics: ['ai', 'eng'], + })); + const p = readProfile() as { resources_shown: string[]; topics: string[] }; + expect(p.resources_shown.sort()).toEqual(['url1', 'url2', 'url3']); + expect(p.topics.sort()).toEqual(['ai', 'eng']); + }); + + test('silently skips invalid JSON input (matches gstack-timeline-log pattern)', () => { + const r = runDev('--log-session', 'not-json'); + expect(r.status).toBe(0); // silent skip, not error + const file = path.join(tmpHome, 'developer-profile.json'); + expect(fs.existsSync(file)).toBe(false); // no stub created either + }); + + test('silently skips JSON missing required fields', () => { + const r = runDev('--log-session', JSON.stringify({ foo: 'bar' })); + expect(r.status).toBe(0); + const file = path.join(tmpHome, 'developer-profile.json'); + expect(fs.existsSync(file)).toBe(false); + }); + + test('injects ts field if missing', () => { + runDev('--log-session', JSON.stringify({ + date: '2026-05-23T00:00:00Z', mode: 'startup', project_slug: 'p', + })); + const p = readProfile() as { sessions: Array<{ ts: string }> }; + expect(p.sessions[0].ts).toMatch(/\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/); + }); + + test('preserves user-set ts field if provided', () => { + runDev('--log-session', JSON.stringify({ + date: '2026-05-23T00:00:00Z', mode: 'startup', project_slug: 'p', + ts: '2026-05-23T12:34:56Z', + })); + const p = readProfile() as { sessions: Array<{ ts: string }> }; + expect(p.sessions[0].ts).toBe('2026-05-23T12:34:56Z'); + }); + + test('do_read picks LAST_* from real sessions, not from a trailing mode:resources entry', () => { + // The Phase 6 resources auto-append happens AFTER the real session in the + // same /office-hours invocation. Without the mode filter, that resources + // entry would clobber LAST_PROJECT/LAST_ASSIGNMENT/LAST_DESIGN_TITLE for + // the next session. + runDev('--log-session', JSON.stringify({ + date: '2026-05-20T00:00:00Z', + mode: 'startup', + project_slug: 'realproj', + assignment: 'real assignment text', + design_doc: 'plans/real.md', + })); + runDev('--log-session', JSON.stringify({ + date: '2026-05-20T01:00:00Z', + mode: 'resources', + project_slug: 'realproj', + assignment: '', + design_doc: '', + resources_shown: ['url1'], + })); + + const r = runDev('--read'); + expect(r.stdout).toContain('LAST_PROJECT: realproj'); + expect(r.stdout).toContain('LAST_ASSIGNMENT: real assignment text'); + expect(r.stdout).toContain('LAST_DESIGN_TITLE: plans/real.md'); + // Resources still aggregate into RESOURCES_SHOWN. + expect(r.stdout).toContain('RESOURCES_SHOWN: url1'); + }); +}); + diff --git a/test/gstack-next-version.test.ts b/test/gstack-next-version.test.ts index 71a80d875..f4ba06926 100644 --- a/test/gstack-next-version.test.ts +++ b/test/gstack-next-version.test.ts @@ -4,6 +4,9 @@ // when the relevant CLI isn't available). import { test, expect, describe } from "bun:test"; +import { mkdirSync, mkdtempSync, writeFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { parseVersion, fmtVersion, @@ -11,6 +14,7 @@ import { cmpVersion, pickNextSlot, markActiveSiblings, + resolveVersionPath, } from "../bin/gstack-next-version"; describe("parseVersion", () => { @@ -150,6 +154,73 @@ describe("markActiveSiblings", () => { }); }); +describe("resolveVersionPath (monorepo VERSION-path support)", () => { + test("CLI flag wins over everything", () => { + const dir = mkdtempSync(join(tmpdir(), "nextver-")); + try { + mkdirSync(join(dir, ".gstack")); + writeFileSync(join(dir, ".gstack", "version-path"), "config/VERSION\n"); + expect(resolveVersionPath("flag/path/VERSION", dir)).toBe("flag/path/VERSION"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test(".gstack/version-path config is picked up", () => { + const dir = mkdtempSync(join(tmpdir(), "nextver-")); + try { + mkdirSync(join(dir, ".gstack")); + writeFileSync(join(dir, ".gstack", "version-path"), "Tinas Second Brain/health-tracker/VERSION\n"); + expect(resolveVersionPath(undefined, dir)).toBe("Tinas Second Brain/health-tracker/VERSION"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test("trims whitespace and ignores blank lines after the first", () => { + const dir = mkdtempSync(join(tmpdir(), "nextver-")); + try { + mkdirSync(join(dir, ".gstack")); + writeFileSync(join(dir, ".gstack", "version-path"), " apps/web/VERSION \n\n# comment-ish line\n"); + expect(resolveVersionPath(undefined, dir)).toBe("apps/web/VERSION"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test("empty config file falls back to default VERSION", () => { + const dir = mkdtempSync(join(tmpdir(), "nextver-")); + try { + mkdirSync(join(dir, ".gstack")); + writeFileSync(join(dir, ".gstack", "version-path"), "\n"); + expect(resolveVersionPath(undefined, dir)).toBe("VERSION"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test("missing config file falls back to default VERSION", () => { + const dir = mkdtempSync(join(tmpdir(), "nextver-")); + try { + expect(resolveVersionPath(undefined, dir)).toBe("VERSION"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test("empty override string falls back to config/default", () => { + // Defensive: "" should NOT win over config — only a non-empty CLI arg should. + const dir = mkdtempSync(join(tmpdir(), "nextver-")); + try { + mkdirSync(join(dir, ".gstack")); + writeFileSync(join(dir, ".gstack", "version-path"), "subproj/VERSION\n"); + expect(resolveVersionPath("", dir)).toBe("subproj/VERSION"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); + // Integration smoke — only runs if gh is available and authenticated. Confirms // the CLI executes end-to-end against real APIs without crashing. describe("integration (smoke)", () => { @@ -181,5 +252,27 @@ describe("integration (smoke)", () => { expect(Array.isArray(parsed.claimed)).toBe(true); expect(parsed).toHaveProperty("siblings"); expect(parsed.siblings).toEqual([]); // --workspace-root null disabled scanning + expect(parsed).toHaveProperty("version_path", "VERSION"); // default when no config + no flag }, 30_000); // Headroom over the 4-5s wall time of the spawned process under load + + test("CLI runs with --version-path and surfaces it in JSON output", async () => { + const proc = Bun.spawnSync([ + "bun", + "run", + "./bin/gstack-next-version", + "--base", + "main", + "--bump", + "patch", + "--current-version", + "1.6.3.0", + "--workspace-root", + "null", + "--version-path", + "Tinas Second Brain/health-tracker/VERSION", + ]); + const out = new TextDecoder().decode(proc.stdout); + const parsed = JSON.parse(out); + expect(parsed).toHaveProperty("version_path", "Tinas Second Brain/health-tracker/VERSION"); + }, 30_000); }); diff --git a/test/gstack-upgrade-migration-v1_40_0_0.test.ts b/test/gstack-upgrade-migration-v1_40_0_0.test.ts new file mode 100644 index 000000000..7bea33007 --- /dev/null +++ b/test/gstack-upgrade-migration-v1_40_0_0.test.ts @@ -0,0 +1,324 @@ +/** + * gstack-upgrade/migrations/v1.40.0.0.sh — migration script unit tests. + * + * Per #1581: the original script unconditionally `touch`ed its done-marker even + * when the jq-gated privacy-map patch was skipped. The fix defers `touch ${DONE}` + * until every required repair either succeeded or was provably unnecessary. + * + * The "regression case" that this file pins is case 2: jq missing + privacy-map + * present → no done-marker. Against the buggy script, case 2 fails (marker is + * written despite skipped patch); against the fix it passes. + * + * Strategy: each test sets up an isolated tmpHome with controlled fixture + * content, and runs the migration via `spawnSync('bash', [MIGRATION], …)`. + * For "jq missing" we point PATH at a curated dir of symlinks to the standard + * utilities the script uses, omitting jq. For "jq mutation fails" we point PATH + * at a dir containing a jq shim that exits 1. + */ + +import { describe, test, expect, beforeEach, afterEach } from "bun:test"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { spawnSync } from "child_process"; + +const ROOT = path.resolve(import.meta.dir, ".."); +const MIGRATION = path.join( + ROOT, + "gstack-upgrade", + "migrations", + "v1.40.0.0.sh", +); + +const NEW_PATTERN = "projects/*/*-eng-review-test-plan-*.md"; +const REAL_PATH = "/usr/bin:/bin:/opt/homebrew/bin"; + +let tmpHome: string; +let gstackHome: string; +let migrationDir: string; +let donePath: string; +let allowlistPath: string; +let privacyPath: string; +let gitattrsPath: string; + +beforeEach(() => { + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), "gstack-mig-v1400-")); + gstackHome = path.join(tmpHome, ".gstack"); + migrationDir = path.join(gstackHome, ".migrations"); + donePath = path.join(migrationDir, "v1.40.0.0.done"); + allowlistPath = path.join(gstackHome, ".brain-allowlist"); + privacyPath = path.join(gstackHome, ".brain-privacy-map.json"); + gitattrsPath = path.join(gstackHome, ".gitattributes"); + fs.mkdirSync(gstackHome, { recursive: true }); +}); + +afterEach(() => { + try { + fs.chmodSync(gstackHome, 0o755); + if (fs.existsSync(allowlistPath)) fs.chmodSync(allowlistPath, 0o644); + if (fs.existsSync(privacyPath)) fs.chmodSync(privacyPath, 0o644); + if (fs.existsSync(gitattrsPath)) fs.chmodSync(gitattrsPath, 0o644); + fs.rmSync(tmpHome, { recursive: true, force: true }); + } catch {} +}); + +/** + * Construct a PATH-style directory of symlinks to standard utilities the + * migration script needs (mkdir, grep, sed, mv, rm, mktemp, cat, touch, printf, + * command, etc.). Optionally omit jq, or substitute a shim. + */ +function makeCuratedPath(opts: { jq?: "missing" | "shim-fail" | "real" } = {}): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "gstack-mig-path-")); + const utils = [ + "bash", + "sh", + "mkdir", + "grep", + "sed", + "mv", + "rm", + "mktemp", + "cat", + "touch", + "printf", + "command", + "echo", + "test", + "[", + "tee", + "true", + "false", + "ls", + "chmod", + ]; + const realDirs = REAL_PATH.split(":"); + for (const u of utils) { + for (const d of realDirs) { + const src = path.join(d, u); + if (fs.existsSync(src)) { + try { + fs.symlinkSync(src, path.join(dir, u)); + } catch {} + break; + } + } + } + const jq = opts.jq ?? "real"; + if (jq === "real") { + for (const d of realDirs) { + const src = path.join(d, "jq"); + if (fs.existsSync(src)) { + try { + fs.symlinkSync(src, path.join(dir, "jq")); + } catch {} + break; + } + } + } else if (jq === "shim-fail") { + const shim = path.join(dir, "jq"); + fs.writeFileSync( + shim, + `#!/usr/bin/env bash\necho "fake jq: refusing" >&2\nexit 1\n`, + { mode: 0o755 }, + ); + } + // jq === "missing" → don't add anything + return dir; +} + +function run(opts: { path?: string } = {}) { + const env = { + HOME: tmpHome, + PATH: opts.path ?? REAL_PATH, + }; + return spawnSync("bash", [MIGRATION], { + env, + encoding: "utf-8", + cwd: tmpHome, + }); +} + +function freshPrivacyMap() { + fs.writeFileSync( + privacyPath, + JSON.stringify( + [{ pattern: "projects/*/*-some-other-*.md", class: "artifact" }], + null, + 2, + ), + ); +} + +function freshAllowlist() { + fs.writeFileSync( + allowlistPath, + "# header\nprojects/*/*-some-other-*.md\n# ---- USER ADDITIONS BELOW\n", + ); +} + +function freshGitattrs() { + fs.writeFileSync(gitattrsPath, "projects/*/*-some-other-*.md merge=union\n"); +} + +describe("migrations/v1.40.0.0.sh", () => { + test("case 1: jq present, fresh privacy-map — all three files patched, marker written", () => { + freshAllowlist(); + freshPrivacyMap(); + freshGitattrs(); + + const r = run(); + + expect(r.status).toBe(0); + expect(fs.existsSync(donePath)).toBe(true); + + const allowlist = fs.readFileSync(allowlistPath, "utf-8"); + expect(allowlist).toContain(NEW_PATTERN); + + const privacy = JSON.parse(fs.readFileSync(privacyPath, "utf-8")); + expect( + privacy.some( + (e: any) => e.pattern === NEW_PATTERN && e.class === "artifact", + ), + ).toBe(true); + + const gitattrs = fs.readFileSync(gitattrsPath, "utf-8"); + expect(gitattrs).toContain(`${NEW_PATTERN} merge=union`); + }); + + test("case 2 (regression for #1581): jq missing, privacy-map exists — marker NOT written, text patches still applied", () => { + freshAllowlist(); + freshPrivacyMap(); + freshGitattrs(); + + const noJq = makeCuratedPath({ jq: "missing" }); + const r = run({ path: noJq }); + + expect(r.status).toBe(0); + expect(r.stderr).toMatch(/jq not found/); + + // Done-marker must NOT be written — this is the whole point of the fix. + expect(fs.existsSync(donePath)).toBe(false); + + // Text-only patches still landed (they don't need jq). + expect(fs.readFileSync(allowlistPath, "utf-8")).toContain(NEW_PATTERN); + expect(fs.readFileSync(gitattrsPath, "utf-8")).toContain( + `${NEW_PATTERN} merge=union`, + ); + + // Privacy-map untouched (still missing the new entry). + const privacy = JSON.parse(fs.readFileSync(privacyPath, "utf-8")); + expect(privacy.some((e: any) => e.pattern === NEW_PATTERN)).toBe(false); + }); + + test("case 3: jq missing, then jq restored — second run completes patch and writes marker", () => { + freshAllowlist(); + freshPrivacyMap(); + freshGitattrs(); + + // First run with jq missing + const noJq = makeCuratedPath({ jq: "missing" }); + const r1 = run({ path: noJq }); + expect(r1.status).toBe(0); + expect(fs.existsSync(donePath)).toBe(false); + + // Second run with jq restored + const r2 = run(); + expect(r2.status).toBe(0); + expect(fs.existsSync(donePath)).toBe(true); + + const privacy = JSON.parse(fs.readFileSync(privacyPath, "utf-8")); + expect( + privacy.some( + (e: any) => e.pattern === NEW_PATTERN && e.class === "artifact", + ), + ).toBe(true); + }); + + test("case 4: jq present, privacy-map already has correct entry — idempotent, marker written", () => { + freshAllowlist(); + fs.writeFileSync( + privacyPath, + JSON.stringify( + [{ pattern: NEW_PATTERN, class: "artifact" }], + null, + 2, + ), + ); + freshGitattrs(); + + const r = run(); + expect(r.status).toBe(0); + expect(fs.existsSync(donePath)).toBe(true); + + const privacy = JSON.parse(fs.readFileSync(privacyPath, "utf-8")); + const matches = privacy.filter((e: any) => e.pattern === NEW_PATTERN); + expect(matches.length).toBe(1); + expect(matches[0].class).toBe("artifact"); + }); + + test("case 5: jq present, privacy-map file missing — allowlist + gitattrs patched, marker written", () => { + freshAllowlist(); + // No privacy-map file + freshGitattrs(); + + const r = run(); + expect(r.status).toBe(0); + expect(fs.existsSync(donePath)).toBe(true); + expect(fs.existsSync(privacyPath)).toBe(false); + + expect(fs.readFileSync(allowlistPath, "utf-8")).toContain(NEW_PATTERN); + expect(fs.readFileSync(gitattrsPath, "utf-8")).toContain( + `${NEW_PATTERN} merge=union`, + ); + }); + + test("case 6: jq present, privacy-map JSON malformed — no marker, error logged, no mutation", () => { + freshAllowlist(); + fs.writeFileSync(privacyPath, "{ this is not json ["); + freshGitattrs(); + + const r = run(); + expect(r.status).toBe(0); + // No marker — broken JSON should NOT be papered over. + expect(fs.existsSync(donePath)).toBe(false); + // Privacy-map content untouched. + expect(fs.readFileSync(privacyPath, "utf-8")).toBe("{ this is not json ["); + }); + + test("case 7: jq present but mutation fails (shim exit 1) — no marker, tempfile cleaned up", () => { + freshAllowlist(); + freshPrivacyMap(); + freshGitattrs(); + + const fakeJq = makeCuratedPath({ jq: "shim-fail" }); + const r = run({ path: fakeJq }); + + expect(r.status).toBe(0); + expect(fs.existsSync(donePath)).toBe(false); + + // Tempfile cleanup: no leftover *.tmp.* sidecars. + const leftovers = fs + .readdirSync(gstackHome) + .filter((n) => n.startsWith(".brain-privacy-map.json.tmp.")); + expect(leftovers.length).toBe(0); + }); + + test("case 8: allowlist append fails (read-only file, no USER ADDITIONS marker) — no marker, warn logged", () => { + // Allowlist WITHOUT the "# ---- USER ADDITIONS BELOW" marker — the script + // falls into the plain `printf >>` append path. Make the file read-only + // so the append fails (sed -i.bak on macOS silently no-ops on read-only + // files, so we have to take the printf path to exercise this). + fs.writeFileSync( + allowlistPath, + "# header\nprojects/*/*-some-other-*.md\n", + ); + freshPrivacyMap(); + freshGitattrs(); + fs.chmodSync(allowlistPath, 0o444); + + const r = run(); + expect(r.status).toBe(0); + // Marker must NOT be written when a required repair failed. + expect(fs.existsSync(donePath)).toBe(false); + }); +}); diff --git a/test/investigate-freeze-path.test.ts b/test/investigate-freeze-path.test.ts new file mode 100644 index 000000000..2ef4a72b7 --- /dev/null +++ b/test/investigate-freeze-path.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, test } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const FILES = ['investigate/SKILL.md.tmpl', 'investigate/SKILL.md']; + +describe('investigate freeze path resolution', () => { + for (const rel of FILES) { + const content = fs.readFileSync(path.join(ROOT, rel), 'utf-8'); + + test(`${rel} hook falls back to standalone gstack-freeze install`, () => { + expect(content).toContain('${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh'); + expect(content).toContain('${CLAUDE_SKILL_DIR}/../gstack-freeze/bin/check-freeze.sh'); + expect(content).toContain('[ -x "$S" ] && bash "$S" || exit 0'); + expect(content).toContain("command: 'bash -c ''"); + }); + + test(`${rel} scope lock availability check supports standalone install`, () => { + expect(content).toContain('_FREEZE_SCRIPT="${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh"'); + expect(content).toContain('[ -x "$_FREEZE_SCRIPT" ] || _FREEZE_SCRIPT="${CLAUDE_SKILL_DIR}/../gstack-freeze/bin/check-freeze.sh"'); + expect(content).toContain('[ -x "$_FREEZE_SCRIPT" ] && echo "FREEZE_AVAILABLE" || echo "FREEZE_UNAVAILABLE"'); + }); + } +}); diff --git a/test/static-no-legacy-writes.test.ts b/test/static-no-legacy-writes.test.ts new file mode 100644 index 000000000..197ec75db --- /dev/null +++ b/test/static-no-legacy-writes.test.ts @@ -0,0 +1,144 @@ +/** + * Static invariant test for #1671: nothing in production code should + * append directly to ~/.gstack/builder-profile.jsonl. All session writes + * must go through `gstack-developer-profile --log-session`. The legacy + * file is now read-only — populated only by the pre-existing migration + * and reconcile paths in bin/gstack-developer-profile. + * + * Prevents future regressions onto the legacy file that would re-create + * the original bug (writer and reader disagreeing on storage location). + * + * Mirrors `test/setup-windows-fallback.test.ts`'s style — static invariant + * via grep, resilient to line-number drift. + */ + +import { describe, test, expect } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; + +const ROOT = path.resolve(import.meta.dir, '..'); + +// Paths allowed to mention builder-profile.jsonl. These read the file +// or document its existence — they do not write to it. +const ALLOWED_FILES = new Set([ + // The binary that reads + reconciles the legacy file. + 'bin/gstack-developer-profile', + // The legacy-shim binary that delegates reads. + 'bin/gstack-builder-profile', + // Memory-ingest reads the legacy file during reconcile period. + 'bin/gstack-memory-ingest.ts', + // The artifacts-init template registers the legacy file in + // .brain-allowlist/.brain-privacy-map for users with pre-existing data. + 'bin/gstack-artifacts-init', + // Documentation files mention the path. + 'CHANGELOG.md', + 'TODOS.md', + 'README.md', + 'office-hours/SKILL.md.tmpl', + 'office-hours/SKILL.md', + 'setup-gbrain/memory.md', + 'docs/designs/FIX_1671_PROFILE_MIGRATION.md', + 'docs/designs/PLAN_TUNING_V0.md', + 'docs/designs/PLAN_TUNING_V1.md', +]); + +// Directories to skip when walking the repo. Everything else is in scope — +// any skill dir, migration script, resolver, or new top-level dir gets +// covered automatically as the repo grows. Catches the "future contributor +// adds the legacy write in retro/SKILL.md.tmpl" regression class. +const SKIP_DIRS = new Set([ + 'node_modules', '.git', '.github', 'dist', 'test', 'docs', + // Vendored binaries / build outputs. + 'browse/dist', 'design/dist', 'extension/node_modules', + // The plan file's directory was already in ALLOWED_FILES; skip docs/ entirely. +]); + +function listSearchDirs(): string[] { + return fs + .readdirSync(ROOT, { withFileTypes: true }) + .filter((d) => d.isDirectory() && !SKIP_DIRS.has(d.name) && !d.name.startsWith('.')) + .map((d) => d.name); +} + +const SEARCH_DIRS = listSearchDirs(); + +function* walk(dir: string): Generator { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const p = path.join(dir, entry.name); + if (entry.isDirectory()) { + yield* walk(p); + } else if (entry.isFile()) { + yield p; + } + } +} + +// Match any literal-path append/write pattern targeting builder-profile.jsonl. +// Captures: `>> .../builder-profile.jsonl`, `writeFileSync(...builder-profile.jsonl...)`, +// `> .../builder-profile.jsonl`. NOTE: this only catches LITERAL-PATH writes — +// variable-indirected writes (`FILE=...builder-profile.jsonl; echo >> "$FILE"`) +// are not detected. The SKILL.md.tmpl assertions below pin the exact #1671 +// regression class directly; this regex is a backstop against the obvious +// pattern, not a comprehensive variable-flow analyzer. +const WRITE_PATTERN = /(>>?\s*["']?[^"'\s]*builder-profile\.jsonl|writeFileSync\([^)]*builder-profile\.jsonl|appendFileSync\([^)]*builder-profile\.jsonl)/; + +describe('#1671 invariant: no production code writes to builder-profile.jsonl', () => { + test('only allowlisted files mention writes to builder-profile.jsonl', () => { + const offending: { file: string; line: number; content: string }[] = []; + + for (const searchDir of SEARCH_DIRS) { + const fullDir = path.join(ROOT, searchDir); + if (!fs.existsSync(fullDir)) continue; + + for (const filePath of walk(fullDir)) { + const rel = path.relative(ROOT, filePath); + + // Skip allowlisted files. + if (ALLOWED_FILES.has(rel)) continue; + + // Only check text-like extensions to avoid binary files. + if (!/\.(sh|ts|js|md|tmpl)$/.test(rel) && !rel.startsWith('bin/')) continue; + + let content: string; + try { + content = fs.readFileSync(filePath, 'utf-8'); + } catch { + continue; + } + + const lines = content.split('\n'); + lines.forEach((line, idx) => { + if (WRITE_PATTERN.test(line)) { + offending.push({ file: rel, line: idx + 1, content: line.trim() }); + } + }); + } + } + + if (offending.length > 0) { + const msg = offending + .map((o) => ` ${o.file}:${o.line} ${o.content}`) + .join('\n'); + throw new Error( + `Found production writes to builder-profile.jsonl outside the allowlist.\n` + + `These would re-create #1671 (writer/reader file mismatch).\n` + + `Use \`gstack-developer-profile --log-session\` instead.\n${msg}`, + ); + } + expect(offending).toEqual([]); + }); + + test('office-hours/SKILL.md uses --log-session, not raw echo append', () => { + const skill = fs.readFileSync(path.join(ROOT, 'office-hours/SKILL.md'), 'utf-8'); + // The two known writer call-sites must use the new subcommand. + expect(skill).toContain('gstack-developer-profile --log-session'); + // And must NOT contain the old echo-append pattern. + expect(skill).not.toMatch(/echo\s+['"][^'"]*['"]?\s*>>\s*["'][^"']*builder-profile\.jsonl/); + }); + + test('office-hours/SKILL.md.tmpl uses --log-session, not raw echo append', () => { + const tmpl = fs.readFileSync(path.join(ROOT, 'office-hours/SKILL.md.tmpl'), 'utf-8'); + expect(tmpl).toContain('gstack-developer-profile --log-session'); + expect(tmpl).not.toMatch(/echo\s+['"][^'"]*['"]?\s*>>\s*["'][^"']*builder-profile\.jsonl/); + }); +}); diff --git a/test/timeline.test.ts b/test/timeline.test.ts index 2504ec1f9..6d30a939e 100644 --- a/test/timeline.test.ts +++ b/test/timeline.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; -import { execSync, ExecSyncOptionsWithStringEncoding } from 'child_process'; +import { execFileSync, execSync, ExecSyncOptionsWithStringEncoding } from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; @@ -42,6 +42,20 @@ function runRead(args: string = ''): string { } } +function runReadArgs(args: string[] = []): string { + const execOpts: ExecSyncOptionsWithStringEncoding = { + cwd: ROOT, + env: { ...process.env, GSTACK_HOME: tmpDir }, + encoding: 'utf-8', + timeout: 15000, + }; + try { + return execFileSync(path.join(BIN, 'gstack-timeline-read'), args, execOpts).trim(); + } catch { + return ''; + } +} + beforeEach(() => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-timeline-')); slugDir = path.join(tmpDir, 'projects'); @@ -136,6 +150,17 @@ describe('gstack-timeline-read', () => { expect(output).not.toContain('feature-b'); }); + test('filters branch names containing single quotes', () => { + runLog(JSON.stringify({ skill: 'review', event: 'completed', branch: "feature/o'hare", outcome: 'approved', ts: '2026-03-28T10:00:00Z' })); + runLog(JSON.stringify({ skill: 'ship', event: 'completed', branch: 'feature-other', outcome: 'merged', ts: '2026-03-28T11:00:00Z' })); + + const output = runReadArgs(['--branch', "feature/o'hare"]); + + expect(output).toContain('review'); + expect(output).toContain("feature/o'hare"); + expect(output).not.toContain('feature-other'); + }); + test('limits output with --limit', () => { for (let i = 0; i < 5; i++) { runLog(JSON.stringify({ skill: 'review', event: 'completed', branch: 'main', outcome: 'approved', ts: `2026-03-2${i}T10:00:00Z` }));