diff --git a/packages/coding-agent/src/prompts/tools/bash.md b/packages/coding-agent/src/prompts/tools/bash.md index 5db18b309..d45ad446e 100644 --- a/packages/coding-agent/src/prompts/tools/bash.md +++ b/packages/coding-agent/src/prompts/tools/bash.md @@ -31,6 +31,15 @@ Executes bash command in shell session for terminal operations like git, bun, ca - `async: true` only defers **reporting** of the result — it does NOT disable, extend, or detach the timeout. A daemon started with `async: true` is still killed when `timeout` elapses, regardless of how long the agent waits before reading the result. - For long-running daemons (dev servers, watchers): either pass an explicit large `timeout` (up to `3600`), or fully detach the process from this shell using `nohup … &` / `setsid … &` / `disown` so it survives independent of the bash call's lifecycle. {{/if}} +{{#if autoBackgroundEnabled}} + +## Auto-background + +- A foreground (non-`async`) call that has not completed within **{{autoBackgroundThresholdSeconds}}s** is automatically converted into a background job and returns a `Background job started: …` notice with the buffered output so far. The command keeps running; the final result is delivered as a follow-up tool call when it completes. +- This is NOT a failure or a re-queue. Treat the notice as "still running, will report back" — do not retry the same command, and do not wait synchronously for it. +- Auto-backgrounding does NOT extend `timeout`: the job is still killed at the original deadline. +- If you need the result inline (e.g. piping into another command), raise `timeout` above the expected duration so it finishes before the threshold matters{{#if asyncEnabled}}, or set `async: true` up front so the contract is explicit{{/if}}. +{{/if}} # Output minimizer diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index 0238695a8..887adbb5e 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -29,6 +29,7 @@ import { type BashInteractiveResult, runInteractiveBashPty } from "./bash-intera import { checkBashInterception } from "./bash-interceptor"; import { canUseInteractiveBashPty } from "./bash-pty-selection"; import { expandInternalUrls, type InternalUrlExpansionOptions } from "./bash-skill-urls"; +import { invalidateGithubCacheForBashCommand } from "./gh-cache-invalidation"; import { formatStyledTruncationWarning, type OutputMeta, stripOutputNotice } from "./output-meta"; import { resolveToCwd } from "./path-utils"; import { capPreviewLines, formatToolWorkingDirectory, replaceTabs } from "./render-utils"; @@ -721,6 +722,12 @@ export class BashTool implements AgentTool { cwd = await expandInternalUrls(cwd, { ...internalUrlOptions, noEscape: true }); } + // Best-effort cache invalidation: drop github-cache rows for any issue/PR + // number touched by a mutating `gh` subcommand inside this bash call so + // subsequent issue:// / pr:// reads pick up the post-mutation state + // instead of the cached pre-mutation snapshot. + invalidateGithubCacheForBashCommand(command); + const commandCwd = cwd ? resolveToCwd(cwd, this.session.cwd) : this.session.cwd; let cwdStat: fs.Stats; try { diff --git a/packages/coding-agent/src/tools/gh-cache-invalidation.ts b/packages/coding-agent/src/tools/gh-cache-invalidation.ts new file mode 100644 index 000000000..42c6da94e --- /dev/null +++ b/packages/coding-agent/src/tools/gh-cache-invalidation.ts @@ -0,0 +1,200 @@ +/** + * Detect cache-mutating `gh` subcommands inside a bash invocation and drop + * the matching `github-cache` rows so a subsequent `issue://` or + * `pr://` read sees the post-mutation state instead of the stale + * pre-mutation snapshot. + * + * Triggered before the bash command runs: on success the cache is now + * empty and the next read fetches fresh; on failure the worst case is one + * extra `gh` round-trip on the following read. That cost is bounded and + * eliminates the much-worse "issue shows OPEN for up to softTtlSec after + * `gh issue close`" failure mode reported by users. + * + * Detector scope: ops that change visible issue/PR state — `close`, + * `reopen`, `merge`, `delete`, `ready`, `lock`, `unlock`, `pin`, `unpin`, + * `transfer`, plus the comment/review/edit ops that change the rendered + * body. We deliberately over-invalidate (e.g. all matching rows for the + * number, all auth_keys) because the upside of staleness elimination + * dwarfs the cost of one cache miss. + */ +import { invalidateAllForNumber } from "./github-cache"; + +const PR_URL_PATTERN = /^https:\/\/github\.com\/([^/\s]+\/[^/\s]+)\/pull\/(\d+)(?:[/?#].*)?$/i; +const ISSUE_URL_PATTERN = /^https:\/\/github\.com\/([^/\s]+\/[^/\s]+)\/issues\/(\d+)(?:[/?#].*)?$/i; + +/** Subcommands that mutate the rendered issue/PR view in any meaningful way. */ +const MUTATING_ISSUE_SUBCMDS: Record = { + close: true, + reopen: true, + delete: true, + edit: true, + comment: true, + lock: true, + unlock: true, + pin: true, + unpin: true, + transfer: true, + develop: true, +}; + +const MUTATING_PR_SUBCMDS: Record = { + close: true, + reopen: true, + merge: true, + ready: true, + edit: true, + comment: true, + review: true, + lock: true, + unlock: true, +}; +/** + * Walk a single shell command's token stream looking for a top-level + * `gh (issue|pr) ` invocation and return the + * invalidation key when one is found. Returns `null` for non-matching + * commands so the caller can iterate cheaply. + */ +function detectGhMutation(tokens: readonly string[]): { number: number; repo?: string } | null { + const ghIdx = tokens.indexOf("gh"); + if (ghIdx === -1) return null; + const subject = tokens[ghIdx + 1]; + if (subject !== "issue" && subject !== "pr") return null; + const subcmd = tokens[ghIdx + 2]; + if (!subcmd) return null; + const expected = subject === "issue" ? MUTATING_ISSUE_SUBCMDS : MUTATING_PR_SUBCMDS; + if (!expected[subcmd]) return null; + + let repo: string | undefined; + // First pass: scan for --repo so it wins regardless of position relative + // to the issue/PR identifier (gh accepts the flag both before and after + // the positional argument). + for (let i = ghIdx + 3; i < tokens.length; i++) { + const token = tokens[i]; + if (token === "-R" || token === "--repo") { + const next = tokens[i + 1]; + if (next) repo = next; + i++; + continue; + } + if (token.startsWith("--repo=")) { + repo = token.slice("--repo=".length); + } + } + for (let i = ghIdx + 3; i < tokens.length; i++) { + const token = tokens[i]; + if (token === "-R" || token === "--repo") { + i++; + continue; + } + if (token.startsWith("-")) continue; + const direct = /^\d+$/.test(token) ? Number(token) : undefined; + if (direct !== undefined && Number.isSafeInteger(direct) && direct > 0) { + return repo !== undefined ? { number: direct, repo } : { number: direct }; + } + const urlMatch = (subject === "pr" ? PR_URL_PATTERN : ISSUE_URL_PATTERN).exec(token); + if (urlMatch) { + const num = Number(urlMatch[2]); + if (Number.isSafeInteger(num) && num > 0) { + // URL carries its own repo and wins over a stray --repo flag. + return { number: num, repo: urlMatch[1] }; + } + } + } + return null; +} + +/** + * Conservative tokenizer that splits a bash command into individual word + * tokens. Handles single/double-quoted strings, backslash escapes, and + * standard operators (`;`, `&&`, `||`, `|`, `&`, newlines) as token + * boundaries that emit a sentinel `";"` so the caller treats the segments + * as independent command sequences. We do not attempt full POSIX shell + * parsing — heredocs, command substitution, and arithmetic expansion are + * out of scope; the detector simply falls through when it cannot find a + * clean `gh issue|pr ` triple. + */ +function tokenize(command: string): string[][] { + const segments: string[][] = []; + let current: string[] = []; + let buffer = ""; + let inSingle = false; + let inDouble = false; + const pushBuffer = () => { + if (buffer.length > 0) { + current.push(buffer); + buffer = ""; + } + }; + const pushSegment = () => { + pushBuffer(); + if (current.length > 0) segments.push(current); + current = []; + }; + for (let i = 0; i < command.length; i++) { + const ch = command[i]; + if (inSingle) { + if (ch === "'") { + inSingle = false; + continue; + } + buffer += ch; + continue; + } + if (inDouble) { + if (ch === "\\" && i + 1 < command.length) { + const next = command[i + 1]; + if (next === '"' || next === "\\" || next === "$" || next === "`") { + buffer += next; + i++; + continue; + } + } + if (ch === '"') { + inDouble = false; + continue; + } + buffer += ch; + continue; + } + if (ch === "'") { + inSingle = true; + continue; + } + if (ch === '"') { + inDouble = true; + continue; + } + if (ch === "\\" && i + 1 < command.length) { + buffer += command[i + 1]; + i++; + continue; + } + if (ch === " " || ch === "\t") { + pushBuffer(); + continue; + } + if (ch === "\n" || ch === ";" || ch === "&" || ch === "|" || ch === "(" || ch === ")") { + pushSegment(); + // `&&`, `||` already collapsed by the segment break above. + continue; + } + buffer += ch; + } + pushSegment(); + return segments; +} + +/** + * Drop `github-cache` rows for any `gh issue|pr ` call + * embedded in `command`. Safe to invoke unconditionally; no-op when the + * command does not touch GitHub state. + */ +export function invalidateGithubCacheForBashCommand(command: string): void { + if (!command?.includes("gh")) return; + const segments = tokenize(command); + for (const segment of segments) { + const hit = detectGhMutation(segment); + if (!hit) continue; + invalidateAllForNumber(hit.number, hit.repo); + } +} diff --git a/packages/coding-agent/src/tools/github-cache.ts b/packages/coding-agent/src/tools/github-cache.ts index 2f3f4ca80..d3a207f24 100644 --- a/packages/coding-agent/src/tools/github-cache.ts +++ b/packages/coding-agent/src/tools/github-cache.ts @@ -316,6 +316,31 @@ export function invalidate( } } +/** + * Drop every cached row for a given issue/PR number, regardless of repo, + * auth key, include_comments flag, or row kind ({@link CacheKind}). Best-effort: + * swallows DB failures the same way {@link invalidate} does. + * + * Used by the bash-side detector that reacts to `gh issue close` / `gh pr merge` + * style mutations. Repo + auth-key narrowing is intentionally skipped because + * the bash command often does not name the repo (defaults to cwd's `gh` + * config) and resolving the *current* repo from `cwd` for every bash call would + * be far more expensive than a write-amplified DELETE. + */ +export function invalidateAllForNumber(number: number, repo?: string): void { + const db = openDb(); + if (!db) return; + try { + if (repo === undefined) { + db.prepare("DELETE FROM github_view_cache WHERE number = ?").run(number); + } else { + db.prepare("DELETE FROM github_view_cache WHERE number = ? AND repo = ?").run(number, normalizeRepo(repo)); + } + } catch (err) { + logger.debug("github cache: invalidateAllForNumber failed", { err: String(err) }); + } +} + /** Drop every cached row. Test helper. */ export function clearAll(): void { const db = openDb(); diff --git a/packages/coding-agent/test/tools/gh-cache-invalidation.test.ts b/packages/coding-agent/test/tools/gh-cache-invalidation.test.ts new file mode 100644 index 000000000..b471bb897 --- /dev/null +++ b/packages/coding-agent/test/tools/gh-cache-invalidation.test.ts @@ -0,0 +1,168 @@ +/** + * Tests for the bash-side gh-cache invalidation parser. Verifies that the + * detector drops cache rows for state-mutating `gh issue|pr` ops while + * leaving unrelated commands and read-only `gh` calls alone. + */ +import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { invalidateGithubCacheForBashCommand } from "@oh-my-pi/pi-coding-agent/tools/gh-cache-invalidation"; +import { + getCached, + putCached, + resetForTests as resetCacheForTests, +} from "@oh-my-pi/pi-coding-agent/tools/github-cache"; + +const REPO = "owner/example"; + +function issuePayload(number: number) { + return { + number, + title: `Issue #${number}`, + state: "OPEN", + author: { login: "octocat" }, + body: "body", + createdAt: "2026-04-01T09:00:00Z", + updatedAt: "2026-04-01T10:00:00Z", + url: `https://github.com/${REPO}/issues/${number}`, + labels: [], + comments: [], + }; +} + +function prPayload(number: number) { + return { + number, + title: `PR #${number}`, + state: "OPEN", + isDraft: false, + baseRefName: "main", + headRefName: "feature/x", + author: { login: "octocat" }, + body: "body", + createdAt: "2026-04-01T09:00:00Z", + updatedAt: "2026-04-01T10:00:00Z", + url: `https://github.com/${REPO}/pull/${number}`, + labels: [], + files: [], + reviews: [], + comments: [], + }; +} + +function seedIssue(number: number, repo = REPO): void { + putCached({ + repo, + kind: "issue", + number, + includeComments: true, + payload: issuePayload(number), + rendered: `issue-${repo}-${number}`, + fetchedAt: 1_000, + }); +} + +function seedPr(number: number, repo = REPO): void { + putCached({ + repo, + kind: "pr", + number, + includeComments: true, + payload: prPayload(number), + rendered: `pr-${repo}-${number}`, + fetchedAt: 1_000, + }); +} + +let tempDir: string; +let originalEnv: string | undefined; + +beforeEach(async () => { + originalEnv = process.env.OMP_GITHUB_CACHE_DB; + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "gh-cache-inv-")); + process.env.OMP_GITHUB_CACHE_DB = path.join(tempDir, "github-cache.db"); + resetCacheForTests(); +}); + +afterEach(async () => { + resetCacheForTests(); + if (originalEnv === undefined) { + delete process.env.OMP_GITHUB_CACHE_DB; + } else { + process.env.OMP_GITHUB_CACHE_DB = originalEnv; + } + await fs.rm(tempDir, { recursive: true, force: true }); +}); + +describe("invalidateGithubCacheForBashCommand", () => { + it("drops cache for `gh issue close `", () => { + seedIssue(42); + invalidateGithubCacheForBashCommand("gh issue close 42"); + expect(getCached(REPO, "issue", 42, true)).toBeNull(); + }); + + it("drops cache for `gh pr merge ` with extra flags", () => { + seedPr(7); + invalidateGithubCacheForBashCommand("gh pr merge 7 --squash --delete-branch"); + expect(getCached(REPO, "pr", 7, true)).toBeNull(); + }); + + it("drops cache for a full PR URL argument", () => { + seedPr(123, "other/repo"); + invalidateGithubCacheForBashCommand("gh pr close https://github.com/other/repo/pull/123"); + expect(getCached("other/repo", "pr", 123, true)).toBeNull(); + }); + + it("drops cache when --repo is supplied separately", () => { + seedIssue(9, "third/repo"); + invalidateGithubCacheForBashCommand("gh issue reopen 9 --repo third/repo"); + expect(getCached("third/repo", "issue", 9, true)).toBeNull(); + }); + + it("drops cache for combined `--repo=` form", () => { + seedIssue(11, "fourth/repo"); + invalidateGithubCacheForBashCommand("gh issue close 11 --repo=fourth/repo"); + expect(getCached("fourth/repo", "issue", 11, true)).toBeNull(); + }); + + it("leaves the cache alone for read-only `gh issue view`", () => { + seedIssue(5); + invalidateGithubCacheForBashCommand("gh issue view 5"); + expect(getCached(REPO, "issue", 5, true)?.rendered).toBe(`issue-${REPO}-5`); + }); + + it("invalidates the relevant issue when the command is chained after another", () => { + seedIssue(1); + invalidateGithubCacheForBashCommand("git add -A && gh issue close 1"); + expect(getCached(REPO, "issue", 1, true)).toBeNull(); + }); + + it("handles quoted issue URL", () => { + seedIssue(33, "quoted/repo"); + invalidateGithubCacheForBashCommand("gh issue close 'https://github.com/quoted/repo/issues/33'"); + expect(getCached("quoted/repo", "issue", 33, true)).toBeNull(); + }); + + it("no-ops on commands that do not mention gh", () => { + seedIssue(99); + invalidateGithubCacheForBashCommand("echo hello world"); + expect(getCached(REPO, "issue", 99, true)?.rendered).toBe(`issue-${REPO}-99`); + }); + + it("invalidates across all repos when only a bare number is supplied", () => { + seedIssue(50, "a/one"); + seedIssue(50, "b/two"); + invalidateGithubCacheForBashCommand("gh issue close 50"); + expect(getCached("a/one", "issue", 50, true)).toBeNull(); + expect(getCached("b/two", "issue", 50, true)).toBeNull(); + }); + + it("invalidates only the matching repo when --repo is supplied", () => { + seedIssue(60, "a/one"); + seedIssue(60, "b/two"); + invalidateGithubCacheForBashCommand("gh issue close 60 --repo a/one"); + expect(getCached("a/one", "issue", 60, true)).toBeNull(); + expect(getCached("b/two", "issue", 60, true)?.rendered).toBe("issue-b/two-60"); + }); +});