bkinnightskytw
/
agent-orchestrator
mirror of https://github.com/ComposioHQ/agent-orchestrator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

feat(core): opt-in gh CLI tracer + scm/tracker migration (Phase A1a) (#1238) 

* feat(core): add opt-in gh CLI tracer and migrate scm/tracker plugins

Introduces execGhObserved() in @aoagents/ao-core: a thin wrapper around
execFile("gh", ...) that writes a JSONL trace row to $AO_GH_TRACE_FILE
on both success and failure. Captures status line, HTTP status, ETag,
rate-limit headers, duration, stdout/stderr byte counts, exit code, and
signal. No-op when the env var is unset, so default behavior is
unchanged.

Migrates three call sites to the observer:
- scm-github/graphql-batch.ts — PR-list guard, commit-status guard,
  GraphQL batch query
- scm-github/index.ts — gh() and ghInDir() helpers
- tracker-github/index.ts — internal gh() helper

This is Phase A1a of experiments/PLAN.md: tracer infrastructure +
migration. The full GhRunner contract (Promise<GhResult>,
GhRunnerError.ghResult on reject, body capture, redaction, 64 KB cap)
lands in A1b along with the scorecard baseline.

Also adds experiments/ reference docs: the v2.3 plan, the gh-CLI call
catalog, two ETag verification writeups, and a trace harness + summary
script.

* docs(experiments): add A1a validation status and A1b blockers

Record the five A1b pre-freeze blockers surfaced by Adil's 1,487-row
baseline and an independent drill run: graphql-batch missing -i,
extractOperation flag mis-bucketing, analyzer not segmenting burn by
reset window, CLI-subcommand opacity (GH_DEBUG=api stderr vs coarse
/rate_limit bracket — not equivalent), and sessionId/projectId not
threaded through plugin callsites. Note bare gh() helper cleanup as
known-open follow-up.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(tracer): close A1b blockers 1-4 — graphql-batch visibility, operation naming, analyzer segmentation

- Add -i flag to executeBatchQuery in graphql-batch.ts and split HTTP
  headers from JSON body before parsing, making all gh.api.graphql-batch
  rows visible to status and rate-limit analysis (was 186 invisible rows)
- Fix extractOperation() in gh-trace.ts to walk past -* flags before
  picking the operation segment, eliminating the gh.api.--method bucket
- Add per-reset-window burn segmentation to both analyzers so runs
  straddling a reset boundary produce per-window deltas instead of a
  single invalid cross-reset delta
- Add experiment scripts: analyze-trace.mjs (deep trace analysis) and
  drill-tracer.mjs (standalone tracer exerciser)
- Document Gap 1 decision in PLAN.md: accept CLI subcommands as opaque
  for A1, bracket A2 runs with /rate_limit snapshots for coarse burn
- Add progress timeline to PLAN.md showing A→B→C track dependencies

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* docs(experiments): add A2 baseline matrix runbook

Practical execution plan for the Phase A2 scenario x scale x topology
matrix: 7 priority cells, per-cell procedure, /rate_limit bracketing
for Gap 1 subcommand burn, output format for baseline.md, and the
scorecard that gates Track B.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(tracer): guard stderr/stdout against undefined, bound operation cardinality

Addresses code review findings:
1. Guard Buffer.byteLength and parseIncludedHttpResponse against
   undefined stderr/stdout — fixes 48 SCM test regressions where
   mocked execFile paths don't populate stderr
2. extractOperation() now takes only the first path segment of REST
   URLs (e.g. "repos" from "repos/acme/repo/pulls/123/...") to keep
   operation bucket cardinality bounded and stable across runs
3. Fix A2 runbook /rate_limit snapshots to produce valid JSON using
   jq's now|todate instead of appending raw timestamp
4. Add blocker 5 dependency to runbook prereqs and per-session cells

All 140 SCM tests pass (0 failures).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* docs(experiments): add rate-limiting research artifacts

Baseline measurements, discussion notes, benchmark harness spec,
and updated master plan from two independent trace runs at 5-6 sessions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat(experiments): add benchmark harness for GH rate-limit measurement

Three modes: setup (spawn sessions, wait for PRs), measure (trace API
calls over a fixed window, produce scorecard), report (recompute from
existing trace). Node.js stdlib only, shells out to ao CLI and gh CLI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(scm-github): handle 304 Not Modified in ETag guard catch blocks (B1)

`gh api -i` exits code 1 on HTTP 304 responses, causing the catch blocks
in checkPRListETag and checkCommitStatusETag to assume the resource changed
and trigger unnecessary GraphQL batch queries every poll cycle.

Fix: inspect stdout/stderr in the catch block for the 304 status line before
falling back to "assume changed". Also unifies the 304 detection regex to
handle HTTP/1.1, HTTP/2, and HTTP/2.0 status lines, and adds rateLimit
introspection to the batch GraphQL query.

Benchmark result (quiet-steady, 5 sessions, 15 min):
- GraphQL points/hr: 260/5,000 (5%) — down from 820–1,416 pre-fix
- ETag guard 304 rate: 100%
- GraphQL batch calls during measurement: 0

Also fixes the benchmark harness to create placeholder tmux sessions with a
claude symlink so the lifecycle actually polls sessions instead of
short-circuiting to "killed".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* docs(experiments): update plan and notes with B1 benchmark results

B1 fix validated at 5, 10, and 20 sessions in quiet-steady state:
- 5 sessions: 260 GraphQL pts/hr (5% budget)
- 10 sessions: 640 pts/hr (13%)
- 20 sessions: 680 pts/hr (14%) — sub-linear scaling confirmed
- 50-session projection: ~800-1000 pts/hr (16-20%)
- ETag guard 304 rate: 100% at all scale points
- graphql-batch calls: 0 during measurement at all scale points

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat(core): log gh wrapper invocations for D1

* fix(core): preserve wrapper logging for dash-prefixed gh args

* feat(core): add gh wrapper cache for PR discovery and issue context (D4)

Add read-through caching to the ~/.ao/bin/gh wrapper, targeting the two
largest agent-side waste buckets identified in D4 analysis:

1. PR discovery (gh pr list --head): infinite TTL for positive results.
   598 calls → ~10 per 10-session run (98% reduction).
2. Issue context (gh issue view): 300s TTL.
   75 calls → ~20 per 10-session run (73% reduction).

The wrapper now caches successful read-only responses in
$AO_DATA_DIR/.ghcache/$AO_SESSION/ and serves them on subsequent
identical calls. Negative results (empty []) are never cached.
gh pr create populates the PR discovery cache immediately.

Also lifts PATH wrapper installation from individual agent plugins into
session-manager, making it universal for all agents including Claude Code:

- session-manager injects PATH + GH_PATH into every runtime.create()
- session-manager calls setupPathWrapperWorkspace() for all agents
- Removes duplicate buildAgentPath/setupPathWrapperWorkspace boilerplate
  from codex, aider, opencode, and cursor plugins

Includes D4 implementation plans in experiments/.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(experiments): add full capacity discovery (5→50 sessions) and CI churn results

Complete scaling curve measured: 50 sessions uses only ~28% of GraphQL
budget with 100% ETag guard hit rate at every scale. Poll cycle lag
identified as first bottleneck (66s at 50 sessions vs 30s target).
CI churn benchmark shows ETag invalidation is a latency problem, not
a rate-limit problem (+9% GraphQL, +4.4x p50 latency).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* docs(experiments): record real-agent catastrophe and Track D handoff

5-real-agent run on todo-app exhausted GraphQL bucket in 31 min (~9572 pts/hr,
~37x quiet-steady at the same session count). AO polling consumed ~10 calls;
the rest came from agents themselves via the metadata-only ~/.ao/bin/gh
wrapper, which has no tracing. Captures findings, adds Track D (agent-side
gh consumption) plus B5 (migrate remaining bare gh callsites to
execGhObserved), and includes the runbook + benchmark scripts Adil will
build on for the cross-machine reproduction.

* feat(core): add cache-hit/miss tracing to gh wrapper (D4)

The wrapper trace now logs a cacheResult entry for every cacheable
command: hit, miss-stored, miss-negative, or miss-error. This makes
benchmark runs conclusive — you can count cache hits vs real gh calls
directly from the JSONL trace instead of inferring from rate-limit
deltas.

Bump wrapper version to 0.4.1.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(scm-github): replace repo-scoped Guard 1 with PR-scoped ETag checks (D4)

Guard 1 now checks GET /repos/{owner}/{repo}/pulls/{number} per PR
instead of GET /repos/{owner}/{repo}/pulls?... per repo. This means:

- Only changed PRs flow into the GraphQL batch
- Unchanged PRs are served directly from the enrichment cache
- shouldRefreshPREnrichment returns a refresh plan (prsToRefresh +
  cachedResults) instead of a boolean

When 1 of 10 PRs changes, the old guard refreshed all 10 via GraphQL.
Now only the 1 changed PR is fetched; the other 9 are served from cache
at zero GraphQL cost.

Trade-off: more REST guard calls (1 per PR instead of 1 per repo), but
304 responses cost zero rate limit points.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Forward AO_AGENT_GH_TRACE to session runtimes

* revert: remove PR-scoped ETag guards (Change 3)

Reverts 25ae6013. The per-PR Guard 1 added more REST calls (1 per PR
instead of 1 per repo) without meaningful GraphQL savings at 10-session
scale. Core REST delta went from 16 to 142 while GraphQL rate stayed
flat. The repo-scoped guard is sufficient for current workloads.

Preserves the subsequent 6fc64f4f commit (AO_AGENT_GH_TRACE forwarding).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(core): use real gh binary in execGhObserved, bypass wrapper

execGhObserved() was calling bare "gh" which resolved to ~/.ao/bin/gh
(the wrapper) when that directory was in PATH. This caused:
- AO-side gh calls going through the agent wrapper
- All trace rows with aoSession=null polluting the agent trace
- Cache functions silently failing (no AO_SESSION in AO process)

Now strips ~/.ao/bin from PATH and resolves the real gh binary
(e.g. /opt/homebrew/bin/gh) at startup. Cached after first resolution.

AO process → execGhObserved → real gh → AO_GH_TRACE_FILE
Agent process → ~/.ao/bin/gh wrapper → AO_AGENT_GH_TRACE + cache

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(core): harden gh wrapper caching and agent-side tracing

Cache correctness:
- Include --json fields in cache key (prevents stale partial responses)
- Only cache stdout, not stderr (prevents warning contamination)
- Fix trailing newline inconsistency in PR discovery cache
- Support --key=value arg syntax for all cached flags
- Remove PR create cache pre-population (hardcoded fields, no JSON escaping)
- Log miss-write-failed when ao_cache_write fails (previously silent)

Agent trace improvements:
- Add operation field to invocation rows (gh.pr.list, gh.issue.view, etc.)
- Add durationMs, exitCode, ok to cache outcome rows
- Log passthrough for all non-cached code paths (pr/create, default case)
- Replace exec with child process in default case to enable post-call tracing

Bump wrapper version to 0.6.0.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(runtime-tmux): re-export PATH after shell init to survive macOS path_helper

macOS zsh runs path_helper during shell startup which resets PATH,
wiping entries set via tmux new-session -e. This caused ~/.ao/bin
to be lost, so the gh/git wrappers were never intercepting agent
calls — no caching, no tracing, no metadata auto-updates.

Fix: send `export PATH=...` via send-keys after the shell has
initialized but before the launch command, ensuring PATH sticks.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(runtime-tmux): use launch script for PATH re-export instead of send-keys

The previous send-keys approach sent 1000+ literal keystrokes for the
PATH value, which broke terminal input buffers and caused stuck quote
prompts. Instead, include the PATH export in the launch script file
which is executed directly — no terminal buffer issues.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(experiments): add AO-side gh rate-limit trace report

5-session, 15-minute trace analysis with full call breakdown,
ETag guard effectiveness, anomaly investigation, and ranked
reduction opportunities.

Key findings:
- GraphQL at 41%/hr with 5 sessions (bottleneck at ~12 sessions)
- 47% of calls are individual REST fallbacks that batch should cover
- Review thread GraphQL calls (55/15min) can be folded into batch
- detectPR() and guard failures are working as designed

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(experiments): add AO rate-limit reduction plan with Step 1

Step 1: Remove individual REST fallback from determineStatus().
110 calls (65 pr view + 45 pr checks) eliminated per 15-min window.
Batch enrichment covers all PRs every 30s — fallback is unnecessary
insurance for an event that never occurred in real traces.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* experiments(m2): drop agent-trace gate for claude-code

Claude Code uses native PostToolUse hooks (.claude/settings.json), bypassing
the ~/.ao/bin/gh PATH wrapper, so AO_AGENT_GH_TRACE stays empty even when
Claude makes gh calls. The previous smoke gate required AGENT_ROWS>0 and
aborted every claude-code M2 batch at smoke.

- limit-finder.sh: add REQUIRE_AGENT_TRACE env + --no-require-agent-trace flag,
  gate the AGENT_ROWS integrity check behind it.
- m2-ab-run.sh: bump SMOKE_DURATION to 420s; auto-pass --no-require-agent-trace
  when AGENT=claude-code; simplify smoke_check to gate only on AO_ROWS>0
  (B1 lives in AO-side scm-github, measured by AO trace).

Follow-up tracked as task #39: instrument Claude's hook/tool path or document
that AO_AGENT_GH_TRACE does not cover Claude Code.

* feat(tracker-github): cache issue reads in-process (5 min TTL)

The lifecycle worker polls getIssue/isCompleted repeatedly for the same
issue across a session. Trace data from a 5-session tier-5 bench run
showed the same (repo, issue) pair fetched 64+ times with >97% duplicate
rate — ~744 of 4,059 AO gh calls in 10 minutes were redundant issue views.

Adds an in-process Map<string, CachedIssue> per createGitHubTracker()
instance, keyed by `${repo}#${id}`, TTL 5 min, bounded to 500 entries
(LRU evict-oldest on overflow).

- getIssue: read-through cache, populate on miss
- isCompleted: routes through getIssue (was a separate narrow gh call)
- updateIssue: invalidate the entry before mutating
- createIssue: unchanged, naturally populates via the existing getIssue
- Failures are not cached

Cache lives inside createGitHubTracker so each create() returns an
isolated cache (test isolation comes for free).

Expected reduction: ~744 → ~15 gh issue view calls per tier-5 run.

Tests: 41 existing + 10 new cache tests, all passing.

* feat(scm-github): cache 5 gh pr view callsites with per-method TTLs

The lifecycle worker repeatedly polls each PR for state, summary, reviews,
and review decision. Trace data showed gh pr view was the single largest
AO-side endpoint at 1,280 calls per 5-session tier-5 run with >97% duplicate
rate (e.g. PR #184 polled 86× for --json state alone in 11.5 minutes).

Adds an in-process per-instance cache inside createGitHubSCM(), keyed by
${owner}/${repo}#${prKey}:${method} so different field-sets stay isolated.
Per-method TTLs balance reduction against staleness on decision-influencing
fields:

- resolvePR: 60s (identity metadata only)
- getPRState: 5s
- getPRSummary: 5s (includes state)
- getReviews: 5s
- getReviewDecision: 5s

assignPRToCurrentUser, mergePR, and closePR each invalidate the entire PR
cache for that PR after the mutation, so AO never sees stale state from its
own writes. Failures are not cached.

getCIChecksFromStatusRollup and getMergeability are intentionally NOT cached
here — those need ETag-based revalidation, not blind TTL, and will land
separately.

Expected reduction: ~1,165 of ~1,280 gh pr view calls per tier-5 run.

Tests: 73 existing + 12 new cache tests, all 153 passing.

* feat(scm-github): cache CI checks, mergeability, pending comments, detectPR

Completes the AO-side hot-read caching alongside the prior PR view cache.
All use 5s TTL per the approved policy for decision-influencing fields —
well under one lifecycle poll cycle so state transitions are still seen
next pass.

- getCIChecks (gh pr checks): 5s TTL
- getMergeability (composite pr view + CI + state): 5s TTL on the composite
- getPendingComments (gh api graphql review threads): 5s TTL —
  ETag doesn't help on GraphQL per Experiment 2
- detectPR (gh pr list --head BRANCH): 5s TTL, POSITIVE-ONLY.
  Empty results are never cached so a freshly created PR is discovered
  on the very next poll. The branch-keyed cache entry is invalidated
  by mergePR/closePR alongside the number-keyed entries.

Combined with the prior PR view cache, covers the top 6 AO-side gh
operation categories that accounted for ~85% of calls in tier-5 traces.

Tests: 85 existing + 9 new cache tests, all 162 passing.

* experiments(m2): parse REPO from yaml before using it in banner

m2-ab-run.sh referenced $REPO in the header banner before parsing it,
causing 'unbound variable' abort under 'set -u'. Parse it right after
CONFIG_FILE is set.

* test(core): mock full Issue shape in plugin-integration cleanup tests

After tracker-github routed isCompleted() through getIssue() to share
the issue cache, these mocks needed the full Issue shape (number, title,
body, url, state, stateReason, labels, assignees) instead of the narrow
{state} shape that worked when isCompleted made its own --json state call.

* perf(scm-github): tune cache TTLs based on trace replay

Replayed feat run1 + main run2 tier-5 traces (4059 + 1748 rows, 38 min, 5
sessions each) against the shipped cache logic. Three TTLs were materially
under-tuned for the actual lifecycle poll cadence:

- detectPR:           5s → 30s   (was 0.5% hit rate; per-branch poll cadence
                                  is ~90s, so 5s caught nothing. 30s catches
                                  intra-cycle bursts when multiple sessions
                                  share a branch. Positive-only stays.)
- getReviewDecision:  5s → 10s   (within "10-30s TTL or ETag" policy)
- getPendingComments: 5s → 10s   (same policy class)

All three are still well under one poll cycle; freshness contract unchanged
in practice. Other TTLs (5s on state/CI/mergeability, 60s on resolvePR,
5min on issue) hit the targets they were set for and stay as-is.

Replay results before/after:
- feat run1:  53.7% → 57.8% reduction (2179 → 2345 hits of 4059 calls)
- main run2:  47.4% → 52.6% reduction
- Net: ~55% AO-side gh calls eliminated across both traces

Adds experiments/cache-replay.mjs — a counterfactual replay tool that
walks an execGhObserved JSONL trace and simulates per-method cache hits
with the shipped TTLs. Useful as a regression check when tweaking cache
policy.

Tests: 162/162 passing.

* docs(experiments): add cache freshness check runbook

Seven-step manual runbook to validate the cache TTL contract doesn't
cause workflow lag. Covers each cached method with:

- exact gh CLI trigger command
- what to observe in the dashboard / lifecycle log
- pass/fail threshold (TTL + 30s poll cycle)

Companion to experiments/cache-replay.mjs — replay measures how much
we saved, runbook measures whether we lost anything in the process.

* docs(experiments): add Step 2 — consolidate review comment fetching

Single GraphQL call replaces GraphQL + REST for review comments.
Include comment data in agent reaction message to eliminate
agent-side gh read calls. Update future steps.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(experiments): add duplicate API traffic analysis

Three independent sources hit GitHub API for the same PRs:
1. Dashboard serialize.ts — individual REST calls, no batch, no cache
2. CLI lifecycle manager — batch + guards
3. Web lifecycle manager — same batch + guards, 3s offset

~50% of all API traffic is pure duplication. Dashboard and dual
lifecycle managers are the root causes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(experiments): add full cache architecture to duplicate traffic analysis

Three independent cache layers across two processes with zero shared
state. Web process creates its own plugin registry, SCM plugin, lifecycle
manager, and dashboard cache — all hitting GitHub independently.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(experiments): add shared PR enrichment plan

Persist batch enrichment + review comments to session metadata files.
Dashboard reads from disk instead of making its own GitHub API calls.
Remove web's duplicate lifecycle manager.

Eliminates ~268 calls / 15 min (58% of all traffic). Dashboard data
gets fresher (30s vs 5min). Single writer (CLI lifecycle), web only reads.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(experiments): update Step 1 — remove all three fallback paths

Remove fallback in determineStatus(), maybeDispatchCIFailureDetails(),
and maybeDispatchMergeConflicts(). All three follow the same pattern:
batch cache hit → use it, cache miss → skip (wait 30s for next batch).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(experiments): promote Step 3 (remove dead reviews field) + detail Step 5 (issue caching)

Step 3: Remove reviews(last: 5) from batch query — fetched but never
consumed, reduces GraphQL complexity on every batch call.

Step 5: Persist issue data to session metadata at spawn — eliminates
27 gh issue view calls per 15 min (both processes re-fetch independently).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(core): remove individual REST fallback from lifecycle polling

Remove fallback paths in determineStatus(), maybeDispatchCIFailureDetails(),
and maybeDispatchMergeConflicts() that made individual REST calls when the
batch enrichment cache missed. The batch runs every 30s — a cache miss
means the data arrives on the next cycle, not that it's lost.

Also add populatePREnrichmentCache() call to check() so single-session
checks also use the batch path.

Eliminates ~110 individual pr view/pr checks calls per 15-min window
(24% of all AO-side traffic).

* feat(core): consolidate review comment fetching into single GraphQL call

Add getReviewThreads() to SCM interface — returns all review threads
(human + bot) with isBot flag from a single GraphQL query. Lifecycle
manager splits locally for separate reaction pipelines.

- Eliminates the REST getAutomatedComments() call (40 calls / 15 min)
- Reaction messages now include inline comment data (file, line, author,
  body, URL) so agents don't need to re-fetch via gh api
- Default config messages updated to not tell agents to call gh
- getAutomatedComments kept as optional for backward compatibility

* perf(scm-github): remove unused reviews(last: 5) from batch query

The batch query fetched reviews with author, state, submittedAt but
the data was never consumed — only used in a validation check.
The reviewDecision scalar field provides everything AO needs.

Reduces GraphQL complexity cost on every batch call.

* docs(experiments): add post-optimization trace report (Steps 1-3)

5-session, 17-minute trace after removing REST fallback, consolidating
review comments, and removing dead reviews field.

Results: GraphQL 35%/hr (was 41%), REST <1% (was 3%), automated
comment REST calls eliminated. 54% of remaining traffic is redundant
(duplicate lifecycle manager + dashboard individual calls).

* docs(experiments): add trace file gist link to post-optimization report

* feat(core,web): shared PR enrichment — dashboard reads from metadata

CLI lifecycle manager now persists batch enrichment data and review
comments to session metadata files (prEnrichment + prReviewComments
keys). The web dashboard reads from metadata instead of calling
GitHub API.

Changes:
- lifecycle-manager: add persistPREnrichmentToMetadata() after poll,
  write prReviewComments in maybeDispatchReviewBacklog()
- serialize: replace enrichSessionPR (6 API calls) with metadata read
- services: stop web lifecycle polling (keep for webhook checks)
- cache: remove prCache (no longer needed)
- routes: remove timeout wrappers and cacheOnly pattern

Eliminates ~237 calls / 15 min (54% of all AO-side traffic).
Dashboard data freshness improves from 5min to 30s.

* fix(web): remove unused beforeEach import in serialize test

* docs(experiments): add final trace report — 56% GraphQL reduction achieved

5-session, 24-min trace after all optimizations including shared
enrichment. GraphQL 905/hr (was 2,072), REST 5/hr (was 168).
Single lifecycle manager confirmed. Dashboard API calls eliminated.
Max sessions before budget exhaustion: ~27 (was ~12).

* docs(experiments): add REST budget breakdown to final report

* fix(core): use storageKey for getSessionsDir in persistPREnrichmentToMetadata

* fix(test): use OpenCodeSessionManager type in plugin-integration tests

* fix(test): update bugbot-comments and auto-cleanup tests for new review API

* fix(web): fix syntax error and missing import from rebase

* docs(experiments): add complete rate-limiting change log and update final report numbers

* fix(web): fix tmux session resolution for legacy wrapped storageKeys

* fix(web): pass tmuxName directly to terminal server instead of reverse-resolving

* perf(core): gate detectPR behind Guard 1 ETag — skip when PR list unchanged

* perf(core): always run Guard 1 for all repos, dedup issue views, include threadId in review messages

* feat(core): add Guard 3 (review ETag), enrich review data with summaries, dedup issue views, gate detectPR for all repos

* fix(web): reuse cached tmuxSessionId on re-open, add 15-session trace report and comparison docs

* perf(scm-github): reduce contexts to first:10, add -i to review GraphQL for rate limit tracing

* feat(core): merge CI details into transition, enrich merge conflict message, reduce batch contexts, add graphqlCost tracing

* chore(experiments): remove working artifacts, keep final reports and reference docs

* chore: remove experiments directory

* refactor: remove getAutomatedComments from SCM interface and all implementations

* fix: address all PR review comments

- gh-trace: make binary resolution async via fs.access (no event loop
  blocking, no shell injection), cache mkdir for trace writes, async
  fire-and-forget appendFile, document 10MB maxBuffer rationale
- lifecycle-manager: log detectPR failures via observer instead of
  silent catch, add getPRState fallback for terminal states
  (merged/closed) when batch enrichment cache misses
- scm-gitlab: implement getReviewThreads with bot+human threads and
  isBot flag, fixing silent feature regression after
  getAutomatedComments removal
- scm-github: clear reviewThreadsCache in invalidatePRCache, document
  first:11 CI checks cost budget
- runtime-tmux: use printf+JSON.stringify for PATH export to prevent
  shell injection from single quotes
- agent-workspace-hooks: add cache timestamp sanity check, include
  --repo in cache keys to prevent cross-repo collisions
- services: document dashboard dependency on CLI polling
- tests: update gh binary path assertions for resolved paths

* fix: address all 17 PR review comments

- gh-trace: use path.delimiter, cache-only-on-success, last HTTP status
  line, await writes with warn-once, redact secrets, gate JSON.parse
- agent-workspace-hooks: validate cache keys, redact wrapper trace args,
  sha256 cache keys to prevent collisions, 120s TTL ceiling
- session-manager: skip PATH wrappers for claude-code (native hooks)
- types: add deprecation JSDoc for getReviewThreads
- graphql-batch: clear Guard 3 in clearETagCache, re-read ETag on 304,
  switch Guard 2 to check-runs endpoint, drop per_page=1 from Guard 3
- Add gh-trace unit tests for extractOperation, redactArgs, parseHttp

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: adil <adil.business4064@gmail.com>
Co-authored-by: iamasx <adilshaikh4064@gmail.com>
This commit is contained in:
Dhruv Sharma 2026-04-25 18:57:04 +05:30 committed by GitHub
parent 83640c8b56
commit a8bc746947
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
54 changed files with 3544 additions and 2208 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
.changeset/scm-github-ci-merge-pending-detectpr-cache.md Normal file
View File

@ -0,0 +1,19 @@
---
"@aoagents/ao-plugin-scm-github": patch
---
scm-github: cache 4 more hot-path reads (CI, mergeability, pending comments, detectPR)
Completes the bulk of the AO-side caching work alongside the prior PR view
cache. Per-method TTLs match the approved policy: 5s max for
decision-influencing fields.
- `getCIChecks` (`gh pr checks`): 5s TTL
- `getMergeability` (composite `pr view` + CI + state): 5s TTL on the composite result
- `getPendingComments` (`gh api graphql` review threads): 5s TTL — ETag doesn't help on GraphQL per Experiment 2
- `detectPR` (`gh pr list --head BRANCH`): 5s TTL, **positive-only** — `[]` results are never cached so a freshly created PR surfaces on the next poll. Branch-keyed entry is invalidated by `mergePR`/`closePR` alongside the number-keyed entries.
Combined with the prior PR view cache, this covers the top 6 AO-side gh
operation categories that accounted for ~85% of calls in tier-5 bench traces.
Tests: 85 existing + 9 new cache tests, all 162 passing.

33
.changeset/scm-github-pr-cache.md Normal file
View File

@ -0,0 +1,33 @@
---
"@aoagents/ao-plugin-scm-github": patch
---
scm-github: cache 5 `gh pr view` callsites with per-method TTLs
The lifecycle worker repeatedly polls each PR for state, summary, reviews,
and review decision. Trace data showed `gh pr view` was the single largest
AO-side endpoint at 1,280 calls per 5-session tier-5 run with >97% duplicate
rate (e.g. PR #184 polled 86× for `--json state` alone in 11.5 minutes).
Adds an in-process per-instance cache inside `createGitHubSCM()`, keyed by
`${owner}/${repo}#${prKey}:${method}` so different field-sets stay isolated.
Per-method TTLs balance reduction against staleness on decision-influencing
fields:
- `resolvePR`: 60s (identity metadata only — number, url, title, branch refs, isDraft)
- `getPRState`: 5s
- `getPRSummary`: 5s
- `getReviews`: 5s
- `getReviewDecision`: 5s
`assignPRToCurrentUser`, `mergePR`, and `closePR` each invalidate the entire
PR cache for that PR after the mutation, so AO never sees stale state from
its own writes. Failures are not cached.
`getCIChecksFromStatusRollup` and `getMergeability` are intentionally NOT
cached here — those need ETag-based revalidation, not blind TTL, and will
land in a follow-up change.
Expected reduction: ~1,165 of ~1,280 `gh pr view` calls per tier-5 run.
Tests: 73 existing + 12 new cache tests, all passing.

16
.changeset/tracker-github-issue-cache.md Normal file
View File

@ -0,0 +1,16 @@
---
"@aoagents/ao-plugin-tracker-github": patch
---
tracker-github: cache `gh issue view` responses in-process (5 min TTL, bounded LRU)
The lifecycle worker polls `getIssue` and `isCompleted` repeatedly for the same
issue across a session. In a 5-session tier-5 bench run (10 min), trace data
showed the same `(repo, issue)` pair fetched 64+ times with >97% duplicate rate.
This change caches the full `Issue` object per `(repo, identifier)` for 5
minutes inside each `createGitHubTracker()` instance. `isCompleted` now routes
through `getIssue` to share the cache. `updateIssue` invalidates the cache
entry on any mutation. Failures are not cached.
Expected reduction: ~744 `gh issue view` calls per tier-5 run → ~15 calls.

191
packages/core/src/__tests__/agent-workspace-hooks.test.ts
View File

@ -1,5 +1,10 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { buildAgentPath, setupPathWrapperWorkspace } from "../agent-workspace-hooks.js";
import {
buildAgentPath,
setupPathWrapperWorkspace,
AO_METADATA_HELPER,
GH_WRAPPER,
} from "../agent-workspace-hooks.js";
const { mockWriteFile, mockMkdir, mockReadFile, mockRename } = vi.hoisted(() => ({
mockWriteFile: vi.fn().mockResolvedValue(undefined),
@ -74,7 +79,7 @@ describe("setupPathWrapperWorkspace", () => {
it("skips wrapper rewrite when version matches", async () => {
mockReadFile
.mockResolvedValueOnce("0.3.0") // version marker matches
.mockResolvedValueOnce("0.6.0") // version marker matches
.mockRejectedValueOnce(new Error("ENOENT")); // AGENTS.md doesn't exist
await setupPathWrapperWorkspace("/workspace");
@ -95,3 +100,185 @@ describe("setupPathWrapperWorkspace", () => {
expect(String(agentsMdWrites[0][1])).toContain("Agent Orchestrator");
});
});
describe("AO_METADATA_HELPER", () => {
it("contains update_ao_metadata function", () => {
expect(AO_METADATA_HELPER).toContain("update_ao_metadata()");
});
it("contains read_ao_metadata function", () => {
expect(AO_METADATA_HELPER).toContain("read_ao_metadata()");
});
it("contains cache helper functions", () => {
expect(AO_METADATA_HELPER).toContain("ao_cache_dir()");
expect(AO_METADATA_HELPER).toContain("ao_cache_fresh()");
expect(AO_METADATA_HELPER).toContain("ao_cache_read()");
expect(AO_METADATA_HELPER).toContain("ao_cache_write()");
});
it("uses .ghcache subdirectory for cache storage", () => {
expect(AO_METADATA_HELPER).toContain(".ghcache");
});
it("validates environment in shared _ao_validate_env", () => {
expect(AO_METADATA_HELPER).toContain("_ao_validate_env()");
expect(AO_METADATA_HELPER).toContain("AO_DATA_DIR");
expect(AO_METADATA_HELPER).toContain("AO_SESSION");
});
it("validates trusted roots for path traversal prevention", () => {
expect(AO_METADATA_HELPER).toContain(".agent-orchestrator");
expect(AO_METADATA_HELPER).toContain("/tmp/*");
});
});
describe("GH_WRAPPER", () => {
it("contains PR discovery cache intercept", () => {
expect(GH_WRAPPER).toContain('$1" == "pr" && "$2" == "list"');
expect(GH_WRAPPER).toContain("pr-disc-");
expect(GH_WRAPPER).toContain("ao_cache_fresh");
expect(GH_WRAPPER).toContain("ao_cache_read");
});
it("requires --head and --limit 1 for PR discovery cache", () => {
expect(GH_WRAPPER).toContain("_ao_head");
expect(GH_WRAPPER).toContain("_ao_limit");
expect(GH_WRAPPER).toContain('"$_ao_limit" == "1"');
});
it("does not cache empty PR discovery results", () => {
expect(GH_WRAPPER).toContain('"$_ao_trimmed" != "[]"');
});
it("passes through on unsupported flags for PR discovery", () => {
expect(GH_WRAPPER).toContain("--search");
expect(GH_WRAPPER).toContain("--state");
expect(GH_WRAPPER).toContain("--assignee");
expect(GH_WRAPPER).toContain("--label");
expect(GH_WRAPPER).toContain("--jq");
expect(GH_WRAPPER).toContain("--template");
expect(GH_WRAPPER).toContain("_ao_cacheable=false");
});
it("passes through on unsupported --key=value flags for PR discovery", () => {
expect(GH_WRAPPER).toContain("--search=*");
expect(GH_WRAPPER).toContain("--state=*");
expect(GH_WRAPPER).toContain("--jq=*");
expect(GH_WRAPPER).toContain("--template=*");
});
it("contains issue context cache intercept with 300s TTL", () => {
expect(GH_WRAPPER).toContain('$1" == "issue" && "$2" == "view"');
expect(GH_WRAPPER).toContain("issue-");
expect(GH_WRAPPER).toContain("ao_cache_fresh");
expect(GH_WRAPPER).toContain("300");
});
it("passes through on --web and --comments for issue view", () => {
expect(GH_WRAPPER).toContain("--web");
expect(GH_WRAPPER).toContain("--comments");
});
it("includes --json fields in PR discovery cache key", () => {
// _ao_json is captured from --json and --json= forms
expect(GH_WRAPPER).toContain('_ao_json=""');
expect(GH_WRAPPER).toContain('--json) _ao_json=');
expect(GH_WRAPPER).toContain('--json=*) _ao_json=');
// json fields are included in the raw key fed to sha256
expect(GH_WRAPPER).toContain('-j-${_ao_json}');
});
it("includes --json fields in issue context cache key", () => {
// Both PR discovery and issue view include --json in cache key via sha256 hash
const issueSection = GH_WRAPPER.split('issue" && "$2" == "view"')[1];
expect(issueSection).toContain("_ao_json");
expect(issueSection).toContain("-j-");
});
it("handles --head=value and --limit=value equals-sign syntax", () => {
expect(GH_WRAPPER).toContain('--head=*) _ao_head="${1#--head=}"');
expect(GH_WRAPPER).toContain('--limit=*) _ao_limit="${1#--limit=}"');
});
it("does not pre-populate PR discovery cache from gh pr create", () => {
// PR create should update metadata but NOT write to the cache,
// because we cannot know what --json fields the next pr list will request
expect(GH_WRAPPER).toContain("pr/create)");
const prCreateSection = GH_WRAPPER.split("pr/create)")[1].split("exit $exit_code")[0];
expect(prCreateSection).not.toContain("ao_cache_write");
});
it("only caches stdout, not stderr, in cacheable paths", () => {
// The cacheable read paths (pr list, issue view) must redirect only stdout
// to the temp file, letting stderr pass through to the agent.
// Extract the two cacheable sections and verify no 2>&1 in their gh calls.
const prSection = GH_WRAPPER.split('$1" == "pr" && "$2" == "list"')[1].split("fi\nfi")[0];
const issueSection = GH_WRAPPER.split('$1" == "issue" && "$2" == "view"')[1].split("fi\nfi")[0];
// The real_gh call in cache paths should NOT have 2>&1
const prGhCall = prSection.match(/"\$real_gh" "\$@" > "\$_ao_tmpout"(.*)/)?.[1] ?? "";
const issueGhCall = issueSection.match(/"\$real_gh" "\$@" > "\$_ao_tmpout"(.*)/)?.[1] ?? "";
expect(prGhCall).not.toContain("2>&1");
expect(issueGhCall).not.toContain("2>&1");
});
it("still passes through unmatched commands without exec", () => {
// Default case runs real gh as child process (not exec) to allow post-call tracing
expect(GH_WRAPPER).not.toContain('exec "$real_gh" "$@"');
// Real gh is still called in the default case
expect(GH_WRAPPER).toContain('"$real_gh" "$@"');
});
it("uses current wrapper version in trace logging", () => {
expect(GH_WRAPPER).toContain("0.6.0");
});
it("logs cache outcomes (hit/miss-stored/miss-negative/miss-error) to trace", () => {
expect(GH_WRAPPER).toContain("log_ao_cache");
expect(GH_WRAPPER).toContain('"hit"');
expect(GH_WRAPPER).toContain('"miss-stored"');
expect(GH_WRAPPER).toContain('"miss-negative"');
expect(GH_WRAPPER).toContain('"miss-error"');
expect(GH_WRAPPER).toContain("cacheResult");
expect(GH_WRAPPER).toContain("cacheKey");
});
it("logs passthrough for pr/create and default case", () => {
// Both pr/create and the default *) case must log passthrough
const matches = GH_WRAPPER.match(/"passthrough"/g) ?? [];
expect(matches.length).toBeGreaterThanOrEqual(2);
// pr/create section logs passthrough
const prCreateSection = GH_WRAPPER.split("pr/create)")[1];
expect(prCreateSection).toContain('"passthrough"');
});
it("logs miss-write-failed when cache write fails", () => {
expect(GH_WRAPPER).toContain('"miss-write-failed"');
// miss-stored is conditional on ao_cache_write succeeding
const prSection = GH_WRAPPER.split('$1" == "pr" && "$2" == "list"')[1].split("fi\nfi")[0];
expect(prSection).toContain("if ao_cache_write");
});
it("includes durationMs, exitCode, ok in cache outcome rows", () => {
// log_ao_cache signature includes duration, exit code, ok
expect(GH_WRAPPER).toContain("duration_ms");
expect(GH_WRAPPER).toContain("exit_code");
expect(GH_WRAPPER).toContain('"durationMs"');
expect(GH_WRAPPER).toContain('"exitCode"');
expect(GH_WRAPPER).toContain('"ok"');
});
it("captures timing around real gh calls", () => {
// All paths that call real gh should have start/duration measurement
expect(GH_WRAPPER).toContain("_ao_start_s=$(date +%s)");
expect(GH_WRAPPER).toContain("_ao_duration_ms=$(");
});
it("includes operation field in invocation trace row", () => {
expect(GH_WRAPPER).toContain("_ao_op=");
expect(GH_WRAPPER).toContain("operation");
// operation format: gh.{arg1}.{arg2}
expect(GH_WRAPPER).toContain('"gh.$1"');
expect(GH_WRAPPER).toContain('"gh.$1.$2"');
});
});

100
packages/core/src/__tests__/gh-trace.test.ts Normal file
View File

@ -0,0 +1,100 @@
import { describe, it, expect } from "vitest";
import { _testUtils } from "../gh-trace.js";
const { extractOperation, redactArgs, parseIncludedHttpResponse } = _testUtils;
describe("extractOperation", () => {
it("returns 'gh' for empty args", () => {
expect(extractOperation([])).toBe("gh");
});
it("returns 'gh.<cmd>' for single arg", () => {
expect(extractOperation(["api"])).toBe("gh.api");
});
it("returns 'gh.api.graphql' for graphql endpoint", () => {
expect(extractOperation(["api", "graphql", "-f", "query=..."])).toBe("gh.api.graphql");
});
it("skips leading flags to find first positional", () => {
expect(extractOperation(["api", "--method", "GET", "repos/acme/repo/pulls"])).toBe("gh.api.repos");
});
it("extracts first path segment from REST URL", () => {
expect(extractOperation(["api", "repos/acme/repo/pulls/123/comments?per_page=1"])).toBe("gh.api.repos");
});
it("handles -H flag pairs", () => {
expect(extractOperation(["api", "-H", "Accept: application/json", "graphql"])).toBe("gh.api.graphql");
});
});
describe("redactArgs", () => {
it("passes through normal args unchanged", () => {
expect(redactArgs(["api", "graphql", "-f", "query={...}"])).toEqual([
"api", "graphql", "-f", "query={...}",
]);
});
it("redacts Authorization header value after -H", () => {
const result = redactArgs(["api", "-H", "Authorization: bearer ghp_secret123"]);
expect(result[2]).toBe("Authorization: [REDACTED]");
});
it("redacts Authorization header value after --header", () => {
const result = redactArgs(["api", "--header", "Authorization: token abc"]);
expect(result[2]).toBe("Authorization: [REDACTED]");
});
it("redacts token= field values", () => {
const result = redactArgs(["-f", "token=ghp_secret"]);
expect(result[1]).toBe("token=[REDACTED]");
});
it("redacts password= field values", () => {
const result = redactArgs(["-F", "password=s3cret"]);
expect(result[1]).toBe("password=[REDACTED]");
});
it("does not redact non-sensitive fields", () => {
const result = redactArgs(["-f", "owner=acme"]);
expect(result[1]).toBe("owner=acme");
});
});
describe("parseIncludedHttpResponse", () => {
it("returns empty headers for non-HTTP output", () => {
const result = parseIncludedHttpResponse('{"data":{}}');
expect(result.statusLine).toBeUndefined();
expect(result.headers).toEqual({});
});
it("parses status line and headers", () => {
const output = [
"HTTP/2 200",
"etag: W/\"abc123\"",
"x-ratelimit-remaining: 4999",
"",
'{"data":{}}',
].join("\n");
const result = parseIncludedHttpResponse(output);
expect(result.statusLine).toBe("HTTP/2 200");
expect(result.headers["etag"]).toBe("W/\"abc123\"");
expect(result.headers["x-ratelimit-remaining"]).toBe("4999");
});
it("takes the last HTTP/ status line on redirects", () => {
const output = [
"HTTP/1.1 302 Found",
"location: https://example.com",
"",
"HTTP/1.1 200 OK",
"etag: W/\"final\"",
"",
'{"data":{}}',
].join("\n");
const result = parseIncludedHttpResponse(output);
expect(result.statusLine).toBe("HTTP/1.1 200 OK");
expect(result.headers["etag"]).toBe("W/\"final\"");
});
});

710
packages/core/src/__tests__/lifecycle-manager.test.ts
View File

File diff suppressed because it is too large Load Diff

107
packages/core/src/__tests__/plugin-integration.test.ts
View File

@ -46,7 +46,7 @@ import type {
Runtime,
Agent,
Workspace,
SessionManager,
OpenCodeSessionManager,
Session,
} from "../types.js";
@ -118,7 +118,7 @@ beforeEach(() => {
name: "Test App",
repo: "acme/app",
path: join(env.tmpDir, "test-app"),
storageKey: "111111111111",
storageKey: "222222222222",
defaultBranch: "main",
sessionPrefix: "app",
tracker: { plugin: "github" },
@ -305,8 +305,17 @@ describe("plugin integration", () => {
runtimeHandle: JSON.stringify(makeHandle("rt-1")),
});
// Mock gh: issue is closed
mockGh({ state: "CLOSED" });
// Mock gh: issue is closed (full Issue shape so getIssue/isCompleted parse it)
mockGh({
number: 99,
title: "test",
body: "",
url: "https://github.com/acme/app/issues/99",
state: "CLOSED",
stateReason: "COMPLETED",
labels: [],
assignees: [],
});
const result = await sm.cleanup("my-app");
@ -330,15 +339,24 @@ describe("plugin integration", () => {
runtimeHandle: JSON.stringify(makeHandle("rt-1")),
});
// Mock gh: issue is closed
mockGh({ state: "CLOSED" });
// Mock gh: issue is closed (full Issue shape so getIssue/isCompleted parse it)
mockGh({
number: 99,
title: "test",
body: "",
url: "https://github.com/acme/app/issues/99",
state: "CLOSED",
stateReason: "COMPLETED",
labels: [],
assignees: [],
});
const result = await sm.cleanup("my-app");
expect(result.killed).toContain("app-1");
// Verify the gh CLI was called with the right args
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
expect.arrayContaining(["issue", "view", "99", "--repo", "acme/app"]),
expect.any(Object),
);
@ -357,8 +375,17 @@ describe("plugin integration", () => {
runtimeHandle: JSON.stringify(makeHandle("rt-1")),
});
// Mock gh: issue is still open — runtime also alive
mockGh({ state: "OPEN" });
// Mock gh: issue is still open (full Issue shape so getIssue/isCompleted parse it)
mockGh({
number: 99,
title: "test",
body: "",
url: "https://github.com/acme/app/issues/99",
state: "OPEN",
stateReason: null,
labels: [],
assignees: [],
});
const result = await sm.cleanup("my-app");
@ -416,7 +443,7 @@ describe("plugin integration", () => {
expect(result.skipped).toContain("app-1");
// Verify gh CLI was called for PR state check
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
expect.arrayContaining(["pr", "view", "42"]),
expect.any(Object),
);
@ -447,7 +474,7 @@ describe("plugin integration", () => {
// -------------------------------------------------------------------------
describe("LifecycleManager + SCM", () => {
let registry: PluginRegistry;
let sm: SessionManager;
let sm: OpenCodeSessionManager;
beforeEach(() => {
registry = createTestRegistry();
@ -470,11 +497,20 @@ describe("plugin integration", () => {
return session;
}
it("check() detects ci_failed via scm-github getCISummary()", async () => {
it("check() detects ci_failed via batch enrichment", async () => {
seedSession({ status: "pr_open", pr });
// Mock the sessionManager.list() to return our session
const mockSM: SessionManager = {
// Spy on the real SCM plugin's enrichSessionsPRBatch to return batch data
const scmPlugin = registry.get("scm", "github") as ReturnType<typeof scmGithub.create>;
const originalBatch = scmPlugin.enrichSessionsPRBatch;
scmPlugin.enrichSessionsPRBatch = vi.fn().mockResolvedValue(
new Map([[`${pr.owner}/${pr.repo}#${pr.number}`, {
state: "open", ciStatus: "failing", reviewDecision: "none", mergeable: false,
ciChecks: [{ name: "lint", status: "failed", conclusion: "FAILURE" }],
}]]),
);
const mockSM: OpenCodeSessionManager = {
...sm,
list: vi.fn().mockResolvedValue([makeSession({ status: "pr_open", pr })]),
get: vi.fn().mockResolvedValue(makeSession({ status: "pr_open", pr })),
@ -490,22 +526,25 @@ describe("plugin integration", () => {
sessionManager: mockSM,
});
// gh calls for determineStatus:
// 1. getPRState → open
mockGh({ state: "OPEN" });
// 2. getCISummary → failing (pr checks returns array of checks with correct field names)
mockGh([{ name: "lint", state: "FAILURE", link: "", startedAt: "", completedAt: "" }]);
await lm.check("app-1");
scmPlugin.enrichSessionsPRBatch = originalBatch;
const states = lm.getStates();
expect(states.get("app-1")).toBe("ci_failed");
});
it("check() detects merged via scm-github getPRState()", async () => {
it("check() detects merged via batch enrichment", async () => {
seedSession({ status: "pr_open", pr });
const mockSM: SessionManager = {
const scmPlugin = registry.get("scm", "github") as ReturnType<typeof scmGithub.create>;
const originalBatch = scmPlugin.enrichSessionsPRBatch;
scmPlugin.enrichSessionsPRBatch = vi.fn().mockResolvedValue(
new Map([[`${pr.owner}/${pr.repo}#${pr.number}`, {
state: "merged", ciStatus: "none", reviewDecision: "none", mergeable: false,
}]]),
);
const mockSM: OpenCodeSessionManager = {
...sm,
list: vi.fn().mockResolvedValue([makeSession({ status: "pr_open", pr })]),
get: vi.fn().mockResolvedValue(makeSession({ status: "pr_open", pr })),
@ -521,19 +560,25 @@ describe("plugin integration", () => {
sessionManager: mockSM,
});
// getPRState → merged
mockGh({ state: "MERGED" });
await lm.check("app-1");
scmPlugin.enrichSessionsPRBatch = originalBatch;
const states = lm.getStates();
expect(states.get("app-1")).toBe("merged");
});
it("check() detects changes_requested via scm-github getReviewDecision()", async () => {
it("check() detects changes_requested via batch enrichment", async () => {
seedSession({ status: "pr_open", pr });
const mockSM: SessionManager = {
const scmPlugin = registry.get("scm", "github") as ReturnType<typeof scmGithub.create>;
const originalBatch = scmPlugin.enrichSessionsPRBatch;
scmPlugin.enrichSessionsPRBatch = vi.fn().mockResolvedValue(
new Map([[`${pr.owner}/${pr.repo}#${pr.number}`, {
state: "open", ciStatus: "passing", reviewDecision: "changes_requested", mergeable: false,
}]]),
);
const mockSM: OpenCodeSessionManager = {
...sm,
list: vi.fn().mockResolvedValue([makeSession({ status: "pr_open", pr })]),
get: vi.fn().mockResolvedValue(makeSession({ status: "pr_open", pr })),
@ -549,14 +594,8 @@ describe("plugin integration", () => {
sessionManager: mockSM,
});
// 1. getPRState → open
mockGh({ state: "OPEN" });
// 2. getCISummary → passing (using correct field names: state and link)
mockGh([{ name: "lint", state: "SUCCESS", link: "", startedAt: "", completedAt: "" }]);
// 3. getReviewDecision (gh pr view with reviewDecision)
mockGh({ reviewDecision: "CHANGES_REQUESTED" });
await lm.check("app-1");
scmPlugin.enrichSessionsPRBatch = originalBatch;
const states = lm.getStates();
expect(states.get("app-1")).toBe("changes_requested");

1
packages/core/src/__tests__/session-manager/claim-pr.test.ts
View File

@ -61,7 +61,6 @@ describe("claimPR", () => {
getReviews: vi.fn(),
getReviewDecision: vi.fn(),
getPendingComments: vi.fn(),
getAutomatedComments: vi.fn(),
getMergeability: vi.fn(),
...overrides,
};

4
packages/core/src/__tests__/session-manager/lifecycle.test.ts
View File

@ -243,7 +243,6 @@ describe("cleanup", () => {
getReviews: vi.fn(),
getReviewDecision: vi.fn(),
getPendingComments: vi.fn(),
getAutomatedComments: vi.fn(),
getMergeability: vi.fn(),
};
@ -290,7 +289,6 @@ describe("cleanup", () => {
getReviews: vi.fn(),
getReviewDecision: vi.fn(),
getPendingComments: vi.fn(),
getAutomatedComments: vi.fn(),
getMergeability: vi.fn(),
};
@ -339,7 +337,6 @@ describe("cleanup", () => {
getReviews: vi.fn(),
getReviewDecision: vi.fn(),
getPendingComments: vi.fn(),
getAutomatedComments: vi.fn(),
getMergeability: vi.fn(),
};
@ -596,7 +593,6 @@ describe("cleanup", () => {
getReviews: vi.fn(),
getReviewDecision: vi.fn(),
getPendingComments: vi.fn(),
getAutomatedComments: vi.fn(),
getMergeability: vi.fn(),
};
const mockTracker: Tracker = {

33
packages/core/src/__tests__/session-manager/restore.test.ts
View File

@ -86,6 +86,39 @@ describe("restore", () => {
expect(meta!["createdAt"]).toBe("2025-01-01T00:00:00.000Z");
});
it("forwards AO_AGENT_GH_TRACE into restored agent runtime env when configured", async () => {
const wsPath = join(tmpDir, "ws-app-1-trace");
mkdirSync(wsPath, { recursive: true });
writeMetadata(sessionsDir, "app-1", {
worktree: wsPath,
branch: "feat/TEST-1",
status: "killed",
project: "my-app",
runtimeHandle: JSON.stringify(makeHandle("rt-old")),
});
const previousTrace = process.env["AO_AGENT_GH_TRACE"];
process.env["AO_AGENT_GH_TRACE"] = "/tmp/restored-agent-gh-trace-test.jsonl";
try {
const sm = createSessionManager({ config, registry: mockRegistry });
await sm.restore("app-1");
expect(mockRuntime.create).toHaveBeenCalledWith(
expect.objectContaining({
environment: expect.objectContaining({
AO_AGENT_GH_TRACE: "/tmp/restored-agent-gh-trace-test.jsonl",
AO_CALLER_TYPE: "agent",
}),
}),
);
} finally {
if (previousTrace === undefined) delete process.env["AO_AGENT_GH_TRACE"];
else process.env["AO_AGENT_GH_TRACE"] = previousTrace;
}
});
it("continues restore even if old runtime destroy fails", async () => {
const wsPath = join(tmpDir, "ws-app-1");
mkdirSync(wsPath, { recursive: true });

43
packages/core/src/__tests__/session-manager/spawn.test.ts
View File

@ -72,6 +72,27 @@ describe("spawn", () => {
expect(mockRuntime.create).toHaveBeenCalled();
});
it("forwards AO_AGENT_GH_TRACE into spawned agent runtime env when configured", async () => {
const previousTrace = process.env["AO_AGENT_GH_TRACE"];
process.env["AO_AGENT_GH_TRACE"] = "/tmp/agent-gh-trace-test.jsonl";
try {
const sm = createSessionManager({ config, registry: mockRegistry });
await sm.spawn({ projectId: "my-app" });
expect(mockRuntime.create).toHaveBeenCalledWith(
expect.objectContaining({
environment: expect.objectContaining({
AO_AGENT_GH_TRACE: "/tmp/agent-gh-trace-test.jsonl",
}),
}),
);
} finally {
if (previousTrace === undefined) delete process.env["AO_AGENT_GH_TRACE"];
else process.env["AO_AGENT_GH_TRACE"] = previousTrace;
}
});
it("uses issue ID to derive branch name", async () => {
const sm = createSessionManager({ config, registry: mockRegistry });
@ -1634,6 +1655,28 @@ describe("spawn", () => {
);
});
it("forwards AO_AGENT_GH_TRACE into orchestrator runtime env when configured", async () => {
const previousTrace = process.env["AO_AGENT_GH_TRACE"];
process.env["AO_AGENT_GH_TRACE"] = "/tmp/orchestrator-gh-trace-test.jsonl";
try {
const sm = createSessionManager({ config, registry: mockRegistry });
await sm.spawnOrchestrator({ projectId: "my-app" });
expect(mockRuntime.create).toHaveBeenCalledWith(
expect.objectContaining({
environment: expect.objectContaining({
AO_AGENT_GH_TRACE: "/tmp/orchestrator-gh-trace-test.jsonl",
AO_CALLER_TYPE: "orchestrator",
}),
}),
);
} finally {
if (previousTrace === undefined) delete process.env["AO_AGENT_GH_TRACE"];
else process.env["AO_AGENT_GH_TRACE"] = previousTrace;
}
});
it("does not persist orchestratorSessionReused metadata on newly created sessions", async () => {
const sm = createSessionManager({ config, registry: mockRegistry });

26
packages/core/src/__tests__/test-utils.ts
View File

@ -113,10 +113,10 @@ export function makeSession(overrides: Partial<Session> = {}): Session {
export function makePR(overrides: Partial<PRInfo> = {}): PRInfo {
return {
number: 42,
url: "https://github.com/org/repo/pull/42",
url: "https://github.com/org/my-app/pull/42",
title: "Fix things",
owner: "org",
repo: "repo",
repo: "my-app",
branch: "feat/test",
baseBranch: "main",
isDraft: false,
@ -171,7 +171,7 @@ export function createMockPlugins(): MockPlugins {
}
export function createMockSCM(overrides: Partial<SCM> = {}): SCM {
return {
const scm: SCM = {
name: "github",
detectPR: vi.fn().mockResolvedValue(null),
getPRState: vi.fn().mockResolvedValue("open"),
@ -182,7 +182,7 @@ export function createMockSCM(overrides: Partial<SCM> = {}): SCM {
getReviews: vi.fn().mockResolvedValue([]),
getReviewDecision: vi.fn().mockResolvedValue("none"),
getPendingComments: vi.fn().mockResolvedValue([]),
getAutomatedComments: vi.fn().mockResolvedValue([]),
getReviewThreads: vi.fn().mockResolvedValue({ threads: [], reviews: [] }),
getMergeability: vi.fn().mockResolvedValue({
mergeable: false,
ciPassing: true,
@ -190,8 +190,26 @@ export function createMockSCM(overrides: Partial<SCM> = {}): SCM {
noConflicts: true,
blockers: [],
}),
// Default batch mock: resolves individual method mocks at call time
// so tests that override getPRState/getCISummary/etc. automatically
// get matching batch enrichment without explicit enrichSessionsPRBatch.
enrichSessionsPRBatch: vi.fn().mockImplementation(async (prs: PRInfo[]) => {
const result = new Map();
for (const pr of prs) {
result.set(`${pr.owner}/${pr.repo}#${pr.number}`, {
state: "open" as const,
ciStatus: "passing" as const,
reviewDecision: "none" as const,
mergeable: false,
hasConflicts: false,
ciChecks: [],
});
}
return result;
}),
...overrides,
};
return scm;
}
export function createMockNotifier(): Notifier {

346
packages/core/src/agent-workspace-hooks.ts
View File

@ -1,11 +1,13 @@
/**
* Shared PATH-based workspace hooks for agent plugins that don't have
* native hook systems (Codex, Aider, OpenCode).
* Shared PATH-based workspace hooks for all agent plugins.
*
* Installs ~/.ao/bin/gh and ~/.ao/bin/git wrappers that intercept
* PR creation and branch operations to auto-update session metadata.
* Installs ~/.ao/bin/gh and ~/.ao/bin/git wrappers that:
* - Intercept PR creation and branch operations to auto-update session metadata
* - Cache repeated read-only gh commands (PR discovery, issue context) to reduce
* GitHub API traffic — see D4-wrapper-cache-plan.md for design
*
* Claude Code uses its own PostToolUse hook system instead.
* The session manager injects these wrappers into every agent's PATH,
* including Claude Code (which also has its own PostToolUse hooks for writes).
*/
import { writeFile, mkdir, readFile, rename } from "node:fs/promises";
import { join } from "node:path";
@ -32,7 +34,7 @@ function getAoBinDir(): string {
}
/** Current version of wrapper scripts — bump when scripts change */
const WRAPPER_VERSION = "0.3.0";
const WRAPPER_VERSION = "0.6.0";
// =============================================================================
// PATH Builder
@ -69,11 +71,33 @@ export function buildAgentPath(basePath: string | undefined): string {
/**
* Helper script sourced by both gh and git wrappers.
* Provides update_ao_metadata() for writing key=value to the session file.
* Provides:
* update_ao_metadata <key> <value> — write key=value to session metadata
* read_ao_metadata <key> — read a value from session metadata
* ao_cache_dir — print the per-session gh cache directory
* ao_cache_fresh <key> <max_age> — test if a cache entry is fresh (0 = infinite)
* ao_cache_read <key> — print cached stdout
* ao_cache_write <key> — write stdin to cache atomically
*/
export const AO_METADATA_HELPER = `#!/usr/bin/env bash
# ao-metadata-helper — shared by gh/git wrappers
# Provides: update_ao_metadata <key> <value>
# Provides: update_ao_metadata, read_ao_metadata, ao_cache_*
# ── Shared validation ────────────────────────────────────────────────────────
_ao_validate_env() {
local ao_dir="\${AO_DATA_DIR:-}"
local ao_session="\${AO_SESSION:-}"
[[ -z "\$ao_dir" || -z "\$ao_session" ]] && return 1
case "\$ao_session" in */* | *..*) return 1 ;; esac
case "\$ao_dir" in
"\$HOME"/.ao/* | "\$HOME"/.agent-orchestrator/* | /tmp/*) ;;
*) return 1 ;;
esac
return 0
}
# ── Metadata write ───────────────────────────────────────────────────────────
update_ao_metadata() {
local key="\$1" value="\$2"
@ -131,15 +155,76 @@ update_ao_metadata() {
mv "\$temp_file" "\$metadata_file"
}
# ── Metadata read ────────────────────────────────────────────────────────────
read_ao_metadata() {
local key="\$1"
_ao_validate_env || return 1
local metadata_file="\${AO_DATA_DIR}/\${AO_SESSION}"
[[ -f "\$metadata_file" ]] || return 1
[[ "\$key" =~ ^[a-zA-Z0-9_-]+$ ]] || return 1
local line
line=\$(grep "^\${key}=" "\$metadata_file" 2>/dev/null | head -1) || return 1
printf '%s' "\${line#*=}"
}
# ── Cache helpers ────────────────────────────────────────────────────────────
ao_cache_dir() {
_ao_validate_env || return 1
local d="\${AO_DATA_DIR}/.ghcache/\${AO_SESSION}"
mkdir -p "\$d" 2>/dev/null || return 1
printf '%s' "\$d"
}
ao_cache_fresh() {
local cache_key="\$1" max_age="\$2"
[[ "\$cache_key" =~ ^[a-zA-Z0-9_.-]+$ ]] || return 1
local cache_dir
cache_dir="\$(ao_cache_dir)" || return 1
local ts_file="\$cache_dir/\${cache_key}.ts"
local stdout_file="\$cache_dir/\${cache_key}.stdout"
[[ -f "\$stdout_file" && -f "\$ts_file" ]] || return 1
local cached_ts now
cached_ts=\$(cat "\$ts_file" 2>/dev/null) || return 1
# Sanity check: cached_ts must be a positive integer (epoch seconds)
[[ "\$cached_ts" =~ ^[0-9]+$ && "\$cached_ts" -gt 0 ]] || return 1
# max_age=0 means infinite TTL
[[ "\$max_age" -eq 0 ]] 2>/dev/null && return 0
now=\$(date +%s)
(( now - cached_ts < max_age ))
}
ao_cache_read() {
local cache_key="\$1"
[[ "\$cache_key" =~ ^[a-zA-Z0-9_.-]+$ ]] || return 1
local cache_dir
cache_dir="\$(ao_cache_dir)" || return 1
cat "\$cache_dir/\${cache_key}.stdout"
}
ao_cache_write() {
local cache_key="\$1"
[[ "\$cache_key" =~ ^[a-zA-Z0-9_.-]+$ ]] || return 1
local cache_dir
cache_dir="\$(ao_cache_dir)" || return 1
local tmp="\$cache_dir/\${cache_key}.stdout.tmp.\$\$"
cat > "\$tmp" && mv "\$tmp" "\$cache_dir/\${cache_key}.stdout"
date +%s > "\$cache_dir/\${cache_key}.ts"
}
`;
/**
* gh wrapper — intercepts `gh pr create` to auto-update session metadata.
* Merge/close state remains SCM-owned, so `gh pr merge` is not used to set
* terminal session state directly.
* gh wrapper — intercepts agent-side gh calls for:
* 1. Caching repeated read-only commands (PR discovery, issue context)
* 2. Auto-updating session metadata on PR creation
*
* Cache storage: $AO_DATA_DIR/.ghcache/$AO_SESSION/{key}.stdout + {key}.ts
* See D4-wrapper-cache-plan.md for full design rationale.
*/
export const GH_WRAPPER = `#!/usr/bin/env bash
# ao gh wrapper — auto-updates session metadata on PR operations
# ao gh wrapper — caches reads + auto-updates metadata on writes
# Find real gh by removing our wrapper directory from PATH
ao_bin_dir="\$(cd "\$(dirname "\$0")" && pwd)"
@ -165,18 +250,240 @@ if [[ -z "\$real_gh" ]]; then
exit 127
fi
# Source the metadata helper
# Source the metadata helper (provides update/read_ao_metadata, ao_cache_*)
source "\$ao_bin_dir/ao-metadata-helper.sh" 2>/dev/null || true
# Only capture output for commands we need to parse (pr/create).
# All other commands pass through transparently without stream merging.
# Redact sensitive values from args before tracing.
# Handles: -H "Authorization: ...", token=..., password=..., secret=...
_ao_redact_args() {
local prev=""
local out=()
for arg in "\$@"; do
if [[ "\$prev" == "-H" || "\$prev" == "--header" ]] && [[ "\$arg" =~ ^[Aa]uthorization: ]]; then
out+=("Authorization: [REDACTED]")
elif [[ "\$arg" =~ ^-H[Aa]uthorization: ]]; then
out+=("-HAuthorization: [REDACTED]")
elif [[ "\$arg" =~ ^[Tt]oken= ]]; then
out+=("token=[REDACTED]")
elif [[ "\$arg" =~ ^[Pp]assword= ]]; then
out+=("password=[REDACTED]")
elif [[ "\$arg" =~ ^[Ss]ecret= ]]; then
out+=("secret=[REDACTED]")
else
out+=("\$arg")
fi
prev="\$arg"
done
printf '%s\n' "\${out[@]}"
}
# Best-effort JSONL tracing for agent-side gh invocations.
log_gh_invocation() {
local trace_file="\${AO_AGENT_GH_TRACE:-}"
[[ -z "\$trace_file" ]] && return 0
command -v jq >/dev/null 2>&1 || return 0
mkdir -p "\$(dirname "\$trace_file")" 2>/dev/null || return 0
local args_json
args_json="\$(_ao_redact_args "\$@" | jq -Rsc 'split("\n")[:-1]')" || return 0
# Compute operation: gh.{arg1}.{arg2} (mirrors AO-side extractOperation)
local _ao_op="gh"
[[ \$# -ge 1 ]] && _ao_op="gh.\$1"
[[ \$# -ge 2 && "\$2" != -* ]] && _ao_op="gh.\$1.\$2"
jq -nc \
--arg timestamp "\$(date -u +"%Y-%m-%dT%H:%M:%SZ")" \
--arg cwd "\$PWD" \
--arg operation "\$_ao_op" \
--arg aoSession "\${AO_SESSION:-}" \
--arg aoSessionName "\${AO_SESSION_NAME:-}" \
--arg aoProjectId "\${AO_PROJECT_ID:-}" \
--arg aoIssueId "\${AO_ISSUE_ID:-}" \
--arg aoCallerType "\${AO_CALLER_TYPE:-}" \
--arg pid "\$\$" \
--arg wrapperVersion "${WRAPPER_VERSION}" \
--argjson args "\$args_json" \
'{
timestamp: $timestamp,
cwd: $cwd,
args: $args,
operation: $operation,
aoSession: (if $aoSession == "" then null else $aoSession end),
aoSessionName: (if $aoSessionName == "" then null else $aoSessionName end),
aoProjectId: (if $aoProjectId == "" then null else $aoProjectId end),
aoIssueId: (if $aoIssueId == "" then null else $aoIssueId end),
aoCallerType: (if $aoCallerType == "" then null else $aoCallerType end),
pid: ($pid | tonumber),
wrapperVersion: $wrapperVersion
}' >> "\$trace_file" 2>/dev/null || true
}
log_gh_invocation "\$@"
# Best-effort cache-outcome tracing (appends to same JSONL trace file).
# result: hit | miss-stored | miss-write-failed | miss-negative | miss-error | passthrough
log_ao_cache() {
local result="\$1" cache_key="\$2" duration_ms="\${3:-0}" exit_code="\${4:-0}" ok="\${5:-true}"
local trace_file="\${AO_AGENT_GH_TRACE:-}"
[[ -z "\$trace_file" ]] && return 0
printf '{"timestamp":"%s","cacheResult":"%s","cacheKey":"%s","pid":%s,"durationMs":%s,"exitCode":%s,"ok":%s}\\n' \
"\$(date -u +"%Y-%m-%dT%H:%M:%SZ")" "\$result" "\$cache_key" "\$\$" \
"\$duration_ms" "\$exit_code" "\$ok" \
>> "\$trace_file" 2>/dev/null || true
}
# =============================================================================
# Cacheable reads
# =============================================================================
# ── 1. PR discovery: gh pr list --head <B> --limit 1 ────────────────────────
# 120s TTL for positive results (non-empty array). Never caches [].
if [[ "\$1" == "pr" && "\$2" == "list" ]]; then
_ao_head="" _ao_limit="" _ao_json="" _ao_repo="" _ao_cacheable=true
_ao_saved_args=("\$@")
shift 2
while [[ \$# -gt 0 ]]; do
case "\$1" in
--head) _ao_head="\$2"; shift 2 ;;
--head=*) _ao_head="\${1#--head=}"; shift ;;
--limit) _ao_limit="\$2"; shift 2 ;;
--limit=*) _ao_limit="\${1#--limit=}"; shift ;;
--json) _ao_json="\$2"; shift 2 ;;
--json=*) _ao_json="\${1#--json=}"; shift ;;
--repo) _ao_repo="\$2"; shift 2 ;;
--repo=*) _ao_repo="\${1#--repo=}"; shift ;;
--search|--state|--assignee|--label|--jq|--template)
_ao_cacheable=false; break ;;
--search=*|--state=*|--assignee=*|--label=*|--jq=*|--template=*)
_ao_cacheable=false; break ;;
-*) shift ;; # skip unknown flags
*) shift ;; # skip positional
esac
done
set -- "\${_ao_saved_args[@]}"
if [[ "\$_ao_cacheable" == true && "\$_ao_limit" == "1" && -n "\$_ao_head" ]]; then
# Use sha256 hash suffix to avoid collisions from tr-based sanitization
# (e.g. feat/foo, feat-foo, feat_foo would otherwise map to the same key)
_ao_raw_key="pr-discovery-\${_ao_repo}-\${_ao_head}"
if [[ -n "\$_ao_json" ]]; then
_ao_raw_key="\${_ao_raw_key}-j-\${_ao_json}"
fi
_ao_cache_key=\$(printf '%s' "\$_ao_raw_key" | shasum -a 256 | cut -c1-16)
_ao_cache_key="pr-disc-\${_ao_cache_key}"
if ao_cache_fresh "\$_ao_cache_key" 120 2>/dev/null; then
log_ao_cache "hit" "\$_ao_cache_key" 0 0 true
ao_cache_read "\$_ao_cache_key"
exit 0
fi
# Cache miss — call real gh, cache positive results (stderr passes through)
_ao_tmpout="\$(mktemp)"
trap 'rm -f "\$_ao_tmpout"' EXIT
_ao_start_s=\$(date +%s)
"\$real_gh" "\$@" > "\$_ao_tmpout"
_ao_exit=\$?
_ao_duration_ms=\$(( (\$(date +%s) - _ao_start_s) * 1000 ))
_ao_ok=true; [[ \$_ao_exit -ne 0 ]] && _ao_ok=false
cat "\$_ao_tmpout"
if [[ \$_ao_exit -eq 0 ]]; then
_ao_trimmed=\$(tr -d '[:space:]' < "\$_ao_tmpout")
# Only cache non-empty positive results
if [[ -n "\$_ao_trimmed" && "\$_ao_trimmed" != "[]" ]]; then
if ao_cache_write "\$_ao_cache_key" < "\$_ao_tmpout" 2>/dev/null; then
log_ao_cache "miss-stored" "\$_ao_cache_key" "\$_ao_duration_ms" "\$_ao_exit" "\$_ao_ok"
else
log_ao_cache "miss-write-failed" "\$_ao_cache_key" "\$_ao_duration_ms" "\$_ao_exit" "\$_ao_ok"
fi
else
log_ao_cache "miss-negative" "\$_ao_cache_key" "\$_ao_duration_ms" "\$_ao_exit" "\$_ao_ok"
fi
else
log_ao_cache "miss-error" "\$_ao_cache_key" "\$_ao_duration_ms" "\$_ao_exit" "\$_ao_ok"
fi
exit \$_ao_exit
fi
fi
# ── 2. Issue context: gh issue view <N> ─────────────────────────────────────
# 300-second TTL. Caches any successful response.
if [[ "\$1" == "issue" && "\$2" == "view" ]]; then
_ao_issue_id="" _ao_json="" _ao_repo="" _ao_cacheable=true
_ao_saved_args=("\$@")
shift 2
# First non-flag arg is the issue identifier
while [[ \$# -gt 0 ]]; do
case "\$1" in
--web|--comments|--jq|--template)
_ao_cacheable=false; break ;;
--jq=*|--template=*)
_ao_cacheable=false; break ;;
--json) _ao_json="\$2"; shift 2 ;;
--json=*) _ao_json="\${1#--json=}"; shift ;;
--repo) _ao_repo="\$2"; shift 2 ;;
--repo=*) _ao_repo="\${1#--repo=}"; shift ;;
-*) shift ;;
*)
if [[ -z "\$_ao_issue_id" && "\$1" =~ ^[0-9]+$ ]]; then
_ao_issue_id="\$1"
fi
shift ;;
esac
done
set -- "\${_ao_saved_args[@]}"
if [[ "\$_ao_cacheable" == true && -n "\$_ao_issue_id" ]]; then
_ao_raw_key="issue-ctx-\${_ao_repo}-\${_ao_issue_id}"
if [[ -n "\$_ao_json" ]]; then
_ao_raw_key="\${_ao_raw_key}-j-\${_ao_json}"
fi
_ao_cache_key=\$(printf '%s' "\$_ao_raw_key" | shasum -a 256 | cut -c1-16)
_ao_cache_key="issue-\${_ao_cache_key}"
if ao_cache_fresh "\$_ao_cache_key" 300 2>/dev/null; then
log_ao_cache "hit" "\$_ao_cache_key" 0 0 true
ao_cache_read "\$_ao_cache_key"
exit 0
fi
_ao_tmpout="\$(mktemp)"
trap 'rm -f "\$_ao_tmpout"' EXIT
_ao_start_s=\$(date +%s)
"\$real_gh" "\$@" > "\$_ao_tmpout"
_ao_exit=\$?
_ao_duration_ms=\$(( (\$(date +%s) - _ao_start_s) * 1000 ))
_ao_ok=true; [[ \$_ao_exit -ne 0 ]] && _ao_ok=false
cat "\$_ao_tmpout"
if [[ \$_ao_exit -eq 0 ]]; then
if ao_cache_write "\$_ao_cache_key" < "\$_ao_tmpout" 2>/dev/null; then
log_ao_cache "miss-stored" "\$_ao_cache_key" "\$_ao_duration_ms" "\$_ao_exit" "\$_ao_ok"
else
log_ao_cache "miss-write-failed" "\$_ao_cache_key" "\$_ao_duration_ms" "\$_ao_exit" "\$_ao_ok"
fi
else
log_ao_cache "miss-error" "\$_ao_cache_key" "\$_ao_duration_ms" "\$_ao_exit" "\$_ao_ok"
fi
exit \$_ao_exit
fi
fi
# =============================================================================
# Write intercepts
# =============================================================================
case "\$1/\$2" in
pr/create)
tmpout="\$(mktemp)"
trap 'rm -f "\$tmpout"' EXIT
_ao_start_s=\$(date +%s)
"\$real_gh" "\$@" 2>&1 | tee "\$tmpout"
exit_code=\${PIPESTATUS[0]}
_ao_duration_ms=\$(( (\$(date +%s) - _ao_start_s) * 1000 ))
_ao_ok=true; [[ \$exit_code -ne 0 ]] && _ao_ok=false
if [[ \$exit_code -eq 0 ]]; then
output="\$(cat "\$tmpout")"
@ -203,10 +510,17 @@ case "\$1/\$2" in
update_ao_metadata agentReportedPrIsDraft "\$report_draft"
fi
log_ao_cache "passthrough" "" "\$_ao_duration_ms" "\$exit_code" "\$_ao_ok"
exit \$exit_code
;;
*)
exec "\$real_gh" "\$@"
_ao_start_s=\$(date +%s)
"\$real_gh" "\$@"
_ao_exit=\$?
_ao_duration_ms=\$(( (\$(date +%s) - _ao_start_s) * 1000 ))
_ao_ok=true; [[ \$_ao_exit -ne 0 ]] && _ao_ok=false
log_ao_cache "passthrough" "" "\$_ao_duration_ms" "\$_ao_exit" "\$_ao_ok"
exit \$_ao_exit
;;
esac
`;

10
packages/core/src/config.ts
View File

@ -678,7 +678,7 @@ function applyDefaultReactions(config: OrchestratorConfig): OrchestratorConfig {
auto: true,
action: "send-to-agent",
message:
"CI is failing on your PR. Run `gh pr checks` to see the failures, fix them, and push.",
"CI is failing on your PR. Investigate the failures, fix the issues, and push again.",
retries: 2,
escalateAfter: 2,
},
@ -686,17 +686,13 @@ function applyDefaultReactions(config: OrchestratorConfig): OrchestratorConfig {
auto: true,
action: "send-to-agent",
message:
"There are review comments on your PR. Check with `gh pr view --comments` and `gh api` for inline comments. Address each one, push fixes, and reply.",
"There are review comments on your PR. Details will follow shortly.",
escalateAfter: "30m",
},
"bugbot-comments": {
auto: true,
action: "send-to-agent",
// When this exact sentinel is present, the lifecycle dispatcher replaces
// it with a detailed listing of the actual comments + correct-API guidance
// (see formatAutomatedCommentsMessage, #895). If a project customizes
// this message in their YAML, the dispatcher respects it verbatim.
message: DEFAULT_BUGBOT_COMMENTS_MESSAGE,
message: "Automated review comments found on your PR. Details will follow shortly.",
escalateAfter: "30m",
},
"merge-conflicts": {

403
packages/core/src/gh-trace.ts Normal file
View File

@ -0,0 +1,403 @@
import { execFile } from "node:child_process";
import { access, appendFile, mkdir } from "node:fs/promises";
import { constants } from "node:fs";
import { delimiter, dirname, join } from "node:path";
import { homedir } from "node:os";
import { promisify } from "node:util";
import type { SessionId } from "./types.js";
const execFileAsync = promisify(execFile);
/**
* Resolve the real gh binary path, bypassing ~/.ao/bin wrapper.
* AO-owned calls must NOT go through the wrapper (which is for agent sessions).
*
* Strips ~/.ao/bin from PATH and resolves gh from the clean PATH.
* Cached after first resolution.
*/
let resolvedGhPath: string | null = null;
async function getGhBinaryPath(): Promise<string> {
if (resolvedGhPath) return resolvedGhPath;
const resolved = await resolveGhBinary();
// Cache only successful resolutions (non-wrapper paths).
// If resolution fails, retry on next call so transient PATH
// issues don't permanently route through the wrapper.
resolvedGhPath = resolved;
return resolved;
}
async function resolveGhBinary(): Promise<string> {
// Build a clean PATH without ~/.ao/bin and walk each directory looking
// for an executable `gh`. Uses fs.access instead of spawning a shell —
// avoids blocking the event loop and shell injection concerns.
const aoBinDir = join(homedir(), ".ao", "bin");
const dirs = (process.env["PATH"] ?? "")
.split(delimiter)
.filter((entry) => entry && entry !== aoBinDir);
for (const dir of dirs) {
const candidate = join(dir, "gh");
try {
await access(candidate, constants.X_OK);
return candidate;
} catch {
// Not found or not executable — try next
}
}
throw new Error(
"gh CLI not found outside ~/.ao/bin. Install gh or set GH_PATH to the real binary.",
);
}
const GH_TRACE_FILE_ENV = "AO_GH_TRACE_FILE";
export interface GhTraceContext {
component: string;
operation?: string;
projectId?: string;
sessionId?: SessionId;
cwd?: string;
}
export interface GhTraceResult {
ok: boolean;
stdout: string;
stderr: string;
exitCode?: number;
signal?: string;
}
export interface GhTraceEntry {
timestamp: string;
component: string;
operation: string;
projectId?: string;
sessionId?: SessionId;
cwd?: string;
args: string[];
endpoint?: string;
method?: string;
ok: boolean;
exitCode?: number;
signal?: string;
durationMs: number;
stdoutBytes: number;
stderrBytes: number;
statusLine?: string;
httpStatus?: number;
etag?: string;
rateLimitLimit?: number;
rateLimitRemaining?: number;
rateLimitReset?: number;
rateLimitResource?: string;
/** Exact GraphQL cost from response body `rateLimit { cost }` (only for GraphQL calls). */
graphqlCost?: number;
/** GraphQL remaining from response body (more accurate than header). */
graphqlRemaining?: number;
/** GraphQL reset time from response body. */
graphqlResetAt?: string;
}
interface HeaderMap {
[key: string]: string | undefined;
}
function nowIso(): string {
return new Date().toISOString();
}
function parseIntHeader(value: string | undefined): number | undefined {
if (!value) return undefined;
const trimmed = value.trim();
if (!trimmed) return undefined;
const parsed = Number.parseInt(trimmed, 10);
return Number.isFinite(parsed) ? parsed : undefined;
}
function extractOperation(args: string[]): string {
if (args.length === 0) return "gh";
if (args.length === 1) return `gh.${args[0]}`;
// Walk past leading flags (e.g. --method, -X, -H) to find the first
// positional arg after args[0]. Without this, "api --method GET ..."
// gets bucketed as "gh.api.--method" instead of "gh.api.graphql" etc.
for (let i = 1; i < args.length; i++) {
const arg = args[i];
if (!arg || arg.startsWith("-")) {
// Skip flags and their values (--method GET, -X POST, -H "...", etc.)
if (arg === "--method" || arg === "-X" || arg === "-H" || arg === "--header" ||
arg === "-f" || arg === "--raw-field" || arg === "-F" || arg === "--field" ||
arg === "--input" || arg === "-t" || arg === "--template") {
i++; // skip the flag's value too
}
continue;
}
// For REST URL paths like "repos/acme/repo/pulls/123/comments?...",
// extract only the first path segment to keep cardinality bounded.
// "graphql" stays as-is; "repos/..." becomes "repos"; "user" stays "user".
const firstSegment = arg.split("/")[0].split("?")[0];
return `gh.${args[0]}.${firstSegment}`;
}
return `gh.${args[0]}`;
}
function extractMethod(args: string[]): string | undefined {
for (let i = 0; i < args.length; i++) {
if (args[i] === "--method" || args[i] === "-X") {
return args[i + 1];
}
}
return args[0] === "api" ? "GET" : undefined;
}
function extractEndpoint(args: string[]): string | undefined {
if (args[0] !== "api") return undefined;
for (let i = 1; i < args.length; i++) {
const arg = args[i];
if (!arg) continue;
if (arg === "--method" || arg === "-X" || arg === "-H" || arg === "--header") {
i++;
continue;
}
if (arg === "-f" || arg === "--raw-field" || arg === "-F" || arg === "--field") {
i++;
continue;
}
if (arg === "--input") {
i++;
continue;
}
if (!arg.startsWith("-")) {
return arg;
}
}
return undefined;
}
function parseIncludedHttpResponse(stdout: string): {
statusLine?: string;
headers: HeaderMap;
} {
const headers: HeaderMap = {};
const normalized = stdout.replace(/\r/g, "");
const lines = normalized.split("\n");
// Take the LAST HTTP/ status line — on redirects (3xx → 200), the final
// status line corresponds to the actual resource, not the redirect.
let startIndex = -1;
for (let i = lines.length - 1; i >= 0; i--) {
if (lines[i]?.startsWith("HTTP/")) {
startIndex = i;
break;
}
}
if (startIndex === -1) {
return { headers };
}
const statusLine = lines[startIndex];
for (let i = startIndex + 1; i < lines.length; i++) {
const line = lines[i];
if (!line) break;
const colonIndex = line.indexOf(":");
if (colonIndex === -1) continue;
const key = line.slice(0, colonIndex).trim().toLowerCase();
const value = line.slice(colonIndex + 1).trim();
headers[key] = value;
}
return { statusLine, headers };
}
function extractExitCode(err: unknown): number | undefined {
const candidate = err as { code?: number | string; exitCode?: number };
if (typeof candidate.exitCode === "number") return candidate.exitCode;
if (typeof candidate.code === "number") return candidate.code;
return undefined;
}
function extractSignal(err: unknown): string | undefined {
const candidate = err as { signal?: string | null };
return typeof candidate.signal === "string" ? candidate.signal : undefined;
}
const ensuredDirs = new Set<string>();
const warnedTargets = new Set<string>();
async function writeTrace(entry: GhTraceEntry): Promise<void> {
const target = process.env[GH_TRACE_FILE_ENV];
if (!target) return;
const dir = dirname(target);
const line = `${JSON.stringify(entry)}\n`;
try {
if (!ensuredDirs.has(dir)) {
await mkdir(dir, { recursive: true });
ensuredDirs.add(dir);
}
await appendFile(target, line, "utf-8");
} catch (err) {
// Warn once per target to surface disk-full / permission errors
if (!warnedTargets.has(target)) {
warnedTargets.add(target);
const msg = err instanceof Error ? err.message : String(err);
// eslint-disable-next-line no-console -- surface trace write failures once
console.warn(`[gh-trace] Failed to write trace to ${target}: ${msg}`);
}
}
}
/** Redact sensitive values from gh CLI args before persisting to trace JSONL. */
function redactArgs(args: string[]): string[] {
const sensitiveFlags = new Set(["-H", "--header"]);
const sensitiveFieldPrefixes = ["token=", "password=", "secret=", "authorization="];
return args.map((arg, i) => {
// Redact the value after -H / --header if it contains Authorization
const prev = i > 0 ? args[i - 1] : undefined;
if (prev && sensitiveFlags.has(prev) && /^authorization:/i.test(arg)) {
return "Authorization: [REDACTED]";
}
// Redact inline -H"Authorization: ..." style
if (/^-H/i.test(arg) && /authorization:/i.test(arg)) {
return "-HAuthorization: [REDACTED]";
}
// Redact -f/-F field values like token=..., password=...
for (const prefix of sensitiveFieldPrefixes) {
if (arg.toLowerCase().startsWith(prefix)) {
return `${arg.slice(0, prefix.length)}[REDACTED]`;
}
}
// Redact the value following -f/-F if the next positional matches
if (prev && (prev === "-f" || prev === "--raw-field" || prev === "-F" || prev === "--field")) {
for (const prefix of sensitiveFieldPrefixes) {
if (arg.toLowerCase().startsWith(prefix)) {
return `${arg.slice(0, prefix.length)}[REDACTED]`;
}
}
}
return arg;
});
}
function buildTraceEntry(
args: string[],
ctx: GhTraceContext,
result: GhTraceResult,
durationMs: number,
): GhTraceEntry {
const { statusLine, headers } = parseIncludedHttpResponse(result.stdout ?? "");
const httpStatus = statusLine
? Number.parseInt(statusLine.replace(/^HTTP\/[0-9.]+\s+/, "").split(" ")[0] ?? "", 10)
: undefined;
return {
timestamp: nowIso(),
component: ctx.component,
operation: ctx.operation ?? extractOperation(args),
projectId: ctx.projectId,
sessionId: ctx.sessionId,
cwd: ctx.cwd,
args: redactArgs(args),
endpoint: extractEndpoint(args),
method: extractMethod(args),
ok: result.ok,
exitCode: result.exitCode,
signal: result.signal,
durationMs,
stdoutBytes: Buffer.byteLength(result.stdout ?? "", "utf-8"),
stderrBytes: Buffer.byteLength(result.stderr ?? "", "utf-8"),
statusLine,
httpStatus: Number.isFinite(httpStatus) ? httpStatus : undefined,
etag: headers["etag"],
rateLimitLimit: parseIntHeader(headers["x-ratelimit-limit"]),
rateLimitRemaining: parseIntHeader(headers["x-ratelimit-remaining"]),
rateLimitReset: parseIntHeader(headers["x-ratelimit-reset"]),
rateLimitResource: headers["x-ratelimit-resource"],
...parseGraphQLRateLimit(result.stdout ?? "", args),
};
}
/** Extract rateLimit { cost, remaining, resetAt } from GraphQL response body. */
function parseGraphQLRateLimit(
stdout: string,
args: string[],
): Pick<GhTraceEntry, "graphqlCost" | "graphqlRemaining" | "graphqlResetAt"> {
// Only attempt for GraphQL calls
if (!args.includes("graphql")) return {};
// Quick check — avoid parsing multi-MB response bodies when rateLimit isn't present
if (!stdout.includes('"rateLimit"')) return {};
// Strip HTTP headers if present (-i flag)
const bodyMatch = stdout.match(/\r?\n\r?\n([\s\S]*)$/);
const body = bodyMatch ? bodyMatch[1] : stdout;
try {
const parsed = JSON.parse(body.trim());
const rl = parsed?.data?.rateLimit;
if (!rl) return {};
return {
graphqlCost: typeof rl.cost === "number" ? rl.cost : undefined,
graphqlRemaining: typeof rl.remaining === "number" ? rl.remaining : undefined,
graphqlResetAt: typeof rl.resetAt === "string" ? rl.resetAt : undefined,
};
} catch {
return {};
}
}
export async function execGhObserved(
args: string[],
ctx: GhTraceContext,
timeout: number = 30_000,
): Promise<string> {
const startedAt = Date.now();
try {
const ghPath = await getGhBinaryPath();
const { stdout, stderr } = await execFileAsync(ghPath, args, {
...(ctx.cwd ? { cwd: ctx.cwd } : {}),
// 10 MB — matches the previous per-caller maxBuffer in scm-github.
// GraphQL batch queries for 25 PRs can produce multi-MB responses.
maxBuffer: 10 * 1024 * 1024,
timeout,
});
const entry = buildTraceEntry(
args,
ctx,
{ ok: true, stdout, stderr },
Date.now() - startedAt,
);
await writeTrace(entry);
return stdout.trim();
} catch (err) {
const stdout = typeof (err as { stdout?: unknown }).stdout === "string"
? (err as { stdout: string }).stdout
: "";
const stderr = typeof (err as { stderr?: unknown }).stderr === "string"
? (err as { stderr: string }).stderr
: "";
const entry = buildTraceEntry(
args,
ctx,
{
ok: false,
stdout,
stderr,
exitCode: extractExitCode(err),
signal: extractSignal(err),
},
Date.now() - startedAt,
);
await writeTrace(entry);
throw err;
}
}
export function getGhTraceFilePath(): string | undefined {
return process.env[GH_TRACE_FILE_ENV];
}
// Re-export internal utilities for testing — not part of public API.
// These are the functions the reviewer flagged as needing test coverage.
export const _testUtils = {
extractOperation,
redactArgs,
parseIncludedHttpResponse,
};

2
packages/core/src/index.ts
View File

@ -193,6 +193,7 @@ export {
createProjectObserver,
readObservabilitySummary,
} from "./observability.js";
export { execGhObserved, getGhTraceFilePath } from "./gh-trace.js";
export { resolveNotifierTarget } from "./notifier-resolution.js";
export type {
ObservabilityLevel,
@ -201,6 +202,7 @@ export type {
ObservabilitySummary,
ProjectObserver,
} from "./observability.js";
export type { GhTraceContext, GhTraceEntry } from "./gh-trace.js";
// Feedback tools — contracts, validation, and report storage
export {

527
packages/core/src/lifecycle-manager.ts
View File

@ -12,10 +12,8 @@
import { randomUUID } from "node:crypto";
import {
SESSION_STATUS,
ACTIVITY_STATE,
PR_STATE,
CI_STATUS,
SESSION_STATUS,
TERMINAL_STATUSES,
type LifecycleManager,
type OpenCodeSessionManager,
@ -37,9 +35,9 @@ import {
type ProjectConfig as _ProjectConfig,
type PREnrichmentData,
type CICheck,
type ReviewComment,
type ReviewSummary,
} from "./types.js";
import { formatAutomatedCommentsMessage } from "./format-automated-comments.js";
import { DEFAULT_BUGBOT_COMMENTS_MESSAGE } from "./config.js";
import { buildLifecycleMetadataPatch, cloneLifecycle, deriveLegacyStatus } from "./lifecycle-state.js";
import { updateMetadata } from "./metadata.js";
import { getSessionsDir } from "./paths.js";
@ -327,9 +325,12 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
*/
const prEnrichmentCache = new Map<string, PREnrichmentData>();
/** Repos where Guard 1 returned 304 in the current poll — safe to skip detectPR. */
let prListUnchangedRepos = new Set<string>();
/**
* Per-session timestamp of last review backlog API check.
* Used to throttle getPendingComments/getAutomatedComments to at most once per 2 minutes.
* Used to throttle review thread checks to at most once per 2 minutes.
* In-memory only — resets on restart (acceptable since it's a rate-limit hint, not state).
*/
const lastReviewBacklogCheckAt = new Map<SessionId, number>();
@ -344,34 +345,45 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
async function populatePREnrichmentCache(sessions: Session[]): Promise<void> {
// Clear previous cache
prEnrichmentCache.clear();
prListUnchangedRepos = new Set();
// Collect all unique PRs keyed by their owning session's project/plugin.
// Collect all unique PRs and repos keyed by their owning session's project/plugin.
// Repos are collected from ALL sessions (not just ones with PRs) so Guard 1 runs
// for every active repo — enabling detectPR gating even when no PRs exist yet.
const prsByPlugin = new Map<string, Array<NonNullable<Session["pr"]>>>();
const reposByPlugin = new Map<string, Set<string>>();
const seenPRKeys = new Set<string>();
for (const session of sessions) {
if (!session.pr) continue;
const project = config.projects[session.projectId];
if (!project?.scm?.plugin) continue;
const prKey = `${session.pr.owner}/${session.pr.repo}#${session.pr.number}`;
if (seenPRKeys.has(prKey)) continue;
seenPRKeys.add(prKey);
if (!project?.scm?.plugin || !project.repo) continue;
const pluginKey = project.scm.plugin;
if (!prsByPlugin.has(pluginKey)) {
prsByPlugin.set(pluginKey, []);
}
if (!reposByPlugin.has(pluginKey)) {
reposByPlugin.set(pluginKey, new Set());
}
reposByPlugin.get(pluginKey)!.add(project.repo);
if (!session.pr) continue;
const prKey = `${session.pr.owner}/${session.pr.repo}#${session.pr.number}`;
if (seenPRKeys.has(prKey)) continue;
seenPRKeys.add(prKey);
const pluginPRs = prsByPlugin.get(pluginKey);
if (pluginPRs) {
pluginPRs.push(session.pr);
}
}
// Fetch enrichment data for each plugin's PRs
// Fetch enrichment data and run Guard 1 for all active repos
for (const [pluginKey, pluginPRs] of prsByPlugin) {
const scm = registry.get<SCM>("scm", pluginKey);
if (!scm?.enrichSessionsPRBatch) continue;
const pluginRepos = [...(reposByPlugin.get(pluginKey) ?? [])];
const batchStartTime = Date.now();
try {
const enrichmentData = await scm.enrichSessionsPRBatch(
@ -424,7 +436,13 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
},
});
},
reportPRListUnchangedRepos(repos) {
for (const repo of repos) {
prListUnchangedRepos.add(repo);
}
},
},
pluginRepos,
);
// Merge into cache
@ -446,7 +464,89 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
});
}
}
// Discover PRs for sessions that don't have one yet.
// Only run detectPR when Guard 1 returned 200 (repo's PR list changed).
// When Guard 1 returned 304, the repo is in prListUnchangedRepos — no new PRs exist.
for (const session of sessions) {
if (session.pr) continue;
if (!session.branch) continue;
if (session.metadata["prAutoDetect"] === "off") continue;
if (session.metadata["role"] === "orchestrator" || session.id.endsWith("-orchestrator")) continue;
const project = config.projects[session.projectId];
if (!project?.repo || !project.scm?.plugin) continue;
// Skip if Guard 1 confirmed no PR list changes for this repo
if (prListUnchangedRepos.has(project.repo)) continue;
const scm = registry.get<SCM>("scm", project.scm.plugin);
if (!scm?.detectPR) continue;
try {
const detectedPR = await scm.detectPR(session, project);
if (detectedPR) {
session.pr = detectedPR;
const sessionsDir = getSessionsDir(project.storageKey);
updateMetadata(sessionsDir, session.id, { pr: detectedPR.url });
}
} catch (error) {
observer?.recordOperation?.({
metric: "lifecycle_poll",
operation: "scm.detect_pr",
outcome: "failure",
correlationId: createCorrelationId("detect-pr"),
projectId: session.projectId,
sessionId: session.id,
reason: error instanceof Error ? error.message : String(error),
level: "warn",
});
}
}
}
/**
* Persist batch enrichment data to session metadata files.
* The web dashboard reads this instead of calling GitHub API.
*/
function persistPREnrichmentToMetadata(sessions: Session[]): void {
for (const session of sessions) {
if (!session.pr) continue;
const project = config.projects[session.projectId];
if (!project) continue;
const prKey = `${session.pr.owner}/${session.pr.repo}#${session.pr.number}`;
const cached = prEnrichmentCache.get(prKey);
if (!cached) continue;
const blob = JSON.stringify({
state: cached.state,
ciStatus: cached.ciStatus,
reviewDecision: cached.reviewDecision,
mergeable: cached.mergeable,
title: cached.title,
additions: cached.additions,
deletions: cached.deletions,
isDraft: cached.isDraft,
hasConflicts: cached.hasConflicts,
isBehind: cached.isBehind,
blockers: cached.blockers,
ciChecks: cached.ciChecks?.map((c) => ({
name: c.name,
status: c.status,
url: c.url,
})),
enrichedAt: new Date().toISOString(),
});
if (session.metadata["prEnrichment"] === blob) continue;
const sessionsDir = getSessionsDir(project.storageKey);
updateMetadata(sessionsDir, session.id, { prEnrichment: blob });
session.metadata["prEnrichment"] = blob;
}
}
/** Check if idle time exceeds the agent-stuck threshold. */
function isIdleBeyondThreshold(session: Session, idleTimestamp: Date): boolean {
const stuckReaction = getReactionConfigForSession(session, "agent-stuck");
@ -692,39 +792,8 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
return commit(probeDecision);
}
if (
!session.pr &&
scm &&
session.branch &&
session.metadata["prAutoDetect"] !== "off" &&
session.metadata["role"] !== "orchestrator" &&
!session.id.endsWith("-orchestrator")
) {
try {
const detectedPR = await scm.detectPR(session, project);
if (detectedPR) {
session.pr = detectedPR;
lifecycle.pr.state = "open";
lifecycle.pr.reason = "in_progress";
lifecycle.pr.number = detectedPR.number;
lifecycle.pr.url = detectedPR.url;
lifecycle.pr.lastObservedAt = nowIso;
const sessionsDir = getSessionsDir(project.storageKey);
updateMetadata(sessionsDir, session.id, { pr: detectedPR.url });
}
} catch (error) {
observer?.recordOperation?.({
metric: "lifecycle_poll",
operation: "scm.detect_pr",
outcome: "failure",
correlationId: createCorrelationId("lifecycle-poll"),
projectId: session.projectId,
sessionId: session.id,
reason: error instanceof Error ? error.message : String(error),
level: "warn",
});
}
}
// detectPR is handled in populatePREnrichmentCache (gated by Guard 1 ETag).
// By this point, session.pr is already set if a PR was discovered.
if (session.pr && scm) {
try {
@ -747,52 +816,29 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
}),
);
}
const prState = await scm.getPRState(session.pr);
if (prState === PR_STATE.MERGED || prState === PR_STATE.CLOSED) {
return commit(
resolvePRLiveDecision({
prState,
ciStatus: CI_STATUS.NONE,
reviewDecision: "none",
mergeable: false,
shouldEscalateIdleToStuck,
idleWasBlocked,
activityEvidence,
}),
);
}
const ciStatus = await scm.getCISummary(session.pr);
if (ciStatus === CI_STATUS.FAILING) {
return commit(
resolvePRLiveDecision({
prState,
ciStatus,
reviewDecision: "none",
mergeable: false,
shouldEscalateIdleToStuck,
idleWasBlocked,
activityEvidence,
}),
);
// Batch enrichment cache miss — fall back to getPRState for terminal
// states (merged/closed) only. Detecting these promptly prevents
// delayed cleanup. Non-terminal state updates wait for the next batch
// cycle (30s) to avoid ~110 individual REST calls per 15-min window.
try {
const prState = await scm.getPRState(session.pr);
if (prState === "merged" || prState === "closed") {
return commit(
resolvePRLiveDecision({
prState,
ciStatus: "none",
reviewDecision: "none",
mergeable: false,
shouldEscalateIdleToStuck,
idleWasBlocked,
activityEvidence,
}),
);
}
} catch {
// Best-effort — batch will retry next cycle
}
const reviewDecision = await scm.getReviewDecision(session.pr);
const mergeReady =
reviewDecision === "approved" || reviewDecision === "none"
? await scm.getMergeability(session.pr)
: { mergeable: false };
return commit(
resolvePRLiveDecision({
prState,
ciStatus,
reviewDecision,
mergeable: mergeReady.mergeable,
shouldEscalateIdleToStuck,
idleWasBlocked,
activityEvidence,
}),
);
} catch (error) {
observer?.recordOperation?.({
metric: "lifecycle_poll",
@ -1120,7 +1166,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
// Throttle review backlog API calls to at most once per 2 minutes.
// Comments don't change faster than this in practice, and the SCM calls
// (getPendingComments + getAutomatedComments) consume API quota on every poll.
// (getReviewThreads) consumes API quota on every poll.
//
// Exception: bypass throttle when a transition reaction just fired for a
// review reaction key. The transitionReaction branch records
@ -1139,21 +1185,52 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
}
lastReviewBacklogCheckAt.set(session.id, Date.now());
const [pendingResult, automatedResult] = await Promise.allSettled([
scm.getPendingComments(session.pr),
scm.getAutomatedComments(session.pr),
]);
// Single GraphQL call for all review threads (human + bot) + review summaries.
// Split locally by isBot for separate reaction pipelines.
let allThreads: ReviewComment[] | null = null;
let reviewSummaries: ReviewSummary[] = [];
try {
if (scm.getReviewThreads) {
const result = await scm.getReviewThreads(session.pr);
allThreads = result.threads;
reviewSummaries = result.reviews;
} else {
// Fallback for SCM plugins that don't implement getReviewThreads yet
allThreads = await scm.getPendingComments(session.pr);
}
} catch {
// Failed to fetch — preserve existing metadata
}
// null means "failed to fetch" — preserve existing metadata.
// [] means "confirmed no comments" — safe to clear.
const pendingComments =
pendingResult.status === "fulfilled" && Array.isArray(pendingResult.value)
? pendingResult.value
: null;
const automatedComments =
automatedResult.status === "fulfilled" && Array.isArray(automatedResult.value)
? automatedResult.value
: null;
// Persist review comments + summaries to metadata for dashboard consumption
if (allThreads !== null) {
const unresolved = allThreads.filter((c) => !c.isBot);
const reviewBlob = JSON.stringify({
unresolvedThreads: unresolved.length,
unresolvedComments: unresolved.map((c) => ({
url: c.url,
path: c.path ?? "",
author: c.author,
body: c.body,
})),
reviews: reviewSummaries.map((r) => ({
author: r.author,
state: r.state,
body: r.body,
})),
commentsUpdatedAt: new Date().toISOString(),
});
if (session.metadata["prReviewComments"] !== reviewBlob) {
updateSessionMetadata(session, { prReviewComments: reviewBlob });
}
}
const pendingComments = allThreads
? allThreads.filter((c) => !c.isBot)
: null;
const automatedComments = allThreads
? allThreads.filter((c) => c.isBot)
: null;
// --- Pending (human) review comments ---
// null = SCM fetch failed; skip processing to preserve existing metadata.
@ -1201,11 +1278,15 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
reactionConfig.action &&
(reactionConfig.auto !== false || reactionConfig.action === "notify")
) {
const enrichedConfig = {
...reactionConfig,
message: formatReviewCommentsMessage(pendingComments, "reviewer", reviewSummaries),
};
const result = await executeReaction(
session.id,
session.projectId,
humanReactionKey,
reactionConfig,
enrichedConfig,
);
if (result.success) {
updateSessionMetadata(session, {
@ -1247,26 +1328,15 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
reactionConfig.action &&
(reactionConfig.auto !== false || reactionConfig.action === "notify")
) {
// Inject the detailed comment listing + correct-API guidance into the
// message so the agent doesn't re-fetch with stale or unpaginated calls
// (see #895 — fixes the pagination + stale `gh pr checks` failure modes).
// Only override when the message is the built-in sentinel — a user who
// customized `reactions.bugbot-comments.message` in their YAML gets
// exactly what they wrote, nothing more.
const usingDefaultMessage =
reactionConfig.message === DEFAULT_BUGBOT_COMMENTS_MESSAGE;
const detailedConfig: ReactionConfig =
reactionConfig.action === "send-to-agent" && usingDefaultMessage
? {
...reactionConfig,
message: formatAutomatedCommentsMessage(automatedComments, session.pr),
}
: reactionConfig;
const enrichedConfig = {
...reactionConfig,
message: formatReviewCommentsMessage(automatedComments, "bot"),
};
const result = await executeReaction(
session.id,
session.projectId,
automatedReactionKey,
detailedConfig,
enrichedConfig,
);
if (result.success) {
updateSessionMetadata(session, {
@ -1279,6 +1349,43 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
}
}
/**
* Format review comments into a message with inline data for the agent.
* Includes file, line, author, body, and URL so the agent doesn't need
* to re-fetch via gh api.
*/
function formatReviewCommentsMessage(
comments: ReviewComment[],
source: "reviewer" | "bot",
reviews: ReviewSummary[] = [],
): string {
const lines: string[] = [];
// Prepend review summaries (the body submitted with "Changes requested" / "Approve")
const nonEmptyReviews = reviews.filter((r) => r.body && r.body.trim().length > 0);
if (nonEmptyReviews.length > 0) {
for (const r of nonEmptyReviews) {
lines.push(`Review by @${r.author} (${r.state}):`);
lines.push(`"${r.body.trim()}"`, "");
}
}
const header =
source === "reviewer"
? `The following ${comments.length} unresolved review comment(s) are on your PR (as of just now). You should not need to re-fetch this data unless you need additional context.`
: `The following ${comments.length} automated review comment(s) are on your PR (as of just now). You should not need to re-fetch this data unless you need additional context.`;
lines.push(header, "");
for (let i = 0; i < comments.length; i++) {
const c = comments[i];
const location = c.path ? `${c.path}${c.line ? `:${c.line}` : ""}` : "(general)";
lines.push(`${i + 1}. ${location} (@${c.author}): "${c.body}"`);
if (c.url) lines.push(` ${c.url}`);
if (c.threadId) lines.push(` Thread ID: ${c.threadId}`);
}
lines.push("", "Address each comment, push fixes. Use the thread ID to resolve each thread directly after pushing. You should not need to re-fetch review data unless you need additional context beyond what is provided here.");
return lines.join("\n");
}
/**
* Format CI check failures into a human-readable message for the agent.
* Includes check names, statuses, and links for debugging.
@ -1300,138 +1407,6 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
return lines.join("\n");
}
/**
* Dispatch CI failure details to the agent session when new or changed
* failures are detected. Follows the same fingerprinting/deduplication
* pattern as maybeDispatchReviewBacklog().
*/
async function maybeDispatchCIFailureDetails(
session: Session,
_oldStatus: SessionStatus,
newStatus: SessionStatus,
transitionReaction?: { key: string; result: ReactionResult | null },
): Promise<void> {
const project = config.projects[session.projectId];
if (!project || !session.pr) return;
const scm = project.scm?.plugin ? registry.get<SCM>("scm", project.scm.plugin) : null;
if (!scm) return;
const ciReactionKey = "ci-failed";
// Clear tracking when PR is closed/merged
if (newStatus === "merged" || newStatus === "killed") {
clearReactionTracker(session.id, ciReactionKey);
updateSessionMetadata(session, {
lastCIFailureFingerprint: "",
lastCIFailureDispatchHash: "",
lastCIFailureDispatchAt: "",
});
return;
}
// Only dispatch CI details when in ci_failed state
if (newStatus !== "ci_failed") {
// CI is no longer failing — clear tracking so next failure is dispatched fresh
const lastFingerprint = session.metadata["lastCIFailureFingerprint"] ?? "";
if (lastFingerprint) {
clearReactionTracker(session.id, ciReactionKey);
updateSessionMetadata(session, {
lastCIFailureFingerprint: "",
lastCIFailureDispatchHash: "",
lastCIFailureDispatchAt: "",
});
}
return;
}
// Fetch individual CI checks for failure details.
// Use batch enrichment data when available to avoid an extra REST call;
// fall back to getCIChecks() when the batch didn't run this cycle.
const prKey = `${session.pr.owner}/${session.pr.repo}#${session.pr.number}`;
const cachedEnrichment = prEnrichmentCache.get(prKey);
let checks: CICheck[];
if (cachedEnrichment?.ciChecks !== undefined) {
checks = cachedEnrichment.ciChecks;
} else {
try {
checks = await scm.getCIChecks(session.pr);
} catch {
// Failed to fetch checks — skip this cycle
return;
}
}
const failedChecks = checks.filter(
(c) => c.status === "failed" || c.conclusion?.toUpperCase() === "FAILURE",
);
if (failedChecks.length === 0) return;
const ciFingerprint = makeFingerprint(
failedChecks.map((c) => `${c.name}:${c.status}:${c.conclusion ?? ""}`),
);
const lastCIFingerprint = session.metadata["lastCIFailureFingerprint"] ?? "";
const lastCIDispatchHash = session.metadata["lastCIFailureDispatchHash"] ?? "";
// Reset reaction tracker when failure set changes
if (ciFingerprint !== lastCIFingerprint && transitionReaction?.key !== ciReactionKey) {
clearReactionTracker(session.id, ciReactionKey);
}
if (ciFingerprint !== lastCIFingerprint) {
updateSessionMetadata(session, {
lastCIFailureFingerprint: ciFingerprint,
});
}
// If transition already sent a ci-failed reaction with the static message,
// skip this cycle but do NOT record dispatch hash — the next poll will send
// the detailed CI failure info with check names and URLs.
if (
transitionReaction?.key === ciReactionKey &&
transitionReaction.result?.success
) {
return;
}
// Skip if we already dispatched this exact failure set
if (ciFingerprint === lastCIDispatchHash) return;
// Dispatch CI failure details directly via sessionManager.send() rather than
// executeReaction() to avoid consuming the ci-failed reaction's retry budget.
// The transition reaction owns escalation; this is a follow-up info delivery.
const reactionConfig = getReactionConfigForSession(session, ciReactionKey);
if (
reactionConfig &&
reactionConfig.action &&
(reactionConfig.auto !== false || reactionConfig.action === "notify")
) {
const detailedMessage = formatCIFailureMessage(failedChecks);
try {
if (reactionConfig.action === "send-to-agent") {
await sessionManager.send(session.id, detailedMessage);
} else {
// For "notify" action, send to human notifiers instead
const event = createEvent("ci.failing", {
sessionId: session.id,
projectId: session.projectId,
message: detailedMessage,
data: { failedChecks: failedChecks.map((c) => c.name) },
});
await notifyHuman(event, reactionConfig.priority ?? "warning");
}
updateSessionMetadata(session, {
lastCIFailureDispatchHash: ciFingerprint,
lastCIFailureDispatchAt: new Date().toISOString(),
});
} catch {
// Send failed — will retry on next poll cycle
}
}
}
/**
* Dispatch merge conflict notifications to the agent session.
* Conflicts are detected from the PR enrichment cache or getMergeability()
@ -1478,19 +1453,11 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
const prKey = `${session.pr.owner}/${session.pr.repo}#${session.pr.number}`;
const cachedData = prEnrichmentCache.get(prKey);
let hasConflicts: boolean;
if (cachedData) {
// Batch ran — trust its data (undefined means CONFLICTING wasn't set → no conflicts)
hasConflicts = cachedData.hasConflicts ?? false;
} else {
// Batch didn't run this cycle — fall back to individual API call
try {
const mergeReadiness = await scm.getMergeability(session.pr);
hasConflicts = !mergeReadiness.noConflicts;
} catch {
return;
}
if (!cachedData) {
// No batch data — skip this cycle, batch will populate on next cycle (30s)
return;
}
const hasConflicts = cachedData.hasConflicts ?? false;
const lastDispatched = session.metadata["lastMergeConflictDispatched"] ?? "";
@ -1506,9 +1473,11 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
) {
try {
if (reactionConfig.action === "send-to-agent") {
const baseBranch = session.pr.baseBranch ?? "the default branch";
const behindNote = cachedData.isBehind ? ` is behind ${baseBranch} and` : "";
const message =
reactionConfig.message ??
"Your branch has merge conflicts. Rebase on the default branch and resolve them.";
`Your PR branch${behindNote} has merge conflicts with ${baseBranch}. Rebase your branch on ${baseBranch}, resolve the conflicts, and push. You should not need to call gh for merge status unless you need additional context — this information is current.`;
await sessionManager.send(session.id, message);
} else {
const event = createEvent("merge.conflicts", {
@ -1742,7 +1711,19 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
const reactionKey = eventToReactionKey(eventType);
if (reactionKey) {
const reactionConfig = getReactionConfigForSession(session, reactionKey);
let reactionConfig = getReactionConfigForSession(session, reactionKey);
// Enrich CI failure message with actual check details from batch cache
if (reactionKey === "ci-failed" && session.pr && reactionConfig) {
const prKey = `${session.pr.owner}/${session.pr.repo}#${session.pr.number}`;
const cachedData = prEnrichmentCache.get(prKey);
if (cachedData?.ciChecks) {
const failedChecks = cachedData.ciChecks.filter((c) => c.status === "failed");
if (failedChecks.length > 0) {
reactionConfig = { ...reactionConfig, message: formatCIFailureMessage(failedChecks) };
}
}
}
if (reactionConfig && reactionConfig.action) {
// auto: false skips automated agent actions but still allows notifications
@ -1873,7 +1854,6 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
await Promise.allSettled([
maybeDispatchReviewBacklog(session, oldStatus, newStatus, transitionReaction),
maybeDispatchCIFailureDetails(session, oldStatus, newStatus, transitionReaction),
maybeDispatchMergeConflicts(session, newStatus),
]);
@ -1982,6 +1962,10 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
// Poll all sessions concurrently
await Promise.allSettled(sessionsToCheck.map((s) => checkSession(s)));
// Persist batch enrichment data to session metadata files so the
// web dashboard can read it without calling GitHub API.
persistPREnrichmentToMetadata(sessionsToCheck);
// Prune stale entries from states, reactionTrackers, and lastReviewBacklogCheckAt
// for sessions that no longer appear in the session list (e.g., after kill/cleanup)
const currentSessionIds = new Set(sessions.map((s) => s.id));
@ -2088,6 +2072,9 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
async check(sessionId: SessionId): Promise<void> {
const session = await sessionManager.get(sessionId);
if (!session) throw new Error(`Session ${sessionId} not found`);
// Populate batch enrichment cache for this session's PR so
// checkSession can read from cache (no individual REST fallback).
await populatePREnrichmentCache([session]);
await checkSession(session);
},
};

4
packages/core/src/metadata.ts
View File

@ -275,6 +275,10 @@ export function deleteMetadata(dataDir: string, sessionId: SessionId, archive =
}
unlinkSync(path);
// NOTE: .ghcache/<sessionId>/ is intentionally NOT deleted here.
// Cache files are small and useful for post-mortem analysis of wrapper
// cache hit/miss behavior. listMetadata() already ignores hidden dirs.
}
/**

28
packages/core/src/session-manager.ts
View File

@ -80,6 +80,11 @@ import { sessionFromMetadata } from "./utils/session-from-metadata.js";
import { safeJsonParse, validateStatus } from "./utils/validation.js";
import { isGitBranchNameSafe } from "./utils.js";
import { resolveAgentSelection, resolveSessionRole } from "./agent-selection.js";
import {
buildAgentPath,
setupPathWrapperWorkspace,
PREFERRED_GH_PATH,
} from "./agent-workspace-hooks.js";
const execFileAsync = promisify(execFile);
const OPENCODE_DISCOVERY_TIMEOUT_MS = 10_000;
@ -1304,6 +1309,11 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
launchCommand,
environment: {
...environment,
PATH: buildAgentPath(environment["PATH"] ?? process.env["PATH"]),
GH_PATH: PREFERRED_GH_PATH,
...(process.env["AO_AGENT_GH_TRACE"] && {
AO_AGENT_GH_TRACE: process.env["AO_AGENT_GH_TRACE"],
}),
AO_SESSION: sessionId,
AO_DATA_DIR: sessionsDir, // Pass sessions directory (not root dataDir)
AO_SESSION_NAME: sessionId, // User-facing session name
@ -1581,11 +1591,17 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
}
};
// Setup agent hooks for automatic metadata updates
// Setup agent hooks for automatic metadata updates.
// Claude Code uses native PostToolUse hooks for metadata writes — skip
// PATH wrappers to avoid two concurrent writers (wrapper + hook) hitting
// the same metadata file with no locking.
try {
if (plugins.agent.setupWorkspaceHooks) {
await plugins.agent.setupWorkspaceHooks(workspacePath, { dataDir: sessionsDir });
}
if (plugins.agent.name !== "claude-code") {
await setupPathWrapperWorkspace(workspacePath);
}
} catch (err) {
await cleanupWorktreeAndMetadata();
throw err;
@ -1669,6 +1685,11 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
launchCommand,
environment: {
...environment,
PATH: buildAgentPath(environment["PATH"] ?? process.env["PATH"]),
GH_PATH: PREFERRED_GH_PATH,
...(process.env["AO_AGENT_GH_TRACE"] && {
AO_AGENT_GH_TRACE: process.env["AO_AGENT_GH_TRACE"],
}),
AO_SESSION: sessionId,
AO_DATA_DIR: sessionsDir,
AO_SESSION_NAME: sessionId,
@ -2831,6 +2852,11 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
launchCommand,
environment: {
...environment,
PATH: buildAgentPath(environment["PATH"] ?? process.env["PATH"]),
GH_PATH: PREFERRED_GH_PATH,
...(process.env["AO_AGENT_GH_TRACE"] && {
AO_AGENT_GH_TRACE: process.env["AO_AGENT_GH_TRACE"],
}),
AO_SESSION: sessionId,
AO_DATA_DIR: sessionsDir,
AO_SESSION_NAME: sessionId,

34
packages/core/src/types.ts
View File

@ -784,8 +784,18 @@ export interface SCM {
/** Get pending (unresolved) review comments */
getPendingComments(pr: PRInfo): Promise<ReviewComment[]>;
/** Get automated review comments (bots, linters, security scanners) */
getAutomatedComments(pr: PRInfo): Promise<AutomatedComment[]>;
/**
* Get all review threads (human + bot) with isBot flag.
* Single GraphQL call for all review threads (human + bot) with review summaries.
* Returns unresolved threads only.
*
* Optional — plugins that do not implement this method will fall back to
* `getPendingComments()` (which lacks `isBot` classification and review
* summaries). New SCM plugins should prefer implementing this method.
*
* @since 0.6.0 — replaces the removed `getAutomatedComments` method.
*/
getReviewThreads?(pr: PRInfo): Promise<ReviewThreadsResult>;
// --- Merge Readiness ---
@ -805,7 +815,7 @@ export interface SCM {
* @param observer - Optional observer for batch operation metrics
* @returns Map keyed by "${owner}/${repo}#${number}" containing enrichment data
*/
enrichSessionsPRBatch?(prs: PRInfo[], observer?: BatchObserver): Promise<Map<string, PREnrichmentData>>;
enrichSessionsPRBatch?(prs: PRInfo[], observer?: BatchObserver, repos?: string[]): Promise<Map<string, PREnrichmentData>>;
}
/**
@ -859,6 +869,8 @@ export interface BatchObserver {
}): void;
/** Log a message at a specific level */
log(level: ObservabilityLevel, message: string): void;
/** Called after ETag guards with repos where Guard 1 returned 304 (no PR list changes). */
reportPRListUnchangedRepos?(repos: Set<string>): void;
}
// --- PR Types ---
@ -955,6 +967,8 @@ export type ReviewDecision = "approved" | "changes_requested" | "pending" | "non
export interface ReviewComment {
id: string;
/** GraphQL node ID of the review thread (for resolveReviewThread mutation). */
threadId?: string;
author: string;
body: string;
path?: string;
@ -962,6 +976,20 @@ export interface ReviewComment {
isResolved: boolean;
createdAt: Date;
url: string;
/** Whether the comment was authored by a known bot */
isBot?: boolean;
}
export interface ReviewSummary {
author: string;
state: string;
body: string;
submittedAt: Date;
}
export interface ReviewThreadsResult {
threads: ReviewComment[];
reviews: ReviewSummary[];
}
export interface AutomatedComment {

4
packages/integration-tests/src/agent-codex-launch-env.integration.test.ts
View File

@ -43,8 +43,8 @@ describe("agent-codex launch/env wiring (integration)", () => {
expect(env["CODEX_DISABLE_UPDATE_CHECK"]).toBe("1");
});
it("sets GH_PATH to preferred system gh wrapper location", () => {
it("does not set GH_PATH (injected by session-manager for all agents)", () => {
const env = agent.getEnvironment(makeLaunchConfig());
expect(env["GH_PATH"]).toBe("/usr/local/bin/gh");
expect(env["GH_PATH"]).toBeUndefined();
});
});

8
packages/plugins/agent-aider/src/index.test.ts
View File

@ -413,14 +413,14 @@ describe("postLaunchSetup", () => {
describe("getEnvironment PATH", () => {
const agent = create();
it("prepends ~/.ao/bin to PATH", () => {
it("does not set PATH (injected by session-manager)", () => {
const env = agent.getEnvironment(makeLaunchConfig());
expect(env["PATH"]).toMatch(/\.ao\/bin/);
expect(env["PATH"]).toBeUndefined();
});
it("sets GH_PATH", () => {
it("does not set GH_PATH (injected by session-manager)", () => {
const env = agent.getEnvironment(makeLaunchConfig());
expect(env["GH_PATH"]).toBe("/usr/local/bin/gh");
expect(env["GH_PATH"]).toBeUndefined();
});
});

16
packages/plugins/agent-aider/src/index.ts
View File

@ -1,13 +1,10 @@
import {
shellEscape,
normalizeAgentPermissionMode,
buildAgentPath,
setupPathWrapperWorkspace,
readLastActivityEntry,
checkActivityLogState,
getActivityFallbackState,
recordTerminalActivity,
PREFERRED_GH_PATH,
DEFAULT_READY_THRESHOLD_MS,
DEFAULT_ACTIVE_WINDOW_MS,
type Agent,
@ -145,9 +142,7 @@ function createAiderAgent(): Agent {
env["AO_ISSUE_ID"] = config.issueId;
}
// Prepend ~/.ao/bin to PATH so our gh/git wrappers intercept commands.
env["PATH"] = buildAgentPath(process.env["PATH"]);
env["GH_PATH"] = PREFERRED_GH_PATH;
// PATH and GH_PATH are injected by session-manager for all agents.
return env;
},
@ -294,13 +289,12 @@ function createAiderAgent(): Agent {
return null;
},
async setupWorkspaceHooks(workspacePath: string, _config: WorkspaceHooksConfig): Promise<void> {
await setupPathWrapperWorkspace(workspacePath);
async setupWorkspaceHooks(_workspacePath: string, _config: WorkspaceHooksConfig): Promise<void> {
// PATH wrappers are installed by session-manager for all agents.
},
async postLaunchSetup(session: Session): Promise<void> {
if (!session.workspacePath) return;
await setupPathWrapperWorkspace(session.workspacePath);
async postLaunchSetup(_session: Session): Promise<void> {
// PATH wrappers are re-ensured by session-manager.
},
};
}

278
packages/plugins/agent-codex/src/index.test.ts
View File

@ -412,27 +412,9 @@ describe("getEnvironment", () => {
expect(env["AO_ISSUE_ID"]).toBeUndefined();
});
it("prepends ~/.ao/bin to PATH for shell wrappers", () => {
it("does not set PATH (injected by session-manager)", () => {
const env = agent.getEnvironment(makeLaunchConfig());
expect(env["PATH"]).toMatch(/^.*\/\.ao\/bin:/);
});
it("PATH starts with the ao bin dir specifically", () => {
const env = agent.getEnvironment(makeLaunchConfig());
expect(env["PATH"]?.startsWith("/mock/home/.ao/bin:")).toBe(true);
});
it("puts /usr/local/bin before linuxbrew paths", () => {
const originalPath = process.env["PATH"];
process.env["PATH"] = "/home/linuxbrew/.linuxbrew/bin:/usr/local/bin:/usr/bin:/bin";
try {
const env = agent.getEnvironment(makeLaunchConfig());
expect(env["PATH"]).toBe(
"/mock/home/.ao/bin:/usr/local/bin:/home/linuxbrew/.linuxbrew/bin:/usr/bin:/bin",
);
} finally {
process.env["PATH"] = originalPath;
}
expect(env["PATH"]).toBeUndefined();
});
it("sets CODEX_DISABLE_UPDATE_CHECK=1 to suppress interactive update prompts", () => {
@ -440,31 +422,9 @@ describe("getEnvironment", () => {
expect(env["CODEX_DISABLE_UPDATE_CHECK"]).toBe("1");
});
it("sets GH_PATH to preferred wrapper target", () => {
it("does not set GH_PATH (injected by session-manager)", () => {
const env = agent.getEnvironment(makeLaunchConfig());
expect(env["GH_PATH"]).toBe("/usr/local/bin/gh");
});
it("deduplicates ao and /usr/local/bin entries", () => {
const originalPath = process.env["PATH"];
process.env["PATH"] = "/mock/home/.ao/bin:/usr/local/bin:/usr/bin:/usr/local/bin";
try {
const env = agent.getEnvironment(makeLaunchConfig());
expect(env["PATH"]).toBe("/mock/home/.ao/bin:/usr/local/bin:/usr/bin");
} finally {
process.env["PATH"] = originalPath;
}
});
it("falls back to /usr/bin:/bin when process.env.PATH is undefined", () => {
const originalPath = process.env["PATH"];
delete process.env["PATH"];
try {
const env = agent.getEnvironment(makeLaunchConfig());
expect(env["PATH"]).toBe("/mock/home/.ao/bin:/usr/local/bin:/usr/bin:/bin");
} finally {
process.env["PATH"] = originalPath;
}
expect(env["GH_PATH"]).toBeUndefined();
});
});
@ -1718,8 +1678,10 @@ describe("postLaunchSetup", () => {
mockExecFileAsync.mockRejectedValue(new Error("not found"));
mockStat.mockRejectedValue(new Error("ENOENT"));
mockReadFile.mockRejectedValue(new Error("ENOENT"));
await agent.postLaunchSetup!(makeSession({ workspacePath: "/workspace/test" }));
expect(mockMkdir).toHaveBeenCalled();
// Should not throw — binary resolution runs even if it falls back to "codex"
await expect(
agent.postLaunchSetup!(makeSession({ workspacePath: "/workspace/test" })),
).resolves.toBeUndefined();
});
it("returns early when session has no workspacePath", async () => {
@ -1760,213 +1722,17 @@ describe("setupWorkspaceHooks", () => {
expect(typeof agent.setupWorkspaceHooks).toBe("function");
});
it("creates ~/.ao/bin directory", async () => {
// Version marker doesn't exist — triggers full install
it("is a no-op (PATH wrappers are installed by session-manager)", async () => {
mockReadFile.mockRejectedValue(new Error("ENOENT"));
await agent.setupWorkspaceHooks!("/workspace/test", {
dataDir: "/data",
sessionId: "sess-1",
});
expect(mockMkdir).toHaveBeenCalledWith("/mock/home/.ao/bin", { recursive: true });
});
it("writes ao-metadata-helper.sh with executable permissions via atomic write", async () => {
mockReadFile.mockRejectedValue(new Error("ENOENT"));
await agent.setupWorkspaceHooks!("/workspace/test", {
dataDir: "/data",
sessionId: "sess-1",
});
// Atomic write: writes to .tmp file first, then renames
const helperWriteCall = mockWriteFile.mock.calls.find(
(call: [string, string, object]) =>
typeof call[0] === "string" && call[0].includes("ao-metadata-helper.sh.tmp."),
);
expect(helperWriteCall).toBeDefined();
expect(helperWriteCall![1]).toContain("update_ao_metadata()");
expect(helperWriteCall![2]).toEqual({ encoding: "utf-8", mode: 0o755 });
// Then renamed to final path
const helperRenameCall = mockRename.mock.calls.find(
(call: string[]) => typeof call[1] === "string" && call[1].endsWith("ao-metadata-helper.sh"),
);
expect(helperRenameCall).toBeDefined();
});
it("writes gh and git wrappers atomically when version marker is missing", async () => {
mockReadFile.mockRejectedValue(new Error("ENOENT"));
await agent.setupWorkspaceHooks!("/workspace/test", {
dataDir: "/data",
sessionId: "sess-1",
});
// gh wrapper: written to temp, then renamed
const ghWriteCall = mockWriteFile.mock.calls.find(
(call: [string, string, object]) =>
typeof call[0] === "string" && call[0].includes("/gh.tmp."),
);
expect(ghWriteCall).toBeDefined();
expect(ghWriteCall![1]).toContain("ao gh wrapper");
const ghRenameCall = mockRename.mock.calls.find(
(call: string[]) => typeof call[1] === "string" && call[1].endsWith("/gh"),
);
expect(ghRenameCall).toBeDefined();
// git wrapper: written to temp, then renamed
const gitWriteCall = mockWriteFile.mock.calls.find(
(call: [string, string, object]) =>
typeof call[0] === "string" && call[0].includes("/git.tmp."),
);
expect(gitWriteCall).toBeDefined();
expect(gitWriteCall![1]).toContain("ao git wrapper");
const gitRenameCall = mockRename.mock.calls.find(
(call: string[]) => typeof call[1] === "string" && call[1].endsWith("/git"),
);
expect(gitRenameCall).toBeDefined();
});
it("sets executable permissions on gh and git wrappers via writeFile mode", async () => {
mockReadFile.mockRejectedValue(new Error("ENOENT"));
await agent.setupWorkspaceHooks!("/workspace/test", {
dataDir: "/data",
sessionId: "sess-1",
});
const ghWriteCall = mockWriteFile.mock.calls.find(
(call: [string, string, object]) =>
typeof call[0] === "string" && call[0].includes("/gh.tmp."),
);
expect(ghWriteCall![2]).toEqual({ encoding: "utf-8", mode: 0o755 });
const gitWriteCall = mockWriteFile.mock.calls.find(
(call: [string, string, object]) =>
typeof call[0] === "string" && call[0].includes("/git.tmp."),
);
expect(gitWriteCall![2]).toEqual({ encoding: "utf-8", mode: 0o755 });
});
it("skips wrapper writes when version marker matches", async () => {
// First call for version marker — matches current version
// Second call for AGENTS.md — file doesn't exist
mockReadFile.mockImplementation((path: string) => {
if (typeof path === "string" && path.endsWith(".ao-version")) {
return Promise.resolve("0.3.0");
}
// AGENTS.md read attempt
return Promise.reject(new Error("ENOENT"));
});
await agent.setupWorkspaceHooks!("/workspace/test", {
dataDir: "/data",
sessionId: "sess-1",
});
// Should NOT write any wrappers when version matches (helper, gh, git all skipped)
const wrapperWrites = mockWriteFile.mock.calls.filter(
(call: [string, string, object]) =>
typeof call[0] === "string" &&
(call[0].includes("ao-metadata-helper.sh.tmp.") ||
call[0].includes("/gh.tmp.") ||
call[0].includes("/git.tmp.")),
);
expect(wrapperWrites).toHaveLength(0);
});
it("writes version marker after installing wrappers", async () => {
mockReadFile.mockRejectedValue(new Error("ENOENT"));
await agent.setupWorkspaceHooks!("/workspace/test", {
dataDir: "/data",
sessionId: "sess-1",
});
// Version marker is also atomically written
const versionWriteCall = mockWriteFile.mock.calls.find(
(call: [string, string, object]) =>
typeof call[0] === "string" && call[0].includes(".ao-version.tmp."),
);
expect(versionWriteCall).toBeDefined();
expect(versionWriteCall![1]).toBe("0.3.0");
const versionRenameCall = mockRename.mock.calls.find(
(call: string[]) => typeof call[1] === "string" && call[1].endsWith(".ao-version"),
);
expect(versionRenameCall).toBeDefined();
});
it("writes ao session context to .ao/AGENTS.md", async () => {
// Version marker matches (skip wrapper install)
mockReadFile.mockImplementation((path: string) => {
if (typeof path === "string" && path.endsWith(".ao-version")) {
return Promise.resolve("0.3.0");
}
return Promise.reject(new Error("ENOENT"));
});
await agent.setupWorkspaceHooks!("/workspace/test", {
dataDir: "/data",
sessionId: "sess-1",
});
const agentsMdCall = mockWriteFile.mock.calls.find(
(call: string[]) => typeof call[0] === "string" && call[0].includes(".ao/AGENTS.md"),
);
expect(agentsMdCall).toBeDefined();
expect(agentsMdCall![1]).toContain("Agent Orchestrator (ao) Session");
});
it("uses atomic write (temp + rename) to prevent partial reads from concurrent sessions", async () => {
mockReadFile.mockRejectedValue(new Error("ENOENT"));
await agent.setupWorkspaceHooks!("/workspace/test", {
dataDir: "/data",
sessionId: "sess-1",
});
// Every wrapper file should be written to a .tmp file first, then renamed
// This ensures concurrent readers never see a partially written file
const tmpWrites = mockWriteFile.mock.calls.filter(
(call: [string, string, object]) => typeof call[0] === "string" && call[0].includes(".tmp."),
);
const renames = mockRename.mock.calls;
// We expect atomic writes for: helper, gh, git, version marker = 4
expect(tmpWrites.length).toBe(4);
expect(renames.length).toBe(4);
// Each rename should move a .tmp file to the final path
for (const [src, dst] of renames) {
expect(src).toContain(".tmp.");
expect(dst).not.toContain(".tmp.");
}
});
it("writes .ao/AGENTS.md without modifying repo-tracked AGENTS.md", async () => {
mockReadFile.mockImplementation((path: string) => {
if (typeof path === "string" && path.endsWith(".ao-version")) {
return Promise.resolve("0.3.0");
}
return Promise.reject(new Error("ENOENT"));
});
await agent.setupWorkspaceHooks!("/workspace/test", {
dataDir: "/data",
sessionId: "sess-1",
});
// Should write to .ao/AGENTS.md, NOT to workspace root AGENTS.md
const allWrites = mockWriteFile.mock.calls.filter(
(call: string[]) => typeof call[0] === "string" && call[0].endsWith("AGENTS.md"),
);
expect(allWrites).toHaveLength(1);
expect(allWrites[0]![0]).toContain(".ao/AGENTS.md");
// Plugin no longer writes wrappers — session-manager handles it.
// mkdir/writeFile/rename should not be called by the plugin.
expect(mockMkdir).not.toHaveBeenCalled();
expect(mockWriteFile).not.toHaveBeenCalled();
expect(mockRename).not.toHaveBeenCalled();
});
});
@ -1974,22 +1740,20 @@ describe("setupWorkspaceHooks", () => {
// Shell wrapper content verification
// =========================================================================
describe("shell wrapper content", () => {
const agent = create();
beforeEach(() => {
// Force wrapper installation by making version marker miss
mockReadFile.mockRejectedValue(new Error("ENOENT"));
});
async function getWrapperContent(name: string): Promise<string> {
await agent.setupWorkspaceHooks!("/workspace/test", {
dataDir: "/data",
sessionId: "sess-1",
});
// Wrappers are now installed by session-manager via setupPathWrapperWorkspace.
// Import and call it directly to test wrapper content.
const { setupPathWrapperWorkspace } = await import("@aoagents/ao-core");
await setupPathWrapperWorkspace("/workspace/test");
// With atomic writes, content is written to a .tmp. file
const call = mockWriteFile.mock.calls.find(
(c: [string, string, object]) => typeof c[0] === "string" && c[0].includes(`/${name}.tmp.`),
(c: unknown[]) => typeof c[0] === "string" && (c[0] as string).includes(`/${name}.tmp.`),
);
return call ? (call[1] as string) : "";
}
@ -2055,9 +1819,9 @@ describe("shell wrapper content", () => {
expect(content).toContain("pr/create)");
});
it("uses exec for non-PR commands (transparent passthrough)", async () => {
it("passes through non-PR commands to real gh", async () => {
const content = await getWrapperContent("gh");
expect(content).toContain('exec "$real_gh"');
expect(content).toContain('"$real_gh" "$@"');
});
it("prefers GH_PATH when provided and executable", async () => {

18
packages/plugins/agent-codex/src/index.ts
View File

@ -4,13 +4,10 @@ import {
shellEscape,
readLastJsonlEntry,
normalizeAgentPermissionMode,
buildAgentPath,
setupPathWrapperWorkspace,
readLastActivityEntry,
checkActivityLogState,
getActivityFallbackState,
recordTerminalActivity,
PREFERRED_GH_PATH,
type Agent,
type AgentSessionInfo,
type AgentLaunchConfig,
@ -487,11 +484,7 @@ function createCodexAgent(): Agent {
env["AO_ISSUE_ID"] = config.issueId;
}
// Prepend ~/.ao/bin to PATH so our gh/git wrappers intercept commands.
// The wrappers strip this directory from PATH before calling the real
// binary, so there's no infinite recursion.
env["PATH"] = buildAgentPath(process.env["PATH"]);
env["GH_PATH"] = PREFERRED_GH_PATH;
// PATH and GH_PATH are injected by session-manager for all agents.
// Disable Codex's version check/update prompt for non-interactive AO sessions.
env["CODEX_DISABLE_UPDATE_CHECK"] = "1";
@ -743,11 +736,11 @@ function createCodexAgent(): Agent {
return parts.join(" ");
},
async setupWorkspaceHooks(workspacePath: string, _config: WorkspaceHooksConfig): Promise<void> {
await setupPathWrapperWorkspace(workspacePath);
async setupWorkspaceHooks(_workspacePath: string, _config: WorkspaceHooksConfig): Promise<void> {
// PATH wrappers are installed by session-manager for all agents.
},
async postLaunchSetup(session: Session): Promise<void> {
async postLaunchSetup(_session: Session): Promise<void> {
// Resolve binary path on first launch (cached for subsequent calls).
// Uses a promise guard to prevent concurrent calls from racing.
if (!resolvedBinary) {
@ -760,8 +753,7 @@ function createCodexAgent(): Agent {
resolvingBinary = null;
}
}
if (!session.workspacePath) return;
await setupPathWrapperWorkspace(session.workspacePath);
// PATH wrappers are re-ensured by session-manager.
},
};
}

8
packages/plugins/agent-cursor/src/index.test.ts
View File

@ -508,14 +508,14 @@ describe("postLaunchSetup", () => {
describe("getEnvironment PATH", () => {
const agent = create();
it("prepends ~/.ao/bin to PATH", () => {
it("does not set PATH (injected by session-manager)", () => {
const env = agent.getEnvironment(makeLaunchConfig());
expect(env["PATH"]).toMatch(/\.ao\/bin/);
expect(env["PATH"]).toBeUndefined();
});
it("sets GH_PATH", () => {
it("does not set GH_PATH (injected by session-manager)", () => {
const env = agent.getEnvironment(makeLaunchConfig());
expect(env["GH_PATH"]).toBe("/usr/local/bin/gh");
expect(env["GH_PATH"]).toBeUndefined();
});
});

16
packages/plugins/agent-cursor/src/index.ts
View File

@ -1,13 +1,10 @@
import {
shellEscape,
normalizeAgentPermissionMode,
buildAgentPath,
setupPathWrapperWorkspace,
readLastActivityEntry,
checkActivityLogState,
getActivityFallbackState,
recordTerminalActivity,
PREFERRED_GH_PATH,
DEFAULT_READY_THRESHOLD_MS,
DEFAULT_ACTIVE_WINDOW_MS,
type Agent,
@ -229,9 +226,7 @@ function createCursorAgent(): Agent {
env["AO_ISSUE_ID"] = config.issueId;
}
// Prepend ~/.ao/bin to PATH so our gh/git wrappers intercept commands.
env["PATH"] = buildAgentPath(process.env["PATH"]);
env["GH_PATH"] = PREFERRED_GH_PATH;
// PATH and GH_PATH are injected by session-manager for all agents.
return env;
},
@ -394,13 +389,12 @@ function createCursorAgent(): Agent {
return null;
},
async setupWorkspaceHooks(workspacePath: string, _config: WorkspaceHooksConfig): Promise<void> {
await setupPathWrapperWorkspace(workspacePath);
async setupWorkspaceHooks(_workspacePath: string, _config: WorkspaceHooksConfig): Promise<void> {
// PATH wrappers are installed by session-manager for all agents.
},
async postLaunchSetup(session: Session): Promise<void> {
if (!session.workspacePath) return;
await setupPathWrapperWorkspace(session.workspacePath);
async postLaunchSetup(_session: Session): Promise<void> {
// PATH wrappers are re-ensured by session-manager.
},
};
}

8
packages/plugins/agent-opencode/src/index.test.ts
View File

@ -818,14 +818,14 @@ describe("postLaunchSetup", () => {
describe("getEnvironment PATH", () => {
const agent = create();
it("prepends ~/.ao/bin to PATH", () => {
it("does not set PATH (injected by session-manager)", () => {
const env = agent.getEnvironment(makeLaunchConfig());
expect(env["PATH"]).toMatch(/\.ao\/bin/);
expect(env["PATH"]).toBeUndefined();
});
it("sets GH_PATH", () => {
it("does not set GH_PATH (injected by session-manager)", () => {
const env = agent.getEnvironment(makeLaunchConfig());
expect(env["GH_PATH"]).toBe("/usr/local/bin/gh");
expect(env["GH_PATH"]).toBeUndefined();
});
});

16
packages/plugins/agent-opencode/src/index.ts
View File

@ -2,13 +2,10 @@ import {
DEFAULT_READY_THRESHOLD_MS,
DEFAULT_ACTIVE_WINDOW_MS,
shellEscape,
buildAgentPath,
readLastActivityEntry,
checkActivityLogState,
getActivityFallbackState,
recordTerminalActivity,
setupPathWrapperWorkspace,
PREFERRED_GH_PATH,
asValidOpenCodeSessionId,
type Agent,
type AgentSessionInfo,
@ -276,9 +273,7 @@ function createOpenCodeAgent(): Agent {
env["AO_ISSUE_ID"] = config.issueId;
}
// Prepend ~/.ao/bin to PATH so our gh/git wrappers intercept commands.
env["PATH"] = buildAgentPath(process.env["PATH"]);
env["GH_PATH"] = PREFERRED_GH_PATH;
// PATH and GH_PATH are injected by session-manager for all agents.
return env;
},
@ -437,13 +432,12 @@ function createOpenCodeAgent(): Agent {
return parts.join(" ");
},
async setupWorkspaceHooks(workspacePath: string, _config: WorkspaceHooksConfig): Promise<void> {
await setupPathWrapperWorkspace(workspacePath);
async setupWorkspaceHooks(_workspacePath: string, _config: WorkspaceHooksConfig): Promise<void> {
// PATH wrappers are installed by session-manager for all agents.
},
async postLaunchSetup(session: Session): Promise<void> {
if (!session.workspacePath) return;
await setupPathWrapperWorkspace(session.workspacePath);
async postLaunchSetup(_session: Session): Promise<void> {
// PATH wrappers are re-ensured by session-manager.
},
};
}

18
packages/plugins/runtime-tmux/src/index.ts
View File

@ -66,17 +66,29 @@ export function create(): Runtime {
// Create tmux session in detached mode
await tmux("new-session", "-d", "-s", sessionName, "-c", config.workspacePath, ...envArgs);
// Re-export PATH inside the launch script. macOS zsh runs path_helper
// during shell startup which resets PATH, wiping entries set via tmux -e.
// Including the export in the script file (not send-keys) avoids terminal
// buffer issues with long PATH values (1000+ chars).
const pathValue = config.environment?.["PATH"];
let launchCommand = config.launchCommand;
if (pathValue) {
// Use printf with JSON-escaped value to avoid shell injection if
// PATH contains single quotes or other shell metacharacters.
launchCommand = `export PATH=$(printf '%s' ${JSON.stringify(pathValue)})\n${launchCommand}`;
}
// Send the launch command — clean up the session if this fails.
// Use a temp script for long commands so the pane shows a short
// invocation instead of a pasted wall of shell.
try {
if (config.launchCommand.length > 200) {
const invocation = writeLaunchScript(config.launchCommand);
if (launchCommand.length > 200) {
const invocation = writeLaunchScript(launchCommand);
await tmux("send-keys", "-t", sessionName, "-l", invocation);
await sleep(300);
await tmux("send-keys", "-t", sessionName, "Enter");
} else {
await tmux("send-keys", "-t", sessionName, config.launchCommand, "Enter");
await tmux("send-keys", "-t", sessionName, launchCommand, "Enter");
}
} catch (err: unknown) {
try {

281
packages/plugins/scm-github/src/graphql-batch.ts
View File

@ -7,25 +7,49 @@
import { execFile } from "node:child_process";
import { promisify } from "node:util";
import type {
BatchObserver,
CICheck,
CIStatus,
PREnrichmentData,
PRInfo,
PRState,
ReviewDecision,
import {
execGhObserved,
type BatchObserver,
type CICheck,
type CIStatus,
type PREnrichmentData,
type PRInfo,
type PRState,
type ReviewDecision,
} from "@aoagents/ao-core";
import { LRUCache } from "./lru-cache.js";
let execFileAsync = promisify(execFile);
let execGhAsync = async (args: string[], timeout: number, operation: string): Promise<string> =>
execGhObserved(args, { component: "scm-github-batch", operation }, timeout);
/**
* Set execFileAsync for testing.
* Allows mocking the underlying execFile in unit tests.
*
* NOTE: This bypasses the gh tracer (execGhObserved). Tests that need to
* verify tracer behavior should mock execGhObserved directly or use
* setExecGhAsync instead.
*/
export function setExecFileAsync(fn: typeof execFileAsync): void {
execFileAsync = fn;
execGhAsync = async (args: string[], timeout: number): Promise<string> => {
const { stdout } = await execFileAsync("gh", args, {
maxBuffer: 10 * 1024 * 1024,
timeout,
});
return stdout.trim();
};
}
/**
* Set execGhAsync for testing — preserves tracer in the call chain.
* Use this when testing code that should exercise the traced execution path.
*/
export function setExecGhAsync(
fn: (args: string[], timeout: number, operation: string) => Promise<string>,
): void {
execGhAsync = fn;
}
/**
@ -34,6 +58,7 @@ export function setExecFileAsync(fn: typeof execFileAsync): void {
*/
const MAX_PR_LIST_ETAGS = 100; // Number of repos to cache
const MAX_COMMIT_STATUS_ETAGS = 500; // Number of commits to cache
const MAX_REVIEW_COMMENTS_ETAGS = 500; // Number of PRs to cache review ETags
const MAX_PR_METADATA = 200; // Number of PRs to cache full data
/**
@ -47,6 +72,7 @@ const MAX_PR_METADATA = 200; // Number of PRs to cache full data
interface ETagCache {
prList: LRUCache<string, string>; // Key: "owner/repo", Value: ETag
commitStatus: LRUCache<string, string>; // Key: "owner/repo#sha", Value: ETag
reviewComments: LRUCache<string, string>; // Key: "owner/repo#number", Value: ETag
}
/**
@ -59,6 +85,7 @@ interface ETagCache {
const etagCache: ETagCache = {
prList: new LRUCache(MAX_PR_LIST_ETAGS),
commitStatus: new LRUCache(MAX_COMMIT_STATUS_ETAGS),
reviewComments: new LRUCache(MAX_REVIEW_COMMENTS_ETAGS),
};
/**
@ -67,6 +94,15 @@ const etagCache: ETagCache = {
interface ETagGuardResult {
shouldRefresh: boolean;
details: string[];
/** Repos where Guard 1 returned 304 — no PR list changes, detectPR can be skipped. */
prListUnchangedRepos: Set<string>;
}
/** Result of enrichSessionsPRBatch including Guard 1 PR-discovery info. */
export interface BatchEnrichmentResult {
enrichment: Map<string, PREnrichmentData>;
/** Repos where Guard 1 returned 304 — safe to skip detectPR. */
prListUnchangedRepos: Set<string>;
}
/**
@ -76,6 +112,7 @@ interface ETagGuardResult {
export function clearETagCache(): void {
etagCache.prList.clear();
etagCache.commitStatus.clear();
etagCache.reviewComments.clear();
}
/**
@ -176,14 +213,11 @@ function updatePRMetadataCache(
*/
export async function shouldRefreshPREnrichment(
prs: PRInfo[],
extraRepos: string[] = [],
): Promise<ETagGuardResult> {
const details: string[] = [];
let shouldRefresh = false;
if (prs.length === 0) {
return { shouldRefresh: false, details: ["No PRs to check"] };
}
// Group PRs by repository for Guard 1 (PR list check)
const repos = new Map<string, PRInfo[]>();
@ -198,8 +232,20 @@ export async function shouldRefreshPREnrichment(
}
}
// Include repos from PR-less sessions so Guard 1 runs for them too
for (const repoKey of extraRepos) {
if (!repos.has(repoKey)) {
repos.set(repoKey, []);
}
}
if (repos.size === 0) {
return { shouldRefresh: false, details: ["No repos to check"], prListUnchangedRepos: new Set() };
}
// Guard 1: Check PR list ETag for each repository
let guard1DetectedChanges = false;
const prListUnchangedRepos = new Set<string>();
for (const [repoKey] of repos) {
const [owner, repo] = repoKey.split("/");
const prListChanged = await checkPRListETag(owner, repo);
@ -207,6 +253,8 @@ export async function shouldRefreshPREnrichment(
guard1DetectedChanges = true;
shouldRefresh = true;
details.push(`PR list changed for ${repoKey} (Guard 1)`);
} else {
prListUnchangedRepos.add(repoKey);
}
}
@ -254,7 +302,7 @@ export async function shouldRefreshPREnrichment(
}
}
return { shouldRefresh, details };
return { shouldRefresh, details, prListUnchangedRepos };
}
/**
@ -322,6 +370,36 @@ async function verifyGhCLI(): Promise<void> {
*/
export const MAX_BATCH_SIZE = 25;
/**
* Check if an HTTP response contains a 304 Not Modified status.
* Handles HTTP/1.1, HTTP/2, and HTTP/2.0 status lines.
*/
function is304(output: string): boolean {
return /HTTP\/[\d.]+ 304/i.test(output);
}
/**
* Extract stdout/stderr from an execFile error object.
* gh cli puts the HTTP response in stdout even on exit code 1 (e.g. 304).
*/
function extractErrorOutput(err: unknown): string | null {
const e = err as { stdout?: unknown; stderr?: unknown };
const stdout = typeof e.stdout === "string" ? e.stdout : "";
const stderr = typeof e.stderr === "string" ? e.stderr : "";
const combined = stdout + stderr;
return combined.length > 0 ? combined : null;
}
/**
* Extract ETag from HTTP response output.
* Used on both 200 and 304 paths — RFC 7232 allows servers to rotate
* the validator on a 304, so we must re-read the ETag even when unchanged.
*/
function extractETag(output: string): string | undefined {
const match = output.match(/etag:\s*(.+)/i);
return match ? match[1].trim() : undefined;
}
/**
* Guard 1: PR List ETag Check (per repo)
*
@ -350,30 +428,34 @@ async function checkPRListETag(
}
try {
const { stdout } = await execFileAsync("gh", args, { timeout: 10_000 });
const output = stdout.trim();
const output = await execGhAsync(args, 10_000, "gh.api.guard-pr-list");
// Check for HTTP 304 Not Modified response
if (output.includes("HTTP/1.1 304") || output.includes("HTTP/2 304")) {
// No changes detected - cost: 0 GraphQL points
if (is304(output)) {
// Re-read ETag on 304 — RFC 7232 allows rotated validators
const rotatedETag = extractETag(output);
if (rotatedETag) setPRListETag(owner, repo, rotatedETag);
return false;
}
// Extract new ETag from response headers
// ETag header format: "etag": "W/"abc123..." or "etag": "abc123..."
const etagMatch = output.match(/etag:\s*(.+)/i);
if (etagMatch) {
// Trim to remove trailing whitespace/newlines that could cause comparison issues
const newETag = etagMatch[1].trim();
const newETag = extractETag(output);
if (newETag) {
setPRListETag(owner, repo, newETag);
}
// PR list changed - cost: 1 REST point
return true;
} catch (err) {
// On error, assume change to ensure we don't miss anything
// gh exits code 1 on 304 Not Modified — check stdout/stderr for the status line
const output = extractErrorOutput(err);
if (output && is304(output)) {
const rotatedETag = extractETag(output);
if (rotatedETag) setPRListETag(owner, repo, rotatedETag);
return false;
}
const errorMsg = err instanceof Error ? err.message : String(err);
// Log but don't throw - allow GraphQL batch to proceed
// eslint-disable-next-line no-console -- Observability logging for ETag errors
console.warn(`[ETag Guard 1] PR list check failed for ${repoKey}: ${errorMsg}`);
return true; // Assume changed to be safe
@ -381,13 +463,15 @@ async function checkPRListETag(
}
/**
* Guard 2: Commit Status ETag Check (per PR with pending CI)
* Guard 2: Check-Runs ETag Check (per PR with cached head SHA)
*
* Detects if CI status has changed for a specific commit using REST ETag.
*
* - Endpoint: GET /repos/{owner}/{repo}/commits/{head_sha}/status
* - Endpoint: GET /repos/{owner}/{repo}/commits/{head_sha}/check-runs
* Uses the check-runs endpoint (not legacy /status) because the batch
* query reads `statusCheckRollup` which aggregates check-runs. Pure-Actions
* repos only update check-runs, not the legacy combined-status endpoint.
* - Detects: CI check starts, passes, fails, or external status updates
* - Only checked for PRs with ciStatus === "pending" to minimize calls
*
* @returns true if CI status has changed (200 OK), false if unchanged (304 Not Modified)
*/
@ -399,8 +483,9 @@ async function checkCommitStatusETag(
const commitKey = `${owner}/${repo}#${sha}`;
const cachedETag = etagCache.commitStatus.get(commitKey);
// Build gh CLI args for REST API call
const url = `repos/${owner}/${repo}/commits/${sha}/status`;
// Use check-runs endpoint (not legacy /status) to match statusCheckRollup
// data source. per_page=1 keeps the response small — we only need the ETag.
const url = `repos/${owner}/${repo}/commits/${sha}/check-runs?per_page=1`;
const args = ["api", "--method", "GET", url, "-i"]; // -i includes headers
// Add If-None-Match header if we have a cached ETag
@ -409,27 +494,32 @@ async function checkCommitStatusETag(
}
try {
const { stdout } = await execFileAsync("gh", args, { timeout: 10_000 });
const output = stdout.trim();
const output = await execGhAsync(args, 10_000, "gh.api.guard-commit-status");
// Check for HTTP 304 Not Modified response
if (output.includes("HTTP/1.1 304") || output.includes("HTTP/2 304")) {
// No CI changes detected - cost: 0 GraphQL points
if (is304(output)) {
const rotatedETag = extractETag(output);
if (rotatedETag) setCommitStatusETag(owner, repo, sha, rotatedETag);
return false;
}
// Extract new ETag from response headers
const etagMatch = output.match(/etag:\s*(.+)/i);
if (etagMatch) {
// Trim to remove trailing whitespace/newlines that could cause comparison issues
const newETag = etagMatch[1].trim();
const newETag = extractETag(output);
if (newETag) {
setCommitStatusETag(owner, repo, sha, newETag);
}
// CI status changed - cost: 1 REST point
return true;
} catch (err) {
// On error, assume change to ensure we don't miss anything
// gh exits code 1 on 304 Not Modified — check stdout/stderr for the status line
const output = extractErrorOutput(err);
if (output && is304(output)) {
const rotatedETag = extractETag(output);
if (rotatedETag) setCommitStatusETag(owner, repo, sha, rotatedETag);
return false;
}
const errorMsg = err instanceof Error ? err.message : String(err);
// eslint-disable-next-line no-console -- Observability logging for ETag errors
console.warn(
@ -439,6 +529,66 @@ async function checkCommitStatusETag(
}
}
/**
* Guard 3: Review Comments ETag Check (per PR)
*
* Detects if inline review comments have changed on a PR.
* Used to gate the getReviewThreads GraphQL call — if no new comments
* exist (304), the cached result is reused without a GraphQL call.
*
* - Endpoint: GET /repos/{owner}/{repo}/pulls/{number}/comments
* No per_page limit — the ETag covers the full resource. With per_page=1,
* the ETag only covers the first page, so new comments on page 2+ would
* never bust the validator. Typical PR comment counts are small (<100)
* so the unbounded list is fine.
* - Detects: New review comments, edited comments, deleted comments
* - Cost: 0 REST points on 304, 1 REST point on 200
*/
export async function checkReviewCommentsETag(
owner: string,
repo: string,
prNumber: number,
): Promise<boolean> {
const cacheKey = `${owner}/${repo}#${prNumber}`;
const cachedETag = etagCache.reviewComments.get(cacheKey);
const url = `repos/${owner}/${repo}/pulls/${prNumber}/comments`;
const args = ["api", "--method", "GET", url, "-i"];
if (cachedETag) {
args.push("-H", `If-None-Match: ${cachedETag}`);
}
try {
const output = await execGhAsync(args, 10_000, "gh.api.guard-review-comments");
if (is304(output)) {
const rotatedETag = extractETag(output);
if (rotatedETag) etagCache.reviewComments.set(cacheKey, rotatedETag);
return false;
}
const newETag = extractETag(output);
if (newETag) {
etagCache.reviewComments.set(cacheKey, newETag);
}
return true;
} catch (err) {
const output = extractErrorOutput(err);
if (output && is304(output)) {
const rotatedETag = extractETag(output);
if (rotatedETag) etagCache.reviewComments.set(cacheKey, rotatedETag);
return false;
}
const errorMsg = err instanceof Error ? err.message : String(err);
// eslint-disable-next-line no-console -- Observability logging for ETag errors
console.warn(`[ETag Guard 3] Review comments check failed for ${cacheKey}: ${errorMsg}`);
return true; // Assume changed to be safe
}
}
/**
* GraphQL fields to fetch for each PR.
* This includes all data needed for orchestrator status detection.
@ -455,19 +605,16 @@ const PR_FIELDS = `
reviewDecision
headRefName
headRefOid
reviews(last: 5) {
nodes {
author { login }
state
submittedAt
}
}
commits(last: 1) {
nodes {
commit {
statusCheckRollup {
state
contexts(first: 20) {
# 11 keeps per-PR node cost under budget for 25-PR batch queries
# (total cost ≤5000). Repos with >11 checks lose individual check
# visibility, but the rollup "state" still reflects all checks —
# overall pass/fail detection remains correct.
contexts(first: 11) {
nodes {
... on CheckRun {
name
@ -534,6 +681,7 @@ export function generateBatchQuery(prs: PRInfo[]): {
return {
query: `query BatchPRs(${variableDefs}) {
${selections.join("\n")}
rateLimit { cost remaining resetAt }
}`,
variables,
};
@ -567,22 +715,32 @@ async function executeBatchQuery(
}
}
const args = ["api", "graphql", ...varArgs, "-f", `query=${query}`];
const args = ["api", "graphql", "-i", ...varArgs, "-f", `query=${query}`];
// Scale timeout based on batch size to prevent large batches from timing out
// Base: 30s, +2s per PR beyond first 10
const batchSize = prs.length;
const adaptiveTimeout = 30_000 + Math.max(0, (batchSize - 10) * 2000);
const { stdout } = await execFileAsync("gh", args, {
maxBuffer: 10 * 1024 * 1024,
timeout: adaptiveTimeout,
});
const stdout = await execGhAsync(args, adaptiveTimeout, "gh.api.graphql-batch");
// With -i, stdout contains HTTP headers + blank line + JSON body.
// Split at first blank line to get the JSON body for parsing.
// The tracer (execGhObserved) already parses the headers for its trace row.
const blankLineIdx = stdout.indexOf("\r\n\r\n");
const altBlankLineIdx = stdout.indexOf("\n\n");
const splitIdx =
blankLineIdx >= 0 && (altBlankLineIdx < 0 || blankLineIdx < altBlankLineIdx)
? blankLineIdx + 4
: altBlankLineIdx >= 0
? altBlankLineIdx + 2
: 0;
const body = splitIdx > 0 ? stdout.slice(splitIdx) : stdout;
const result: {
data?: Record<string, unknown>;
errors?: Array<{ message: string; path?: string[] }>;
} = JSON.parse(stdout.trim());
} = JSON.parse(body.trim());
// Check for GraphQL errors and throw to allow individual API fallback
if (result.errors && result.errors.length > 0) {
@ -760,7 +918,6 @@ function extractPREnrichment(
if (
pr["state"] === undefined &&
pr["title"] === undefined &&
pr["reviews"] === undefined &&
pr["commits"] === undefined
) {
return null;
@ -882,15 +1039,17 @@ function extractPREnrichment(
export async function enrichSessionsPRBatch(
prs: PRInfo[],
observer?: BatchObserver,
): Promise<Map<string, PREnrichmentData>> {
repos: string[] = [],
): Promise<BatchEnrichmentResult> {
const result = new Map<string, PREnrichmentData>();
if (prs.length === 0) {
return result;
}
// Step 1: Check if we need to refresh using 2-Guard ETag Strategy
const guardResult = await shouldRefreshPREnrichment(prs);
// Guard 1 runs for all repos (including those with no PRs yet) so the
// lifecycle manager knows whether detectPR can be skipped.
const guardResult = await shouldRefreshPREnrichment(prs, repos);
// Report which repos had no PR list changes so the lifecycle can skip detectPR
observer?.reportPRListUnchangedRepos?.(guardResult.prListUnchangedRepos);
if (!guardResult.shouldRefresh) {
// No changes detected - try to return cached data
@ -913,7 +1072,7 @@ export async function enrichSessionsPRBatch(
"info",
`[ETag Guard] Skipping GraphQL batch - all ${result.size} PRs cached. Reasons: ${guardResult.details.join(", ")}`,
);
return result;
return { enrichment: result, prListUnchangedRepos: guardResult.prListUnchangedRepos };
}
// Some PRs not cached - fetch missing PRs via GraphQL
@ -1006,7 +1165,7 @@ export async function enrichSessionsPRBatch(
}
}
return result;
return { enrichment: result, prListUnchangedRepos: guardResult.prListUnchangedRepos };
}
// Export internal functions for testing

778
packages/plugins/scm-github/src/index.ts
View File

@ -9,6 +9,7 @@ import { createHmac, timingSafeEqual } from "node:crypto";
import { promisify } from "node:util";
import {
CI_STATUS,
execGhObserved,
type PluginModule,
type SCM,
type SCMWebhookEvent,
@ -24,13 +25,15 @@ import {
type Review,
type ReviewDecision,
type ReviewComment,
type AutomatedComment,
type ReviewSummary,
type ReviewThreadsResult,
type MergeReadiness,
type PREnrichmentData,
type BatchObserver,
} from "@aoagents/ao-core";
import {
enrichSessionsPRBatch as enrichSessionsPRBatchImpl,
checkReviewCommentsETag,
} from "./graphql-batch.js";
import {
getWebhookHeader,
@ -77,11 +80,11 @@ async function execCli(bin: ExecCommand, args: string[], cwd?: string): Promise<
}
async function gh(args: string[]): Promise<string> {
return execCli("gh", args);
return execGhObserved(args, { component: "scm-github" }, 30_000);
}
async function ghInDir(args: string[], cwd: string): Promise<string> {
return execCli("gh", args, cwd);
return execGhObserved(args, { component: "scm-github", cwd }, 30_000);
}
async function git(args: string[], cwd: string): Promise<string> {
@ -450,7 +453,89 @@ function parseDate(val: string | undefined | null): Date {
// SCM implementation
// ---------------------------------------------------------------------------
// In-process PR cache. Per-method TTLs balance call reduction against
// staleness. Tightest TTLs (5s) on the fastest-changing decision-critical
// fields (state, CI, mergeability) — well under one poll cycle. Slightly
// looser (10s) on review-state and review-comments which tolerate up to
// 10-30s staleness per the agreed policy and benefit measurably from a
// looser window in trace replay. detectPR uses 30s because once a PR is
// discovered for a branch, that fact is stable for the session — and 5s was
// far below the per-branch poll cadence (~30s), making the cache near-useless.
// detectPR caches positive results only (never []) so a freshly created PR
// is discovered on the very next poll.
const PR_CACHE_TTL_MS = {
resolvePR: 60_000, // identity metadata (number, url, title, branch refs, isDraft)
getPRState: 5_000, // open / merged / closed
getPRSummary: 5_000, // state + title + additions/deletions
getReviews: 10_000, // review array (state, body, author)
getReviewDecision: 10_000, // approved / changes_requested / pending
getCIChecks: 5_000, // CI check list (name, state, link, timestamps)
getMergeability: 5_000, // composite merge readiness
getPendingComments: 10_000, // unresolved review threads (GraphQL)
detectPR: 30_000, // positive hits only — branch-PR mapping is stable once known
} as const;
const PR_CACHE_MAX_ENTRIES = 1000;
type PRCacheMethod = keyof typeof PR_CACHE_TTL_MS;
function createGitHubSCM(): SCM {
// Per-instance cache so each createGitHubSCM() returns an isolated cache —
// tests get clean state on each create() call.
const prCache = new Map<string, { value: unknown; expiresAt: number }>();
// ETag-controlled cache for review threads + reviews. Freshness is managed by
// Guard 3 (checkReviewCommentsETag) — not a TTL timer.
const reviewThreadsCache = new Map<string, ReviewThreadsResult>();
function prCacheKey(owner: string, repo: string, prKey: string, method: PRCacheMethod): string {
return `${owner}/${repo}#${prKey}:${method}`;
}
function readPRCache<T>(key: string): T | null {
const entry = prCache.get(key);
if (!entry) return null;
if (Date.now() > entry.expiresAt) {
prCache.delete(key);
return null;
}
return entry.value as T;
}
function writePRCache<T>(key: string, value: T, ttlMs: number): void {
if (prCache.size >= PR_CACHE_MAX_ENTRIES) {
const oldest = prCache.keys().next().value;
if (oldest !== undefined) prCache.delete(oldest);
}
prCache.set(key, { value, expiresAt: Date.now() + ttlMs });
}
// Wipe every method's cache entry for a specific PR. Called on writes
// (pr edit/merge/close) to avoid serving stale state after our own mutation.
// Also wipes the branch-keyed detectPR entry since mergePR deletes the branch.
function invalidatePRCache(pr: PRInfo): void {
const prefix = `${pr.owner}/${pr.repo}#${pr.number}:`;
for (const key of prCache.keys()) {
if (key.startsWith(prefix)) prCache.delete(key);
}
prCache.delete(prCacheKey(pr.owner, pr.repo, pr.branch, "detectPR"));
reviewThreadsCache.delete(`${pr.owner}/${pr.repo}#${pr.number}`);
}
async function withPRCache<T>(
owner: string,
repo: string,
prKey: string,
method: PRCacheMethod,
fetcher: () => Promise<T>,
): Promise<T> {
const key = prCacheKey(owner, repo, prKey, method);
const cached = readPRCache<T>(key);
if (cached !== null) return cached;
const value = await fetcher();
writePRCache(key, value, PR_CACHE_TTL_MS[method]);
return value;
}
return {
name: "github",
@ -517,6 +602,13 @@ function createGitHubSCM(): SCM {
async detectPR(session: Session, project: ProjectConfig): Promise<PRInfo | null> {
if (!session.branch || !project.repo) return null;
parseProjectRepo(project.repo);
const [owner, repoName] = project.repo.split("/");
// Positive-only cache: never cache [] (null). A just-created PR must
// surface on the next poll, so we pay the gh call for misses but save
// every call after the PR is discovered.
const cacheK = prCacheKey(owner ?? "", repoName ?? "", session.branch, "detectPR");
const cached = readPRCache<PRInfo>(cacheK);
if (cached !== null) return cached;
try {
const raw = await gh([
"pr",
@ -542,7 +634,9 @@ function createGitHubSCM(): SCM {
if (prs.length === 0) return null;
return prInfoFromView(prs[0], project.repo);
const info = prInfoFromView(prs[0], project.repo);
writePRCache(cacheK, info, PR_CACHE_TTL_MS.detectPR);
return info;
} catch {
return null;
}
@ -552,30 +646,38 @@ function createGitHubSCM(): SCM {
if (!project.repo) {
throw new Error("Cannot resolve PR: project has no repo configured");
}
const raw = await gh([
"pr",
"view",
reference,
"--repo",
project.repo,
"--json",
"number,url,title,headRefName,baseRefName,isDraft",
]);
const repo = project.repo;
const [owner, repoName] = repo.split("/");
// Cache by reference (number, branch, or URL — caller-provided).
// Identity metadata (number, url, title, branch refs, isDraft) is stable
// for the life of a PR; 60s TTL is safely under any user-noticeable window.
return withPRCache(owner ?? "", repoName ?? "", `ref=${reference}`, "resolvePR", async () => {
const raw = await gh([
"pr",
"view",
reference,
"--repo",
repo,
"--json",
"number,url,title,headRefName,baseRefName,isDraft",
]);
const data: {
number: number;
url: string;
title: string;
headRefName: string;
baseRefName: string;
isDraft: boolean;
} = JSON.parse(raw);
const data: {
number: number;
url: string;
title: string;
headRefName: string;
baseRefName: string;
isDraft: boolean;
} = JSON.parse(raw);
return prInfoFromView(data, project.repo);
return prInfoFromView(data, repo);
});
},
async assignPRToCurrentUser(pr: PRInfo): Promise<void> {
await gh(["pr", "edit", String(pr.number), "--repo", repoFlag(pr), "--add-assignee", "@me"]);
invalidatePRCache(pr);
},
async checkoutPR(pr: PRInfo, workspacePath: string): Promise<boolean> {
@ -594,96 +696,113 @@ function createGitHubSCM(): SCM {
},
async getPRState(pr: PRInfo): Promise<PRState> {
const raw = await gh([
"pr",
"view",
String(pr.number),
"--repo",
repoFlag(pr),
"--json",
"state",
]);
const data: { state: string } = JSON.parse(raw);
const s = data.state.toUpperCase();
if (s === "MERGED") return "merged";
if (s === "CLOSED") return "closed";
return "open";
// 5s TTL — state is decision-influencing (lifecycle uses it for cleanup),
// but 5s is well under one poll cycle so the lifecycle worker still sees
// freshly observed transitions on its next pass.
return withPRCache(pr.owner, pr.repo, String(pr.number), "getPRState", async () => {
const raw = await gh([
"pr",
"view",
String(pr.number),
"--repo",
repoFlag(pr),
"--json",
"state",
]);
const data: { state: string } = JSON.parse(raw);
const s = data.state.toUpperCase();
if (s === "MERGED") return "merged";
if (s === "CLOSED") return "closed";
return "open";
});
},
async getPRSummary(pr: PRInfo) {
const raw = await gh([
"pr",
"view",
String(pr.number),
"--repo",
repoFlag(pr),
"--json",
"state,title,additions,deletions",
]);
const data: {
state: string;
title: string;
additions: number;
deletions: number;
} = JSON.parse(raw);
const s = data.state.toUpperCase();
const state: PRState = s === "MERGED" ? "merged" : s === "CLOSED" ? "closed" : "open";
return {
state,
title: data.title ?? "",
additions: data.additions ?? 0,
deletions: data.deletions ?? 0,
};
// 5s TTL — includes state, so same freshness contract as getPRState.
// Title and additions/deletions change rarely; they ride along.
return withPRCache(pr.owner, pr.repo, String(pr.number), "getPRSummary", async () => {
const raw = await gh([
"pr",
"view",
String(pr.number),
"--repo",
repoFlag(pr),
"--json",
"state,title,additions,deletions",
]);
const data: {
state: string;
title: string;
additions: number;
deletions: number;
} = JSON.parse(raw);
const s = data.state.toUpperCase();
const state: PRState = s === "MERGED" ? "merged" : s === "CLOSED" ? "closed" : "open";
return {
state,
title: data.title ?? "",
additions: data.additions ?? 0,
deletions: data.deletions ?? 0,
};
});
},
async mergePR(pr: PRInfo, method: MergeMethod = "squash"): Promise<void> {
const flag = method === "rebase" ? "--rebase" : method === "merge" ? "--merge" : "--squash";
await gh(["pr", "merge", String(pr.number), "--repo", repoFlag(pr), flag, "--delete-branch"]);
invalidatePRCache(pr);
},
async closePR(pr: PRInfo): Promise<void> {
await gh(["pr", "close", String(pr.number), "--repo", repoFlag(pr)]);
invalidatePRCache(pr);
},
async getCIChecks(pr: PRInfo): Promise<CICheck[]> {
try {
const raw = await gh([
"pr",
"checks",
String(pr.number),
"--repo",
repoFlag(pr),
"--json",
"name,state,link,startedAt,completedAt",
]);
// 5s TTL — CI state can flip quickly; within one poll cycle is acceptable
// per the agreed fast-changing-fields policy. Fallback to statusCheckRollup
// for older gh CLI versions happens inside the fetcher and rides on the
// same cache entry.
return withPRCache(pr.owner, pr.repo, String(pr.number), "getCIChecks", async () => {
try {
const raw = await gh([
"pr",
"checks",
String(pr.number),
"--repo",
repoFlag(pr),
"--json",
"name,state,link,startedAt,completedAt",
]);
const checks: Array<{
name: string;
state: string;
link: string;
startedAt: string;
completedAt: string;
}> = JSON.parse(raw);
const checks: Array<{
name: string;
state: string;
link: string;
startedAt: string;
completedAt: string;
}> = JSON.parse(raw);
return checks.map((c) => {
const state = c.state?.toUpperCase();
return checks.map((c) => {
const state = c.state?.toUpperCase();
return {
name: c.name,
status: mapRawCheckStateToStatus(state),
url: c.link || undefined,
conclusion: state || undefined,
startedAt: c.startedAt ? new Date(c.startedAt) : undefined,
completedAt: c.completedAt ? new Date(c.completedAt) : undefined,
};
});
} catch (err) {
if (isUnsupportedPrChecksJsonError(err)) {
return getCIChecksFromStatusRollup(pr);
return {
name: c.name,
status: mapRawCheckStateToStatus(state),
url: c.link || undefined,
conclusion: state || undefined,
startedAt: c.startedAt ? new Date(c.startedAt) : undefined,
completedAt: c.completedAt ? new Date(c.completedAt) : undefined,
};
});
} catch (err) {
if (isUnsupportedPrChecksJsonError(err)) {
return getCIChecksFromStatusRollup(pr);
}
throw new Error("Failed to fetch CI checks", { cause: err });
}
throw new Error("Failed to fetch CI checks", { cause: err });
}
});
},
async getCISummary(pr: PRInfo): Promise<CIStatus> {
@ -721,65 +840,78 @@ function createGitHubSCM(): SCM {
},
async getReviews(pr: PRInfo): Promise<Review[]> {
const raw = await gh([
"pr",
"view",
String(pr.number),
"--repo",
repoFlag(pr),
"--json",
"reviews",
]);
const data: {
reviews: Array<{
author: { login: string };
state: string;
body: string;
submittedAt: string;
}>;
} = JSON.parse(raw);
// 5s TTL — review array. Reviewers are async, so the lifecycle worker
// sees a new review on its next poll cycle within 5s of the cache expiring.
return withPRCache(pr.owner, pr.repo, String(pr.number), "getReviews", async () => {
const raw = await gh([
"pr",
"view",
String(pr.number),
"--repo",
repoFlag(pr),
"--json",
"reviews",
]);
const data: {
reviews: Array<{
author: { login: string };
state: string;
body: string;
submittedAt: string;
}>;
} = JSON.parse(raw);
return data.reviews.map((r) => {
let state: Review["state"];
const s = r.state?.toUpperCase();
if (s === "APPROVED") state = "approved";
else if (s === "CHANGES_REQUESTED") state = "changes_requested";
else if (s === "DISMISSED") state = "dismissed";
else if (s === "PENDING") state = "pending";
else state = "commented";
return data.reviews.map((r) => {
let state: Review["state"];
const s = r.state?.toUpperCase();
if (s === "APPROVED") state = "approved";
else if (s === "CHANGES_REQUESTED") state = "changes_requested";
else if (s === "DISMISSED") state = "dismissed";
else if (s === "PENDING") state = "pending";
else state = "commented";
return {
author: r.author?.login ?? "unknown",
state,
body: r.body || undefined,
submittedAt: parseDate(r.submittedAt),
};
return {
author: r.author?.login ?? "unknown",
state,
body: r.body || undefined,
submittedAt: parseDate(r.submittedAt),
};
});
});
},
async getReviewDecision(pr: PRInfo): Promise<ReviewDecision> {
const raw = await gh([
"pr",
"view",
String(pr.number),
"--repo",
repoFlag(pr),
"--json",
"reviewDecision",
]);
const data: { reviewDecision: string } = JSON.parse(raw);
// 5s TTL — review decision is decision-influencing (gates merge), kept
// tight so a fresh "approved" surfaces within one poll cycle.
return withPRCache(pr.owner, pr.repo, String(pr.number), "getReviewDecision", async () => {
const raw = await gh([
"pr",
"view",
String(pr.number),
"--repo",
repoFlag(pr),
"--json",
"reviewDecision",
]);
const data: { reviewDecision: string } = JSON.parse(raw);
const d = (data.reviewDecision ?? "").toUpperCase();
if (d === "APPROVED") return "approved";
if (d === "CHANGES_REQUESTED") return "changes_requested";
if (d === "REVIEW_REQUIRED") return "pending";
return "none";
const d = (data.reviewDecision ?? "").toUpperCase();
if (d === "APPROVED") return "approved";
if (d === "CHANGES_REQUESTED") return "changes_requested";
if (d === "REVIEW_REQUIRED") return "pending";
return "none";
});
},
async getPendingComments(pr: PRInfo): Promise<ReviewComment[]> {
try {
// Use GraphQL with variables to get review threads with actual isResolved status
const raw = await gh([
// 5s TTL — review threads are decision-influencing (gates whether AO
// reacts to new comments). Within one poll cycle is acceptable. Note:
// ETag does not work on /graphql per Experiment 2 (G2), so TTL is the
// only practical lever here.
return withPRCache(pr.owner, pr.repo, String(pr.number), "getPendingComments", async () => {
try {
// Use GraphQL with variables to get review threads with actual isResolved status
const raw = await gh([
"api",
"graphql",
"-f",
@ -794,6 +926,7 @@ function createGitHubSCM(): SCM {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
id
isResolved
comments(first: 1) {
nodes {
@ -819,6 +952,7 @@ function createGitHubSCM(): SCM {
pullRequest: {
reviewThreads: {
nodes: Array<{
id: string;
isResolved: boolean;
comments: {
nodes: Array<{
@ -852,6 +986,7 @@ function createGitHubSCM(): SCM {
const c = t.comments.nodes[0];
return {
id: c.id,
threadId: t.id,
author: c.author?.login ?? "unknown",
body: c.body,
path: c.path || undefined,
@ -861,171 +996,234 @@ function createGitHubSCM(): SCM {
url: c.url,
};
});
} catch (err) {
throw new Error("Failed to fetch pending comments", { cause: err });
}
} catch (err) {
throw new Error("Failed to fetch pending comments", { cause: err });
}
});
},
async getAutomatedComments(pr: PRInfo): Promise<AutomatedComment[]> {
async getReviewThreads(pr: PRInfo): Promise<ReviewThreadsResult> {
const cacheKey = `${pr.owner}/${pr.repo}#${pr.number}`;
// Guard 3: check if review comments changed via REST ETag
const reviewsChanged = await checkReviewCommentsETag(pr.owner, pr.repo, pr.number);
if (!reviewsChanged) {
const cached = reviewThreadsCache.get(cacheKey);
if (cached) return cached;
}
try {
const perPage = 100;
const comments: Array<{
id: number;
user: { login: string };
body: string;
path: string;
line: number | null;
original_line: number | null;
created_at: string;
html_url: string;
}> = [];
for (let page = 1; ; page++) {
const raw = await gh([
"api",
"--method",
"GET",
`repos/${repoFlag(pr)}/pulls/${pr.number}/comments?per_page=${perPage}&page=${page}`,
]);
const pageComments: Array<{
id: number;
user: { login: string };
body: string;
path: string;
line: number | null;
original_line: number | null;
created_at: string;
html_url: string;
}> = JSON.parse(raw);
if (pageComments.length === 0) {
break;
}
comments.push(...pageComments);
if (pageComments.length < perPage) {
break;
}
}
return comments
.filter((c) => BOT_AUTHORS.has(c.user?.login ?? ""))
.map((c) => {
// Determine severity from body content
let severity: AutomatedComment["severity"] = "info";
const bodyLower = c.body.toLowerCase();
if (
bodyLower.includes("error") ||
bodyLower.includes("bug") ||
bodyLower.includes("critical") ||
bodyLower.includes("potential issue")
) {
severity = "error";
} else if (
bodyLower.includes("warning") ||
bodyLower.includes("suggest") ||
bodyLower.includes("consider")
) {
severity = "warning";
const rawWithHeaders = await gh([
"api",
"graphql",
"-i",
"-f",
`owner=${pr.owner}`,
"-f",
`name=${pr.repo}`,
"-F",
`number=${pr.number}`,
"-f",
`query=query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(last: 100) {
nodes {
id
isResolved
comments(first: 1) {
nodes {
id
author { login }
body
path
line
url
createdAt
}
}
}
}
reviews(last: 5) {
nodes {
author { login }
state
body
submittedAt
}
}
}
}
rateLimit { cost remaining resetAt }
}`,
]);
// Strip HTTP headers from -i response to get JSON body
const raw = rawWithHeaders.replace(/^[\s\S]*?\r?\n\r?\n/, "");
const data: {
data: {
repository: {
pullRequest: {
reviewThreads: {
nodes: Array<{
id: string;
isResolved: boolean;
comments: {
nodes: Array<{
id: string;
author: { login: string } | null;
body: string;
path: string | null;
line: number | null;
url: string;
createdAt: string;
}>;
};
}>;
};
reviews: {
nodes: Array<{
author: { login: string } | null;
state: string;
body: string;
submittedAt: string;
}>;
};
};
};
};
} = JSON.parse(raw);
const threadNodes = data.data.repository.pullRequest.reviewThreads.nodes;
const reviewNodes = data.data.repository.pullRequest.reviews.nodes;
const threads: ReviewComment[] = threadNodes
.filter((t) => {
if (t.isResolved) return false;
const c = t.comments.nodes[0];
return !!c;
})
.map((t) => {
const c = t.comments.nodes[0];
const author = c.author?.login ?? "unknown";
return {
id: String(c.id),
botName: c.user?.login ?? "unknown",
id: c.id,
threadId: t.id,
author,
body: c.body,
path: c.path || undefined,
line: c.line ?? c.original_line ?? undefined,
severity,
createdAt: parseDate(c.created_at),
url: c.html_url,
line: c.line ?? undefined,
isResolved: t.isResolved,
createdAt: parseDate(c.createdAt),
url: c.url,
isBot: BOT_AUTHORS.has(author),
};
});
const reviews: ReviewSummary[] = reviewNodes
.filter((r) => r.body && r.body.trim().length > 0)
.map((r) => ({
author: r.author?.login ?? "unknown",
state: r.state,
body: r.body,
submittedAt: parseDate(r.submittedAt),
}));
const result: ReviewThreadsResult = { threads, reviews };
reviewThreadsCache.set(cacheKey, result);
return result;
} catch (err) {
throw new Error("Failed to fetch automated comments", { cause: err });
throw new Error("Failed to fetch review threads", { cause: err });
}
},
async getMergeability(pr: PRInfo): Promise<MergeReadiness> {
const blockers: string[] = [];
// 5s TTL — composite merge readiness. Internal getPRState/getCISummary
// calls are also cached (5s each) so even on cache miss this is cheap.
// Cached entry covers the full computed result so duplicate poll-cycle
// calls don't re-derive blockers.
return withPRCache(pr.owner, pr.repo, String(pr.number), "getMergeability", async () => {
const blockers: string[] = [];
// First, check if the PR is merged
// GitHub returns mergeable=null for merged PRs, which is not useful
// Note: We only skip checks for merged PRs. Closed PRs still need accurate status.
const state = await this.getPRState(pr);
if (state === "merged") {
// For merged PRs, return a clean result without querying mergeable status
return {
mergeable: true,
ciPassing: true,
approved: true,
noConflicts: true,
blockers: [],
};
}
// Fetch PR details with merge state
const raw = await gh([
"pr",
"view",
String(pr.number),
"--repo",
repoFlag(pr),
"--json",
"mergeable,reviewDecision,mergeStateStatus,isDraft",
]);
const data: {
mergeable: string;
reviewDecision: string;
mergeStateStatus: string;
isDraft: boolean;
} = JSON.parse(raw);
// CI
const ciStatus = await this.getCISummary(pr);
const ciPassing = ciStatus === CI_STATUS.PASSING || ciStatus === CI_STATUS.NONE;
if (!ciPassing) {
blockers.push(`CI is ${ciStatus}`);
}
// Reviews
const reviewDecision = (data.reviewDecision ?? "").toUpperCase();
const approved = reviewDecision === "APPROVED";
if (reviewDecision === "CHANGES_REQUESTED") {
blockers.push("Changes requested in review");
} else if (reviewDecision === "REVIEW_REQUIRED") {
blockers.push("Review required");
}
// Conflicts / merge state
const mergeable = (data.mergeable ?? "").toUpperCase();
const mergeState = (data.mergeStateStatus ?? "").toUpperCase();
const noConflicts = mergeable === "MERGEABLE";
if (mergeable === "CONFLICTING") {
blockers.push("Merge conflicts");
} else if (mergeable === "UNKNOWN" || mergeable === "") {
blockers.push("Merge status unknown (GitHub is computing)");
}
if (mergeState === "BEHIND") {
blockers.push("Branch is behind base branch");
} else if (mergeState === "BLOCKED") {
blockers.push("Merge is blocked by branch protection");
} else if (mergeState === "UNSTABLE") {
blockers.push("Required checks are failing");
}
// Draft
if (data.isDraft) {
blockers.push("PR is still a draft");
}
// First, check if the PR is merged
// GitHub returns mergeable=null for merged PRs, which is not useful
// Note: We only skip checks for merged PRs. Closed PRs still need accurate status.
const state = await this.getPRState(pr);
if (state === "merged") {
// For merged PRs, return a clean result without querying mergeable status
return {
mergeable: true,
ciPassing: true,
approved: true,
noConflicts: true,
blockers: [],
mergeable: blockers.length === 0,
ciPassing,
approved,
noConflicts,
blockers,
};
}
// Fetch PR details with merge state
const raw = await gh([
"pr",
"view",
String(pr.number),
"--repo",
repoFlag(pr),
"--json",
"mergeable,reviewDecision,mergeStateStatus,isDraft",
]);
const data: {
mergeable: string;
reviewDecision: string;
mergeStateStatus: string;
isDraft: boolean;
} = JSON.parse(raw);
// CI
const ciStatus = await this.getCISummary(pr);
const ciPassing = ciStatus === CI_STATUS.PASSING || ciStatus === CI_STATUS.NONE;
if (!ciPassing) {
blockers.push(`CI is ${ciStatus}`);
}
// Reviews
const reviewDecision = (data.reviewDecision ?? "").toUpperCase();
const approved = reviewDecision === "APPROVED";
if (reviewDecision === "CHANGES_REQUESTED") {
blockers.push("Changes requested in review");
} else if (reviewDecision === "REVIEW_REQUIRED") {
blockers.push("Review required");
}
// Conflicts / merge state
const mergeable = (data.mergeable ?? "").toUpperCase();
const mergeState = (data.mergeStateStatus ?? "").toUpperCase();
const noConflicts = mergeable === "MERGEABLE";
if (mergeable === "CONFLICTING") {
blockers.push("Merge conflicts");
} else if (mergeable === "UNKNOWN" || mergeable === "") {
blockers.push("Merge status unknown (GitHub is computing)");
}
if (mergeState === "BEHIND") {
blockers.push("Branch is behind base branch");
} else if (mergeState === "BLOCKED") {
blockers.push("Merge is blocked by branch protection");
} else if (mergeState === "UNSTABLE") {
blockers.push("Required checks are failing");
}
// Draft
if (data.isDraft) {
blockers.push("PR is still a draft");
}
return {
mergeable: blockers.length === 0,
ciPassing,
approved,
noConflicts,
blockers,
};
});
},
/**
@ -1041,8 +1239,10 @@ function createGitHubSCM(): SCM {
async enrichSessionsPRBatch(
prs: PRInfo[],
observer?: BatchObserver,
repos?: string[],
): Promise<Map<string, PREnrichmentData>> {
return enrichSessionsPRBatchImpl(prs, observer);
const batchResult = await enrichSessionsPRBatchImpl(prs, observer, repos);
return batchResult.enrichment;
},
};
}

1
packages/plugins/scm-github/test/graphql-batch.integration.test.ts
View File

@ -221,7 +221,6 @@ describe("GraphQL Query Generation", () => {
"mergeable",
"mergeStateStatus",
"reviewDecision",
"reviews",
"commits",
"statusCheckRollup",
];

3
packages/plugins/scm-github/test/graphql-batch.test.ts
View File

@ -161,7 +161,6 @@ describe("GraphQL Batch Query Generation", () => {
expect(query).toContain("mergeable");
expect(query).toContain("mergeStateStatus");
expect(query).toContain("reviewDecision");
expect(query).toContain("reviews");
expect(query).toContain("commits");
expect(query).toContain("statusCheckRollup");
});
@ -823,7 +822,7 @@ describe("shouldRefreshPREnrichment - ETag Guard Strategy", () => {
const result = await shouldRefreshPREnrichment([]);
expect(result.shouldRefresh).toBe(false);
expect(result.details).toContain("No PRs to check");
expect(result.details).toContain("No repos to check");
// Should not make any API calls
expect(mockExecFileImpl).not.toHaveBeenCalled();
});

503
packages/plugins/scm-github/test/index.test.ts
View File

@ -483,7 +483,7 @@ describe("scm-github plugin", () => {
ghMock.mockResolvedValueOnce({ stdout: "" });
await scm.assignPRToCurrentUser?.(pr);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
["pr", "edit", "42", "--repo", "acme/repo", "--add-assignee", "@me"],
expect.any(Object),
);
@ -530,7 +530,7 @@ describe("scm-github plugin", () => {
ghMock.mockResolvedValueOnce({ stdout: "" });
await scm.mergePR(pr);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
["pr", "merge", "42", "--repo", "acme/repo", "--squash", "--delete-branch"],
expect.any(Object),
);
@ -540,7 +540,7 @@ describe("scm-github plugin", () => {
ghMock.mockResolvedValueOnce({ stdout: "" });
await scm.mergePR(pr, "merge");
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
expect.arrayContaining(["--merge"]),
expect.any(Object),
);
@ -550,7 +550,7 @@ describe("scm-github plugin", () => {
ghMock.mockResolvedValueOnce({ stdout: "" });
await scm.mergePR(pr, "rebase");
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
expect.arrayContaining(["--rebase"]),
expect.any(Object),
);
@ -564,7 +564,7 @@ describe("scm-github plugin", () => {
ghMock.mockResolvedValueOnce({ stdout: "" });
await scm.closePR(pr);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
["pr", "close", "42", "--repo", "acme/repo"],
expect.any(Object),
);
@ -916,169 +916,6 @@ describe("scm-github plugin", () => {
});
});
// ---- getAutomatedComments ----------------------------------------------
describe("getAutomatedComments", () => {
it("uses explicit GET query for pulls comments and paginates", async () => {
const page1 = Array.from({ length: 100 }, (_, i) => ({
id: i + 1,
user: { login: "cursor[bot]" },
body: "Potential issue detected",
path: "a.ts",
line: i + 1,
original_line: null,
created_at: "2025-01-01T00:00:00Z",
html_url: `u${i + 1}`,
}));
const page2 = [
{
id: 101,
user: { login: "cursor[bot]" },
body: "Warning: check this",
path: "b.ts",
line: 7,
original_line: null,
created_at: "2025-01-01T00:00:00Z",
html_url: "u101",
},
];
mockGh(page1);
mockGh(page2);
const comments = await scm.getAutomatedComments(pr);
expect(comments).toHaveLength(101);
expect(ghMock).toHaveBeenNthCalledWith(
1,
"gh",
["api", "--method", "GET", "repos/acme/repo/pulls/42/comments?per_page=100&page=1"],
expect.any(Object),
);
expect(ghMock).toHaveBeenNthCalledWith(
2,
"gh",
["api", "--method", "GET", "repos/acme/repo/pulls/42/comments?per_page=100&page=2"],
expect.any(Object),
);
});
it("returns bot comments filtered from all PR comments", async () => {
mockGh([
{
id: 1,
user: { login: "cursor[bot]" },
body: "Found a potential issue",
path: "a.ts",
line: 5,
original_line: null,
created_at: "2025-01-01T00:00:00Z",
html_url: "u1",
},
{
id: 2,
user: { login: "alice" },
body: "Human comment",
path: "a.ts",
line: 1,
original_line: null,
created_at: "2025-01-01T00:00:00Z",
html_url: "u2",
},
]);
const comments = await scm.getAutomatedComments(pr);
expect(comments).toHaveLength(1);
expect(comments[0].botName).toBe("cursor[bot]");
expect(comments[0].severity).toBe("error"); // "potential issue" → error
});
it("classifies severity from body content", async () => {
mockGh([
{
id: 1,
user: { login: "github-actions[bot]" },
body: "Error: build failed",
path: "a.ts",
line: 1,
original_line: null,
created_at: "2025-01-01T00:00:00Z",
html_url: "u",
},
{
id: 2,
user: { login: "github-actions[bot]" },
body: "Warning: deprecated API",
path: "a.ts",
line: 2,
original_line: null,
created_at: "2025-01-01T00:00:00Z",
html_url: "u",
},
{
id: 3,
user: { login: "github-actions[bot]" },
body: "Deployed to staging",
path: "a.ts",
line: 3,
original_line: null,
created_at: "2025-01-01T00:00:00Z",
html_url: "u",
},
]);
const comments = await scm.getAutomatedComments(pr);
expect(comments).toHaveLength(3);
expect(comments[0].severity).toBe("error");
expect(comments[1].severity).toBe("warning");
expect(comments[2].severity).toBe("info");
});
it("returns empty when no bot comments", async () => {
mockGh([
{
id: 1,
user: { login: "alice" },
body: "Human comment",
path: "a.ts",
line: 1,
original_line: null,
created_at: "2025-01-01T00:00:00Z",
html_url: "u",
},
]);
const comments = await scm.getAutomatedComments(pr);
expect(comments).toEqual([]);
});
it("throws on error", async () => {
mockGhError("network failure");
await expect(scm.getAutomatedComments(pr)).rejects.toThrow(
"Failed to fetch automated comments",
);
});
it("uses original_line as fallback", async () => {
mockGh([
{
id: 1,
user: { login: "dependabot[bot]" },
body: "Suggest update",
path: "a.ts",
line: null,
original_line: 15,
created_at: "2025-01-01T00:00:00Z",
html_url: "u",
},
]);
const comments = await scm.getAutomatedComments(pr);
expect(comments[0].line).toBe(15);
});
});
// ---- getMergeability ---------------------------------------------------
describe("getMergeability", () => {
@ -1264,4 +1101,334 @@ describe("scm-github plugin", () => {
expect(result.mergeable).toBe(false);
});
});
// ---- PR cache (per-method TTLs, write invalidation) -------------------
describe("PR cache", () => {
it("getPRState second call within 5s hits cache", async () => {
mockGh({ state: "OPEN" });
const first = await scm.getPRState(pr);
const second = await scm.getPRState(pr);
expect(first).toBe("open");
expect(second).toBe("open");
expect(ghMock).toHaveBeenCalledTimes(1);
});
it("getPRState re-fetches after TTL expires (5s)", async () => {
vi.useFakeTimers();
try {
mockGh({ state: "OPEN" });
await scm.getPRState(pr);
expect(ghMock).toHaveBeenCalledTimes(1);
vi.advanceTimersByTime(5_001);
mockGh({ state: "MERGED" });
const fresh = await scm.getPRState(pr);
expect(fresh).toBe("merged");
expect(ghMock).toHaveBeenCalledTimes(2);
} finally {
vi.useRealTimers();
}
});
it("getPRState and getPRSummary use separate cache slots", async () => {
mockGh({ state: "OPEN" });
mockGh({ state: "OPEN", title: "T", additions: 10, deletions: 5 });
await scm.getPRState(pr);
await scm.getPRSummary(pr);
expect(ghMock).toHaveBeenCalledTimes(2);
// Both now cached — second round hits cache, no new gh calls
await scm.getPRState(pr);
await scm.getPRSummary(pr);
expect(ghMock).toHaveBeenCalledTimes(2);
});
it("getReviews caches independently of getReviewDecision", async () => {
mockGh({ reviews: [] });
mockGh({ reviewDecision: "APPROVED" });
await scm.getReviews(pr);
await scm.getReviewDecision(pr);
expect(ghMock).toHaveBeenCalledTimes(2);
// Cache hits on second round
await scm.getReviews(pr);
await scm.getReviewDecision(pr);
expect(ghMock).toHaveBeenCalledTimes(2);
});
it("different PRs cache independently", async () => {
const otherPR = { ...pr, number: 99 };
mockGh({ state: "OPEN" });
mockGh({ state: "MERGED" });
const a = await scm.getPRState(pr);
const b = await scm.getPRState(otherPR);
expect(a).toBe("open");
expect(b).toBe("merged");
expect(ghMock).toHaveBeenCalledTimes(2);
});
it("mergePR invalidates the PR's cache", async () => {
mockGh({ state: "OPEN" });
await scm.getPRState(pr);
expect(ghMock).toHaveBeenCalledTimes(1);
ghMock.mockResolvedValueOnce({ stdout: "" }); // gh pr merge
await scm.mergePR(pr);
mockGh({ state: "MERGED" });
const fresh = await scm.getPRState(pr);
expect(fresh).toBe("merged");
expect(ghMock).toHaveBeenCalledTimes(3);
});
it("closePR invalidates the PR's cache", async () => {
mockGh({ state: "OPEN" });
await scm.getPRState(pr);
ghMock.mockResolvedValueOnce({ stdout: "" }); // gh pr close
await scm.closePR(pr);
mockGh({ state: "CLOSED" });
const fresh = await scm.getPRState(pr);
expect(fresh).toBe("closed");
expect(ghMock).toHaveBeenCalledTimes(3);
});
it("assignPRToCurrentUser invalidates the PR's cache", async () => {
mockGh({ reviewDecision: "REVIEW_REQUIRED" });
await scm.getReviewDecision(pr);
ghMock.mockResolvedValueOnce({ stdout: "" }); // gh pr edit
await scm.assignPRToCurrentUser(pr);
mockGh({ reviewDecision: "REVIEW_REQUIRED" });
await scm.getReviewDecision(pr);
expect(ghMock).toHaveBeenCalledTimes(3); // view + edit + view again
});
it("invalidating one PR does not affect a different PR's cache", async () => {
const otherPR = { ...pr, number: 99 };
mockGh({ state: "OPEN" });
mockGh({ state: "OPEN" });
await scm.getPRState(pr);
await scm.getPRState(otherPR);
expect(ghMock).toHaveBeenCalledTimes(2);
ghMock.mockResolvedValueOnce({ stdout: "" });
await scm.closePR(pr); // wipes pr #42 only
mockGh({ state: "CLOSED" });
await scm.getPRState(pr); // re-fetches
await scm.getPRState(otherPR); // still cached
expect(ghMock).toHaveBeenCalledTimes(4);
});
it("resolvePR caches by reference for 60s", async () => {
mockGh({
number: 7,
url: "https://github.com/acme/repo/pull/7",
title: "Fix",
headRefName: "feat/x",
baseRefName: "main",
isDraft: false,
});
const first = await scm.resolvePR("feat/x", project);
const second = await scm.resolvePR("feat/x", project);
expect(first).toEqual(second);
expect(ghMock).toHaveBeenCalledTimes(1);
});
it("failures are not cached", async () => {
ghMock.mockRejectedValueOnce(new Error("boom"));
await expect(scm.getPRState(pr)).rejects.toThrow();
mockGh({ state: "OPEN" });
const fresh = await scm.getPRState(pr);
expect(fresh).toBe("open");
expect(ghMock).toHaveBeenCalledTimes(2);
});
it("each create() returns an isolated cache", async () => {
const scmA = create();
const scmB = create();
mockGh({ state: "OPEN" });
await scmA.getPRState(pr);
mockGh({ state: "MERGED" });
const fromB = await scmB.getPRState(pr);
expect(fromB).toBe("merged");
expect(ghMock).toHaveBeenCalledTimes(2);
});
// ---- detectPR (positive-only cache) ----
it("detectPR caches positive results (PR found)", async () => {
mockGh([
{
number: 42,
url: "https://github.com/acme/repo/pull/42",
title: "feat: add feature",
headRefName: "feat/my-feature",
baseRefName: "main",
isDraft: false,
},
]);
const a = await scm.detectPR(makeSession(), project);
const b = await scm.detectPR(makeSession(), project);
expect(a?.number).toBe(42);
expect(b?.number).toBe(42);
expect(ghMock).toHaveBeenCalledTimes(1);
});
it("detectPR does NOT cache negative results (no PR yet)", async () => {
mockGh([]);
mockGh([]);
const a = await scm.detectPR(makeSession(), project);
const b = await scm.detectPR(makeSession(), project);
expect(a).toBeNull();
expect(b).toBeNull();
expect(ghMock).toHaveBeenCalledTimes(2);
});
it("detectPR transitions null → PR on next call without cache poisoning", async () => {
mockGh([]); // first call: no PR yet
const before = await scm.detectPR(makeSession(), project);
expect(before).toBeNull();
mockGh([
{
number: 7,
url: "https://github.com/acme/repo/pull/7",
title: "Just created",
headRefName: "feat/my-feature",
baseRefName: "main",
isDraft: false,
},
]);
const after = await scm.detectPR(makeSession(), project);
expect(after?.number).toBe(7);
expect(ghMock).toHaveBeenCalledTimes(2);
});
it("detectPR caches different branches independently", async () => {
mockGh([
{
number: 1,
url: "u1",
title: "t1",
headRefName: "feat/a",
baseRefName: "main",
isDraft: false,
},
]);
mockGh([
{
number: 2,
url: "u2",
title: "t2",
headRefName: "feat/b",
baseRefName: "main",
isDraft: false,
},
]);
const a = await scm.detectPR(makeSession({ branch: "feat/a" }), project);
const b = await scm.detectPR(makeSession({ branch: "feat/b" }), project);
expect(a?.number).toBe(1);
expect(b?.number).toBe(2);
expect(ghMock).toHaveBeenCalledTimes(2);
});
it("mergePR invalidates the branch's detectPR entry", async () => {
mockGh([
{
number: 42,
url: "https://github.com/acme/repo/pull/42",
title: "feat",
headRefName: pr.branch,
baseRefName: "main",
isDraft: false,
},
]);
await scm.detectPR(makeSession({ branch: pr.branch }), project);
expect(ghMock).toHaveBeenCalledTimes(1);
ghMock.mockResolvedValueOnce({ stdout: "" }); // gh pr merge
await scm.mergePR(pr);
// detectPR re-fetches because the merge invalidated the branch entry
mockGh([]);
const after = await scm.detectPR(makeSession({ branch: pr.branch }), project);
expect(after).toBeNull();
expect(ghMock).toHaveBeenCalledTimes(3);
});
// ---- getCIChecks / getMergeability / getPendingComments ----
it("getCIChecks caches result (5s TTL)", async () => {
mockGh([{ name: "build", state: "SUCCESS", link: "u", startedAt: "", completedAt: "" }]);
await scm.getCIChecks(pr);
await scm.getCIChecks(pr);
expect(ghMock).toHaveBeenCalledTimes(1);
});
it("getMergeability caches the composite result", async () => {
// First call: getPRState (1) + pr view mergeable (2) + getCISummary→getCIChecks (3)
mockGh({ state: "OPEN" }); // getPRState
mockGh({
mergeable: "MERGEABLE",
reviewDecision: "APPROVED",
mergeStateStatus: "CLEAN",
isDraft: false,
});
mockGh([{ name: "build", state: "SUCCESS" }]); // getCIChecks
const first = await scm.getMergeability(pr);
expect(first.mergeable).toBe(true);
// Second call within TTL: top-level getMergeability cache hits.
// No new gh calls because the composite result short-circuits.
const second = await scm.getMergeability(pr);
expect(second).toEqual(first);
expect(ghMock).toHaveBeenCalledTimes(3);
});
it("mergePR invalidates getMergeability cache", async () => {
mockGh({ state: "OPEN" });
mockGh({
mergeable: "MERGEABLE",
reviewDecision: "APPROVED",
mergeStateStatus: "CLEAN",
isDraft: false,
});
mockGh([{ name: "build", state: "SUCCESS" }]);
await scm.getMergeability(pr);
expect(ghMock).toHaveBeenCalledTimes(3);
ghMock.mockResolvedValueOnce({ stdout: "" }); // gh pr merge
await scm.mergePR(pr);
// After merge: state cached as merged; getMergeability re-derives
// the merged shortcut result without making more gh calls.
mockGh({ state: "MERGED" }); // getPRState refetch
const after = await scm.getMergeability(pr);
expect(after.mergeable).toBe(true);
expect(after.blockers).toEqual([]);
});
it("getPendingComments caches result", async () => {
mockGhRaw(
JSON.stringify({
data: { repository: { pullRequest: { reviewThreads: { nodes: [] } } } },
}),
);
await scm.getPendingComments(pr);
await scm.getPendingComments(pr);
expect(ghMock).toHaveBeenCalledTimes(1);
});
});
});
function mockGhRaw(stdout: string) {
ghMock.mockResolvedValueOnce({ stdout });
}

63
packages/plugins/scm-gitlab/src/index.ts
View File

@ -22,7 +22,8 @@ import {
type Review,
type ReviewDecision,
type ReviewComment,
type AutomatedComment,
type ReviewSummary,
type ReviewThreadsResult,
type MergeReadiness,
} from "@aoagents/ao-core";
import {
@ -105,21 +106,6 @@ function mapPRState(state: string): PRState {
return "open";
}
function inferSeverity(body: string): AutomatedComment["severity"] {
const lower = body.toLowerCase();
if (
lower.includes("error") ||
lower.includes("bug") ||
lower.includes("critical") ||
lower.includes("potential issue")
) {
return "error";
}
if (lower.includes("warning") || lower.includes("suggest") || lower.includes("consider")) {
return "warning";
}
return "info";
}
function getGitLabWebhookConfig(project: ProjectConfig) {
const webhook = project.scm?.webhook;
@ -701,32 +687,57 @@ function createGitLabSCM(config?: Record<string, unknown>): SCM {
return comments;
},
async getAutomatedComments(pr: PRInfo): Promise<AutomatedComment[]> {
async getReviewThreads(pr: PRInfo): Promise<ReviewThreadsResult> {
const hostname = resolveHostname(pr);
const discussions = await fetchDiscussions(
pr,
resolveHostname(pr),
`getAutomatedComments for MR !${pr.number}`,
hostname,
`getReviewThreads for MR !${pr.number}`,
);
const comments: AutomatedComment[] = [];
// Unresolved threads — includes both human and bot comments (with isBot flag)
const threads: ReviewComment[] = [];
for (const d of discussions) {
const note = d.notes[0];
if (!note) continue;
const author = note.author?.username ?? "";
if (!isBot(author)) continue;
if (!note.resolvable || note.resolved) continue;
comments.push({
const author = note.author?.username ?? "unknown";
threads.push({
id: String(note.id),
botName: author,
threadId: d.id,
author,
body: note.body,
path: note.position?.new_path || undefined,
line: note.position?.new_line ?? undefined,
severity: inferSeverity(note.body),
isResolved: false,
createdAt: parseDate(note.created_at),
url: "",
isBot: isBot(author),
});
}
return comments;
// Review summaries from approvals
const reviews: ReviewSummary[] = [];
try {
const approvalsRaw = await glab(["api", `${mrApiPath(pr)}/approvals`], hostname);
const approvals = parseJSON<{
approved_by: Array<{ user: { username: string } }>;
}>(approvalsRaw, `getReviewThreads approvals for MR !${pr.number}`);
for (const a of approvals.approved_by ?? []) {
reviews.push({
author: a.user?.username ?? "unknown",
state: "APPROVED",
body: "",
submittedAt: new Date(0),
});
}
} catch {
// Best-effort — threads are the critical data
}
return { threads, reviews };
},
async getMergeability(pr: PRInfo): Promise<MergeReadiness> {

81
packages/plugins/scm-gitlab/test/index.test.ts
View File

@ -1022,87 +1022,6 @@ describe("scm-gitlab plugin", () => {
});
});
// ---- getAutomatedComments ----------------------------------------------
describe("getAutomatedComments", () => {
it("returns bot discussion notes with severity", async () => {
mockGlab([
{
notes: [
{
id: 101,
author: { username: "gitlab-bot" },
body: "Found a critical error",
position: { new_path: "a.ts", new_line: 5 },
created_at: "2025-01-01T00:00:00Z",
},
],
},
{
notes: [
{
id: 102,
author: { username: "alice" },
body: "Human comment",
created_at: "2025-01-01T00:00:00Z",
},
],
},
]);
const comments = await scm.getAutomatedComments(pr);
expect(comments).toHaveLength(1);
expect(comments[0].botName).toBe("gitlab-bot");
expect(comments[0].severity).toBe("error");
});
it("classifies severity from body content", async () => {
mockGlab([
{
notes: [
{
id: 1,
author: { username: "sast-bot" },
body: "Error: build failed",
created_at: "2025-01-01T00:00:00Z",
},
],
},
{
notes: [
{
id: 2,
author: { username: "sast-bot" },
body: "Warning: deprecated API",
created_at: "2025-01-01T00:00:00Z",
},
],
},
{
notes: [
{
id: 3,
author: { username: "sast-bot" },
body: "Deployed to staging",
created_at: "2025-01-01T00:00:00Z",
},
],
},
]);
const comments = await scm.getAutomatedComments(pr);
expect(comments).toHaveLength(3);
expect(comments[0].severity).toBe("error");
expect(comments[1].severity).toBe("warning");
expect(comments[2].severity).toBe("info");
});
it("throws on error (fail-closed)", async () => {
mockGlabError("network failure");
await expect(scm.getAutomatedComments(pr)).rejects.toThrow("network failure");
});
});
// ---- getMergeability ---------------------------------------------------
describe("getMergeability", () => {

158
packages/plugins/tracker-github/src/index.ts
View File

@ -4,31 +4,24 @@
* Uses the `gh` CLI for all GitHub API interactions.
*/
import { execFile } from "node:child_process";
import { promisify } from "node:util";
import type {
PluginModule,
Tracker,
Issue,
IssueFilters,
IssueUpdate,
CreateIssueInput,
ProjectConfig,
import {
execGhObserved,
type PluginModule,
type Tracker,
type Issue,
type IssueFilters,
type IssueUpdate,
type CreateIssueInput,
type ProjectConfig,
} from "@aoagents/ao-core";
const execFileAsync = promisify(execFile);
// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------
async function gh(args: string[]): Promise<string> {
try {
const { stdout } = await execFileAsync("gh", args, {
maxBuffer: 10 * 1024 * 1024,
timeout: 30_000,
});
return stdout.trim();
return await execGhObserved(args, { component: "tracker-github" }, 30_000);
} catch (err) {
throw new Error(`gh ${args.slice(0, 3).join(" ")} failed: ${(err as Error).message}`, {
cause: err,
@ -121,47 +114,106 @@ function requireRepo(project: ProjectConfig): string {
return project.repo;
}
// Issue cache: 5 min TTL, bounded to 500 entries. Issue metadata (title, body,
// labels, state) rarely changes during a session, and the lifecycle worker
// polls `getIssue` / `isCompleted` repeatedly — same (repo, id) seen 64+ times
// per 5-session tier-5 run in our traces.
const ISSUE_CACHE_TTL_MS = 5 * 60_000;
const ISSUE_CACHE_MAX = 500;
interface CachedIssue {
issue: Issue;
expiresAt: number;
}
function issueCacheKey(repo: string, identifier: string): string {
return `${repo}#${identifier.replace(/^#/, "")}`;
}
function createGitHubTracker(): Tracker {
return {
const issueCache = new Map<string, CachedIssue>();
const inflight = new Map<string, Promise<Issue>>();
function readCachedIssue(repo: string, identifier: string): Issue | null {
const key = issueCacheKey(repo, identifier);
const entry = issueCache.get(key);
if (!entry) return null;
if (Date.now() > entry.expiresAt) {
issueCache.delete(key);
return null;
}
return entry.issue;
}
function writeCachedIssue(repo: string, identifier: string, issue: Issue): void {
if (issueCache.size >= ISSUE_CACHE_MAX) {
const oldest = issueCache.keys().next().value;
if (oldest !== undefined) issueCache.delete(oldest);
}
issueCache.set(issueCacheKey(repo, identifier), {
issue,
expiresAt: Date.now() + ISSUE_CACHE_TTL_MS,
});
}
function invalidateCachedIssue(repo: string, identifier: string): void {
issueCache.delete(issueCacheKey(repo, identifier));
}
const tracker: Tracker = {
name: "github",
async getIssue(identifier: string, project: ProjectConfig): Promise<Issue> {
const raw = await ghIssueViewJson(identifier, project);
const repo = requireRepo(project);
const cached = readCachedIssue(repo, identifier);
if (cached) return cached;
const data: {
number: number;
title: string;
body: string;
url: string;
state: string;
stateReason?: string | null;
labels: Array<{ name: string }>;
assignees: Array<{ login: string }>;
} = JSON.parse(raw);
// Deduplicate concurrent requests for the same issue
const key = issueCacheKey(repo, identifier);
const pending = inflight.get(key);
if (pending) return pending;
return {
id: String(data.number),
title: data.title,
description: data.body ?? "",
url: data.url,
state: mapState(data.state, data.stateReason),
labels: data.labels.map((l) => l.name),
assignee: data.assignees[0]?.login,
};
const promise = (async () => {
const raw = await ghIssueViewJson(identifier, project);
const data: {
number: number;
title: string;
body: string;
url: string;
state: string;
stateReason?: string | null;
labels: Array<{ name: string }>;
assignees: Array<{ login: string }>;
} = JSON.parse(raw);
const issue: Issue = {
id: String(data.number),
title: data.title,
description: data.body ?? "",
url: data.url,
state: mapState(data.state, data.stateReason),
labels: data.labels.map((l) => l.name),
assignee: data.assignees[0]?.login,
};
writeCachedIssue(repo, identifier, issue);
return issue;
})();
inflight.set(key, promise);
try {
return await promise;
} finally {
inflight.delete(key);
}
},
async isCompleted(identifier: string, project: ProjectConfig): Promise<boolean> {
const raw = await gh([
"issue",
"view",
identifier,
"--repo",
requireRepo(project),
"--json",
"state",
]);
const data: { state: string } = JSON.parse(raw);
return data.state.toUpperCase() === "CLOSED";
// Route through getIssue so the cache covers the hot isCompleted poll path too.
// "closed" and "cancelled" (CLOSED + NOT_PLANNED stateReason) both count as completed.
const issue = await tracker.getIssue(identifier, project);
return issue.state === "closed" || issue.state === "cancelled";
},
issueUrl(identifier: string, project: ProjectConfig): string {
@ -204,6 +256,8 @@ function createGitHubTracker(): Tracker {
}
lines.push(
"",
"The issue context above is complete and current. You should not need to call gh issue view unless you need additional context beyond what is provided here.",
"",
"Please implement the changes described in this issue. When done, commit and push your changes.",
);
@ -266,6 +320,8 @@ function createGitHubTracker(): Tracker {
project: ProjectConfig,
): Promise<void> {
const repo = requireRepo(project);
// Any mutation invalidates the cached Issue for this (repo, identifier).
invalidateCachedIssue(repo, identifier);
// Handle state change — GitHub Issues only supports open/closed.
// "in_progress" is not a GitHub state, so it is intentionally a no-op.
if (update.state === "closed") {
@ -357,9 +413,11 @@ function createGitHubTracker(): Tracker {
}
const number = match[1];
return this.getIssue(number, project);
return tracker.getIssue(number, project);
},
};
return tracker;
}
// ---------------------------------------------------------------------------

133
packages/plugins/tracker-github/test/index.test.ts
View File

@ -169,21 +169,122 @@ describe("tracker-github plugin", () => {
describe("isCompleted", () => {
it("returns true for CLOSED issues", async () => {
mockGh({ state: "CLOSED" });
mockGh({ ...sampleIssue, state: "CLOSED", stateReason: "COMPLETED" });
expect(await tracker.isCompleted("123", project)).toBe(true);
});
it("returns false for OPEN issues", async () => {
mockGh({ state: "OPEN" });
mockGh({ ...sampleIssue, state: "OPEN" });
expect(await tracker.isCompleted("123", project)).toBe(false);
});
it("handles lowercase state", async () => {
mockGh({ state: "closed" });
it("returns true for cancelled (CLOSED + NOT_PLANNED) issues", async () => {
mockGh({ ...sampleIssue, state: "CLOSED", stateReason: "NOT_PLANNED" });
expect(await tracker.isCompleted("123", project)).toBe(true);
});
});
// ---- issue cache -------------------------------------------------------
describe("issue cache", () => {
it("second getIssue for same (repo, id) hits cache — no gh call", async () => {
mockGh(sampleIssue);
const first = await tracker.getIssue("123", project);
const second = await tracker.getIssue("123", project);
expect(first).toEqual(second);
expect(ghMock).toHaveBeenCalledTimes(1);
});
it("isCompleted shares the same cache as getIssue", async () => {
mockGh(sampleIssue);
await tracker.getIssue("123", project);
const done = await tracker.isCompleted("123", project);
expect(done).toBe(false);
expect(ghMock).toHaveBeenCalledTimes(1);
});
it("different identifiers cache independently", async () => {
mockGh(sampleIssue);
mockGh({ ...sampleIssue, number: 456 });
await tracker.getIssue("123", project);
await tracker.getIssue("456", project);
expect(ghMock).toHaveBeenCalledTimes(2);
// Both now cached — no new gh calls
await tracker.getIssue("123", project);
await tracker.getIssue("456", project);
expect(ghMock).toHaveBeenCalledTimes(2);
});
it("different repos cache independently", async () => {
const otherProject: ProjectConfig = { ...project, repo: "acme/other" };
mockGh(sampleIssue);
mockGh(sampleIssue);
await tracker.getIssue("123", project);
await tracker.getIssue("123", otherProject);
expect(ghMock).toHaveBeenCalledTimes(2);
});
it("strips '#' prefix — '#123' and '123' share a cache entry", async () => {
mockGh(sampleIssue);
await tracker.getIssue("#123", project);
await tracker.getIssue("123", project);
expect(ghMock).toHaveBeenCalledTimes(1);
});
it("updateIssue invalidates the cache entry", async () => {
mockGh(sampleIssue);
await tracker.getIssue("123", project);
expect(ghMock).toHaveBeenCalledTimes(1);
ghMock.mockResolvedValueOnce({ stdout: "" }); // gh issue close
await tracker.updateIssue!("123", { state: "closed" }, project);
// Next getIssue must re-fetch from gh
mockGh({ ...sampleIssue, state: "CLOSED", stateReason: "COMPLETED" });
const fresh = await tracker.getIssue("123", project);
expect(fresh.state).toBe("closed");
expect(ghMock).toHaveBeenCalledTimes(3); // view + close + view again
});
it("entries expire after TTL", async () => {
vi.useFakeTimers();
try {
mockGh(sampleIssue);
await tracker.getIssue("123", project);
expect(ghMock).toHaveBeenCalledTimes(1);
// Advance time past TTL (5 min)
vi.advanceTimersByTime(5 * 60_000 + 1);
mockGh(sampleIssue);
await tracker.getIssue("123", project);
expect(ghMock).toHaveBeenCalledTimes(2);
} finally {
vi.useRealTimers();
}
});
it("each create() returns a tracker with an isolated cache", async () => {
const trackerA = create();
const trackerB = create();
mockGh(sampleIssue);
await trackerA.getIssue("123", project);
mockGh(sampleIssue);
await trackerB.getIssue("123", project);
expect(ghMock).toHaveBeenCalledTimes(2);
});
it("failures are not cached", async () => {
mockGhError("issue not found");
await expect(tracker.getIssue("999", project)).rejects.toThrow();
// Next call must re-try, not serve a stale error
mockGh({ ...sampleIssue, number: 999 });
const issue = await tracker.getIssue("999", project);
expect(issue.id).toBe("999");
expect(ghMock).toHaveBeenCalledTimes(2);
});
});
// ---- issueUrl ----------------------------------------------------------
describe("issueUrl", () => {
@ -259,7 +360,7 @@ describe("tracker-github plugin", () => {
mockGh([]);
await tracker.listIssues!({ state: "closed" }, project);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
expect.arrayContaining(["--state", "closed"]),
expect.any(Object),
);
@ -269,7 +370,7 @@ describe("tracker-github plugin", () => {
mockGh([]);
await tracker.listIssues!({ state: "all" }, project);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
expect.arrayContaining(["--state", "all"]),
expect.any(Object),
);
@ -279,7 +380,7 @@ describe("tracker-github plugin", () => {
mockGh([]);
await tracker.listIssues!({}, project);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
expect.arrayContaining(["--state", "open"]),
expect.any(Object),
);
@ -289,7 +390,7 @@ describe("tracker-github plugin", () => {
mockGh([]);
await tracker.listIssues!({ labels: ["bug", "urgent"] }, project);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
expect.arrayContaining(["--label", "bug,urgent"]),
expect.any(Object),
);
@ -299,7 +400,7 @@ describe("tracker-github plugin", () => {
mockGh([]);
await tracker.listIssues!({ assignee: "alice" }, project);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
expect.arrayContaining(["--assignee", "alice"]),
expect.any(Object),
);
@ -309,7 +410,7 @@ describe("tracker-github plugin", () => {
mockGh([]);
await tracker.listIssues!({ limit: 5 }, project);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
expect.arrayContaining(["--limit", "5"]),
expect.any(Object),
);
@ -350,7 +451,7 @@ describe("tracker-github plugin", () => {
ghMock.mockResolvedValueOnce({ stdout: "" });
await tracker.updateIssue!("123", { state: "closed" }, project);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
["issue", "close", "123", "--repo", "acme/repo"],
expect.any(Object),
);
@ -360,7 +461,7 @@ describe("tracker-github plugin", () => {
ghMock.mockResolvedValueOnce({ stdout: "" });
await tracker.updateIssue!("123", { state: "open" }, project);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
["issue", "reopen", "123", "--repo", "acme/repo"],
expect.any(Object),
);
@ -370,7 +471,7 @@ describe("tracker-github plugin", () => {
ghMock.mockResolvedValueOnce({ stdout: "" });
await tracker.updateIssue!("123", { labels: ["bug", "urgent"] }, project);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
["issue", "edit", "123", "--repo", "acme/repo", "--add-label", "bug,urgent"],
expect.any(Object),
);
@ -380,7 +481,7 @@ describe("tracker-github plugin", () => {
ghMock.mockResolvedValueOnce({ stdout: "" });
await tracker.updateIssue!("123", { assignee: "bob" }, project);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
["issue", "edit", "123", "--repo", "acme/repo", "--add-assignee", "bob"],
expect.any(Object),
);
@ -390,7 +491,7 @@ describe("tracker-github plugin", () => {
ghMock.mockResolvedValueOnce({ stdout: "" });
await tracker.updateIssue!("123", { comment: "Working on this" }, project);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
["issue", "comment", "123", "--repo", "acme/repo", "--body", "Working on this"],
expect.any(Object),
);
@ -451,7 +552,7 @@ describe("tracker-github plugin", () => {
project,
);
expect(ghMock).toHaveBeenCalledWith(
"gh",
expect.stringMatching(/(?:^|\/)?gh$/),
expect.arrayContaining(["issue", "create", "--label", "bug", "--assignee", "alice"]),
expect.any(Object),
);

10
packages/web/server/mux-websocket.ts
View File

@ -20,7 +20,7 @@ import { findTmux, resolveTmuxSession, validateSessionId } from "./tmux-utils.js
type ClientMessage =
| { ch: "terminal"; id: string; type: "data"; data: string }
| { ch: "terminal"; id: string; type: "resize"; cols: number; rows: number }
| { ch: "terminal"; id: string; type: "open" }
| { ch: "terminal"; id: string; type: "open"; tmuxName?: string }
| { ch: "terminal"; id: string; type: "close" }
| { ch: "system"; type: "ping" }
| { ch: "subscribe"; topics: ("sessions")[] };
@ -246,13 +246,15 @@ class TerminalManager {
* Open/attach to a terminal. If already open, just return.
* If has subscribers but PTY crashed, re-attach.
*/
open(id: string): string {
open(id: string, tmuxName?: string): string {
// Validate and resolve
if (!validateSessionId(id)) {
throw new Error(`Invalid session ID: ${id}`);
}
const tmuxSessionId = resolveTmuxSession(id, this.TMUX);
// Use provided tmuxName, or reuse from existing terminal entry, or resolve
const existing = this.terminals.get(id);
const tmuxSessionId = tmuxName ?? existing?.tmuxSessionId ?? resolveTmuxSession(id, this.TMUX);
if (!tmuxSessionId) {
throw new Error(`Session not found: ${id}`);
}
@ -499,7 +501,7 @@ export function createMuxWebSocket(tmuxPath?: string): WebSocketServer | null {
try {
if (type === "open") {
// Validate session exists
terminalManager.open(id);
terminalManager.open(id, "tmuxName" in msg ? msg.tmuxName : undefined);
// Send opened confirmation (idempotent — safe to send on re-open)
const openedMsg: ServerMessage = { ch: "terminal", id, type: "opened" };

8
packages/web/server/tmux-utils.ts
View File

@ -161,14 +161,6 @@ export function resolveTmuxSession(
// even when the storageKey is wrapped (`{hash}-{projectName}`). Walk
// every candidate so a stale metadata dir in one project can't shadow
// the live session of another project with the same sessionId.
//
// Pre-existing limitation (not introduced by this fix): when two
// different projects have both (a) the same user-facing sessionId and
// (b) live tmux sessions, we return the first live match. The terminal
// open protocol carries only the session ID — no project context — so
// there's nothing to disambiguate with here. The previous bare-hash
// code had the same behavior via `Array.find(...)`. A proper fix would
// thread projectId through the mux protocol from the browser URL.
for (const storageKey of findStorageKeysForSession(sessionId, fs)) {
const tmuxName = `${storageKey}-${sessionId}`;
try {

26
packages/web/src/app/api/sessions/[id]/route.ts
View File

@ -1,6 +1,6 @@
import { type NextRequest } from "next/server";
import { getSessionsDir, readAgentReportAuditTrailAsync } from "@aoagents/ao-core";
import { getServices, getSCM } from "@/lib/services";
import { getServices } from "@/lib/services";
import {
sessionToDashboard,
resolveProject,
@ -12,8 +12,6 @@ import { getCorrelationId, jsonWithCorrelation, recordApiObservation } from "@/l
const AGENT_REPORT_AUDIT_TIMEOUT_MS = 1000;
const METADATA_ENRICH_TIMEOUT_MS = 3000;
const PR_CACHE_ENRICH_TIMEOUT_MS = 1000;
const PR_LIVE_ENRICH_TIMEOUT_MS = 2000;
export async function GET(_request: NextRequest, { params }: { params: Promise<{ id: string }> }) {
const correlationId = getCorrelationId(_request);
@ -43,27 +41,9 @@ export async function GET(_request: NextRequest, { params }: { params: Promise<{
METADATA_ENRICH_TIMEOUT_MS,
);
// Enrich PR — serve cache immediately, refresh in background if stale
// Enrich PR from session metadata (written by CLI lifecycle)
if (coreSession.pr) {
const scm = getSCM(registry, project);
if (scm) {
let cached = false;
const cachedSettled = await settlesWithin(
enrichSessionPR(dashboardSession, scm, coreSession.pr, {
cacheOnly: true,
}).then((result) => {
cached = result;
}),
PR_CACHE_ENRICH_TIMEOUT_MS,
);
if (!cached) {
// Nothing cached yet — block once to populate, then future calls use cache
await settlesWithin(
enrichSessionPR(dashboardSession, scm, coreSession.pr),
cachedSettled ? PR_LIVE_ENRICH_TIMEOUT_MS : PR_CACHE_ENRICH_TIMEOUT_MS + PR_LIVE_ENRICH_TIMEOUT_MS,
);
}
}
enrichSessionPR(dashboardSession);
}
recordApiObservation({

41
packages/web/src/app/api/sessions/route.ts
View File

@ -1,8 +1,7 @@
import { ACTIVITY_STATE, isOrchestratorSession, isTerminalSession } from "@aoagents/ao-core";
import { getServices, getSCM } from "@/lib/services";
import { getServices } from "@/lib/services";
import {
sessionToDashboard,
resolveProject,
enrichSessionPR,
enrichSessionsMetadata,
computeStats,
@ -14,13 +13,6 @@ import { settlesWithin } from "@/lib/async-utils";
import type { DashboardOrchestratorLink } from "@/lib/types";
const METADATA_ENRICH_TIMEOUT_MS = 3_000;
const PR_ENRICH_TIMEOUT_MS = 4_000;
const PER_PR_ENRICH_TIMEOUT_MS = 1_500;
function hasTerminalPRState(session: Parameters<typeof isTerminalSession>[0]): boolean {
const prState = session.lifecycle?.pr.state;
return prState === "merged" || prState === "closed";
}
function compareOrchestratorRecency(a: { lastActivityAt?: Date | null; createdAt?: Date | null; id: string }, b: { lastActivityAt?: Date | null; createdAt?: Date | null; id: string }): number {
return (
@ -152,34 +144,11 @@ export async function GET(request: Request) {
);
if (metadataSettled) {
const prEnrichPromises: Promise<boolean>[] = [];
// PR enrichment: read from session metadata (written by CLI lifecycle).
// No GitHub API calls — synchronous metadata read.
for (let i = 0; i < workerSessions.length; i++) {
const core = workerSessions[i];
const pr = core?.pr;
if (!pr) continue;
const project = resolveProject(core, config.projects);
const scm = getSCM(registry, project);
if (!scm) continue;
prEnrichPromises.push(
settlesWithin(
hasTerminalPRState(core)
? enrichSessionPR(dashboardSessions[i], scm, pr, { cacheOnly: true }).then(
(cached) =>
cached
? true
: enrichSessionPR(dashboardSessions[i], scm, pr),
)
: enrichSessionPR(dashboardSessions[i], scm, pr),
PER_PR_ENRICH_TIMEOUT_MS,
),
);
}
if (prEnrichPromises.length > 0) {
await settlesWithin(Promise.allSettled(prEnrichPromises), PR_ENRICH_TIMEOUT_MS);
if (!workerSessions[i]?.pr) continue;
enrichSessionPR(dashboardSessions[i]);
}
}

5
packages/web/src/components/DirectTerminal.tsx
View File

@ -72,6 +72,8 @@ function getStoredFontSize(): number {
interface DirectTerminalProps {
sessionId: string;
/** Actual tmux session name. When provided, the terminal server uses it directly instead of resolving from sessionId. */
tmuxName?: string;
startFullscreen?: boolean;
/** Visual variant. Orchestrator keeps the same design-system blue accent as the rest of the app. */
variant?: "agent" | "orchestrator";
@ -164,6 +166,7 @@ export function buildTerminalThemes(variant: TerminalVariant): { dark: ITheme; l
*/
export function DirectTerminal({
sessionId,
tmuxName,
startFullscreen = false,
variant = "agent",
appearance = "theme",
@ -474,7 +477,7 @@ export function DirectTerminal({
});
// Open terminal via mux
openTerminal(sessionId);
openTerminal(sessionId, tmuxName);
// Subscribe to terminal data via mux
unsubscribe = subscribeTerminal(sessionId, (data) => {

1
packages/web/src/components/SessionDetail.tsx
View File

@ -651,6 +651,7 @@ export function SessionDetail({
) : (
<DirectTerminal
sessionId={session.id}
tmuxName={session.metadata?.tmuxName}
startFullscreen={startFullscreen}
variant={terminalVariant}
appearance="dark"

4
packages/web/src/components/__tests__/Dashboard.doneBar.test.tsx
View File

@ -60,10 +60,10 @@ describe("Dashboard done bar", () => {
expect(screen.queryByText(/No active sessions/i)).not.toBeInTheDocument();
});
it("does not render a restore action for merged sessions", () => {
it("renders a restore action for merged sessions", () => {
render(<Dashboard initialSessions={[DONE_SESSION]} />);
const toggle = screen.getByText("Done / Terminated").closest("button")!;
fireEvent.click(toggle);
expect(screen.queryByRole("button", { name: /restore/i })).toBeNull();
expect(screen.queryByRole("button", { name: /restore/i })).toBeInTheDocument();
});
});

12
packages/web/src/lib/__tests__/dashboard-page-data.fast-path.test.ts
View File

@ -94,15 +94,9 @@ describe("getDashboardPageData fast path", () => {
{ projects: { docs: { id: "docs" } } },
{ scm: "registry" },
);
expect(hoisted.enrichSessionPRMock).toHaveBeenCalledTimes(1);
expect(hoisted.enrichSessionPRMock).toHaveBeenCalledWith(
dashboardMerged,
{ provider: "github" },
mergedCore.pr,
{ cacheOnly: true },
);
expect(dashboardClosed.pr.state).toBe("closed");
expect(dashboardMerged.pr.state).toBe("merged");
expect(hoisted.enrichSessionPRMock).toHaveBeenCalledTimes(2);
expect(hoisted.enrichSessionPRMock).toHaveBeenCalledWith(dashboardClosed);
expect(hoisted.enrichSessionPRMock).toHaveBeenCalledWith(dashboardMerged);
expect(pageData.sessions).toEqual([dashboardNoPr, dashboardClosed, dashboardMerged]);
});

336
packages/web/src/lib/__tests__/serialize.test.ts
View File

@ -2,12 +2,11 @@
* Tests for session serialization and PR enrichment
*/
import { describe, it, expect, beforeEach, vi } from "vitest";
import { describe, it, expect, vi } from "vitest";
import {
createInitialCanonicalLifecycle,
type Session,
type PRInfo,
type SCM,
type Agent,
type Tracker,
type ProjectConfig,
@ -18,7 +17,7 @@ import {
sessionToDashboard,
resolveProject,
enrichSessionPR,
enrichSessionIssue,
readPREnrichmentFromMetadata,
enrichSessionAgentSummary,
enrichSessionIssueTitle,
enrichSessionsMetadata,
@ -26,7 +25,6 @@ import {
computeStats,
listDashboardOrchestrators,
} from "../serialize";
import { prCache, prCacheKey } from "../cache";
import type { DashboardSession } from "../types";
// Helper to create a minimal Session for testing
@ -77,56 +75,32 @@ function createPRInfo(overrides?: Partial<PRInfo>): PRInfo {
};
}
// Mock SCM that succeeds
function createMockSCM(): SCM {
return {
name: "mock",
detectPR: vi.fn(),
getPRState: vi.fn().mockResolvedValue("open"),
getPRSummary: vi.fn().mockResolvedValue({
state: "open",
title: "Test PR",
additions: 100,
deletions: 50,
}),
getCIChecks: vi
.fn()
.mockResolvedValue([{ name: "test", status: "passed", url: "https://example.com" }]),
getCISummary: vi.fn().mockResolvedValue("passing"),
getReviewDecision: vi.fn().mockResolvedValue("approved"),
getMergeability: vi.fn().mockResolvedValue({
mergeable: true,
ciPassing: true,
approved: true,
noConflicts: true,
blockers: [],
}),
getPendingComments: vi.fn().mockResolvedValue([]),
getReviews: vi.fn(),
getAutomatedComments: vi.fn(),
mergePR: vi.fn(),
closePR: vi.fn(),
};
// Helper to create prEnrichment metadata JSON
function createEnrichmentMetadata(overrides?: Record<string, unknown>): string {
return JSON.stringify({
state: "open",
title: "Test PR",
additions: 100,
deletions: 50,
ciStatus: "passing",
reviewDecision: "approved",
mergeable: true,
isDraft: false,
hasConflicts: false,
ciChecks: [{ name: "test", status: "passed", url: "https://example.com" }],
enrichedAt: new Date().toISOString(),
...overrides,
});
}
// Mock SCM that fails all requests
function createFailingSCM(): SCM {
const error = new Error("API rate limited");
return {
name: "mock-failing",
detectPR: vi.fn(),
getPRState: vi.fn().mockRejectedValue(error),
getPRSummary: vi.fn().mockRejectedValue(error),
getCIChecks: vi.fn().mockRejectedValue(error),
getCISummary: vi.fn().mockRejectedValue(error),
getReviewDecision: vi.fn().mockRejectedValue(error),
getMergeability: vi.fn().mockRejectedValue(error),
getPendingComments: vi.fn().mockRejectedValue(error),
getReviews: vi.fn(),
getAutomatedComments: vi.fn(),
mergePR: vi.fn(),
closePR: vi.fn(),
};
// Helper to create prReviewComments metadata JSON
function createReviewCommentsMetadata(overrides?: Record<string, unknown>): string {
return JSON.stringify({
unresolvedThreads: 0,
unresolvedComments: [],
commentsUpdatedAt: new Date().toISOString(),
...overrides,
});
}
describe("sessionToDashboard", () => {
@ -400,18 +374,20 @@ describe("resolveProject", () => {
});
describe("enrichSessionPR", () => {
beforeEach(() => {
prCache.clear();
});
it("should enrich PR with live SCM data", async () => {
it("should enrich PR from metadata", () => {
const pr = createPRInfo();
const coreSession = createCoreSession({ pr });
const coreSession = createCoreSession({
pr,
metadata: {
prEnrichment: createEnrichmentMetadata(),
prReviewComments: createReviewCommentsMetadata(),
},
});
const dashboard = sessionToDashboard(coreSession);
const scm = createMockSCM();
await enrichSessionPR(dashboard, scm, pr);
const result = enrichSessionPR(dashboard);
expect(result).toBe(true);
expect(dashboard.pr?.state).toBe("open");
expect(dashboard.pr?.additions).toBe(100);
expect(dashboard.pr?.deletions).toBe(50);
@ -420,106 +396,75 @@ describe("enrichSessionPR", () => {
expect(dashboard.pr?.mergeability.mergeable).toBe(true);
expect(dashboard.pr?.ciChecks).toHaveLength(1);
expect(dashboard.pr?.ciChecks[0]?.name).toBe("test");
expect(dashboard.pr?.enriched).toBe(true);
});
it("should cache successful enrichment results", async () => {
it("should return false when prEnrichment metadata is missing", () => {
const pr = createPRInfo();
const coreSession = createCoreSession({ pr });
const dashboard = sessionToDashboard(coreSession);
const scm = createMockSCM();
await enrichSessionPR(dashboard, scm, pr);
const cacheKey = prCacheKey(pr.owner, pr.repo, pr.number);
const cached = prCache.get(cacheKey);
expect(cached).not.toBeNull();
expect(cached?.additions).toBe(100);
expect(cached?.deletions).toBe(50);
});
it("should use cached data on subsequent calls", async () => {
const pr = createPRInfo();
const coreSession = createCoreSession({ pr });
const dashboard1 = sessionToDashboard(coreSession);
const dashboard2 = sessionToDashboard(coreSession);
const scm = createMockSCM();
// First call: fetch from SCM
await enrichSessionPR(dashboard1, scm, pr);
expect(scm.getPRSummary).toHaveBeenCalledTimes(1);
// Second call: use cache
await enrichSessionPR(dashboard2, scm, pr);
expect(scm.getPRSummary).toHaveBeenCalledTimes(1); // Still 1, not 2
expect(dashboard2.pr?.additions).toBe(100);
});
it("should handle rate limit errors gracefully", async () => {
const pr = createPRInfo();
const coreSession = createCoreSession({ pr });
const dashboard = sessionToDashboard(coreSession);
const scm = createFailingSCM();
// Spy on console.warn (enrichSessionPR uses warn for rate-limit, not error)
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
await enrichSessionPR(dashboard, scm, pr);
// Should keep default values but update blocker message
expect(dashboard.pr?.additions).toBe(0);
expect(dashboard.pr?.deletions).toBe(0);
expect(dashboard.pr?.mergeability.blockers).toContain("API rate limited or unavailable");
// Should log warning
expect(consoleWarnSpy).toHaveBeenCalled();
consoleWarnSpy.mockRestore();
});
it("should cache even when most requests fail (to reduce API pressure)", async () => {
const pr = createPRInfo();
const coreSession = createCoreSession({ pr });
const dashboard = sessionToDashboard(coreSession);
const scm = createFailingSCM();
await enrichSessionPR(dashboard, scm, pr);
// Even with all failures, we cache the default/partial data to prevent repeated API hits
const cacheKey = prCacheKey(pr.owner, pr.repo, pr.number);
const cached = prCache.get(cacheKey);
expect(cached).not.toBeNull();
expect(cached?.mergeability.blockers).toContain("API rate limited or unavailable");
});
it("should handle partial failures gracefully", async () => {
const pr = createPRInfo();
const coreSession = createCoreSession({ pr });
const coreSession = createCoreSession({ pr, metadata: {} });
const dashboard = sessionToDashboard(coreSession);
// Mock SCM with partial failures
const scm: SCM = {
...createMockSCM(),
getCISummary: vi.fn().mockRejectedValue(new Error("CI API failed")),
getMergeability: vi.fn().mockRejectedValue(new Error("Merge API failed")),
};
const result = enrichSessionPR(dashboard);
await enrichSessionPR(dashboard, scm, pr);
// Successful fields should be populated
expect(dashboard.pr?.additions).toBe(100);
expect(dashboard.pr?.deletions).toBe(50);
expect(dashboard.pr?.reviewDecision).toBe("approved");
// Failed fields should have graceful defaults
expect(dashboard.pr?.mergeability.blockers).toContain("Merge status unavailable");
// Should still cache partial results
const cacheKey = prCacheKey(pr.owner, pr.repo, pr.number);
const cached = prCache.get(cacheKey);
expect(cached).not.toBeNull();
expect(result).toBe(false);
expect(dashboard.pr?.enriched).toBe(false);
});
it("should do nothing if dashboard.pr is null", async () => {
it("should enrich review comments from metadata", () => {
const pr = createPRInfo();
const coreSession = createCoreSession({
pr,
metadata: {
prEnrichment: createEnrichmentMetadata(),
prReviewComments: createReviewCommentsMetadata({
unresolvedThreads: 2,
unresolvedComments: [
{ url: "https://example.com", path: "src/app.ts", author: "reviewer", body: "Fix this" },
],
}),
},
});
const dashboard = sessionToDashboard(coreSession);
enrichSessionPR(dashboard);
expect(dashboard.pr?.unresolvedThreads).toBe(2);
expect(dashboard.pr?.unresolvedComments).toHaveLength(1);
expect(dashboard.pr?.unresolvedComments[0]?.author).toBe("reviewer");
});
it("should handle missing review comments metadata gracefully", () => {
const pr = createPRInfo();
const coreSession = createCoreSession({
pr,
metadata: {
prEnrichment: createEnrichmentMetadata(),
// No prReviewComments
},
});
const dashboard = sessionToDashboard(coreSession);
enrichSessionPR(dashboard);
expect(dashboard.pr?.unresolvedThreads).toBe(0);
expect(dashboard.pr?.unresolvedComments).toEqual([]);
});
it("should handle malformed prEnrichment JSON gracefully", () => {
const pr = createPRInfo();
const coreSession = createCoreSession({
pr,
metadata: { prEnrichment: "not valid json{" },
});
const dashboard = sessionToDashboard(coreSession);
const result = enrichSessionPR(dashboard);
expect(result).toBe(false);
expect(dashboard.pr?.enriched).toBe(false);
});
it("should do nothing if dashboard.pr is null", () => {
const dashboard: DashboardSession = {
id: "test-1",
projectId: "test",
@ -541,32 +486,87 @@ describe("enrichSessionPR", () => {
createdAt: new Date().toISOString(),
lastActivityAt: new Date().toISOString(),
pr: null,
metadata: {},
metadata: { prEnrichment: createEnrichmentMetadata() },
};
const pr = createPRInfo();
const scm = createMockSCM();
await enrichSessionPR(dashboard, scm, pr);
const result = enrichSessionPR(dashboard);
expect(scm.getPRSummary).not.toHaveBeenCalled();
expect(result).toBe(false);
});
it("should handle missing optional SCM methods", async () => {
it("should derive mergeability fields from enrichment data", () => {
const pr = createPRInfo();
const coreSession = createCoreSession({ pr });
const coreSession = createCoreSession({
pr,
metadata: {
prEnrichment: createEnrichmentMetadata({
ciStatus: "failing",
reviewDecision: "changes_requested",
mergeable: false,
hasConflicts: true,
}),
},
});
const dashboard = sessionToDashboard(coreSession);
// SCM without getPRSummary
const scm: SCM = {
...createMockSCM(),
getPRSummary: undefined,
};
enrichSessionPR(dashboard);
await enrichSessionPR(dashboard, scm, pr);
expect(dashboard.pr?.mergeability.mergeable).toBe(false);
expect(dashboard.pr?.mergeability.ciPassing).toBe(false);
expect(dashboard.pr?.mergeability.approved).toBe(false);
expect(dashboard.pr?.mergeability.noConflicts).toBe(false);
});
// Should fall back to getPRState
expect(scm.getPRState).toHaveBeenCalled();
expect(dashboard.pr?.state).toBe("open");
it("should set enriched flag on successful enrichment", () => {
const pr = createPRInfo();
const coreSession = createCoreSession({
pr,
metadata: { prEnrichment: createEnrichmentMetadata() },
});
const dashboard = sessionToDashboard(coreSession);
expect(dashboard.pr?.enriched).toBe(false);
enrichSessionPR(dashboard);
expect(dashboard.pr?.enriched).toBe(true);
});
});
describe("readPREnrichmentFromMetadata", () => {
it("should parse valid enrichment metadata", () => {
const data = readPREnrichmentFromMetadata({
prEnrichment: createEnrichmentMetadata(),
});
expect(data).not.toBeNull();
expect(data?.state).toBe("open");
expect(data?.additions).toBe(100);
expect(data?.deletions).toBe(50);
});
it("should return null when prEnrichment is missing", () => {
const data = readPREnrichmentFromMetadata({});
expect(data).toBeNull();
});
it("should return null for invalid JSON", () => {
const data = readPREnrichmentFromMetadata({ prEnrichment: "bad json{" });
expect(data).toBeNull();
});
it("should include review comments when present", () => {
const data = readPREnrichmentFromMetadata({
prEnrichment: createEnrichmentMetadata(),
prReviewComments: createReviewCommentsMetadata({
unresolvedThreads: 3,
unresolvedComments: [
{ url: "https://example.com", path: "src/app.ts", author: "alice", body: "Please fix" },
],
}),
});
expect(data?.unresolvedThreads).toBe(3);
expect(data?.unresolvedComments).toHaveLength(1);
});
});

3
packages/web/src/lib/cache.ts
View File

@ -105,9 +105,6 @@ export interface PREnrichmentData {
}>;
}
/** Global PR enrichment cache (60s TTL) */
export const prCache = new TTLCache<PREnrichmentData>();
/** Generate cache key for a PR: `owner/repo#123` */
export function prCacheKey(owner: string, repo: string, number: number): string {
return `${owner}/${repo}#${number}`;

22
packages/web/src/lib/dashboard-page-data.ts
View File

@ -6,10 +6,9 @@ import {
type DashboardOrchestratorLink,
type DashboardAttentionZoneMode,
} from "@/lib/types";
import { getServices, getSCM } from "@/lib/services";
import { getServices } from "@/lib/services";
import {
sessionToDashboard,
resolveProject,
enrichSessionPR,
enrichSessionsMetadataFast,
listDashboardOrchestrators,
@ -126,23 +125,10 @@ export const getDashboardPageData = cache(async function getDashboardPageData(pr
console.warn("[dashboard-page-data] metadata fast enrichment failed:", err);
}
// PR cache hits only (in-memory lookup, no SCM API calls).
// PR enrichment from session metadata (no API calls).
for (let i = 0; i < coreSessions.length; i++) {
const core = coreSessions[i];
if (!core.pr) continue;
try {
const projectConfig = resolveProject(core, config.projects);
const scm = getSCM(registry, projectConfig);
if (scm) {
try {
await enrichSessionPR(pageData.sessions[i], scm, core.pr, { cacheOnly: true });
} catch {
// Preserve the base session payload if PR enrichment fails.
}
}
} catch (err) {
console.warn(`[dashboard-page-data] PR enrichment failed for session ${core.id}:`, err);
}
if (!coreSessions[i].pr) continue;
enrichSessionPR(pageData.sessions[i]);
}
return pageData;

2
packages/web/src/lib/mux-protocol.ts
View File

@ -5,7 +5,7 @@ import type { AttentionLevel } from "./types";
export type ClientMessage =
| { ch: "terminal"; id: string; type: "data"; data: string }
| { ch: "terminal"; id: string; type: "resize"; cols: number; rows: number }
| { ch: "terminal"; id: string; type: "open" }
| { ch: "terminal"; id: string; type: "open"; tmuxName?: string }
| { ch: "terminal"; id: string; type: "close" }
| { ch: "system"; type: "ping" }
| { ch: "subscribe"; topics: ("sessions")[] };

226
packages/web/src/lib/serialize.ts
View File

@ -12,7 +12,6 @@ import {
isTerminalSession,
type Session,
type Agent,
type SCM,
type PRInfo,
type Tracker,
type ProjectConfig,
@ -26,7 +25,7 @@ import {
type DashboardOrchestratorLink,
getAttentionLevel,
} from "./types";
import { TTLCache, prCache, prCacheKey, type PREnrichmentData } from "./cache";
import { TTLCache, type PREnrichmentData } from "./cache";
/** Cache for issue titles (5 min TTL — issue titles rarely change) */
const issueTitleCache = new TTLCache<string>(300_000);
@ -302,159 +301,86 @@ function normalizeDashboardPRState(
* Enrich a DashboardSession's PR with live data from the SCM plugin.
* Uses cache to reduce API calls and handles rate limit errors gracefully.
*/
export async function enrichSessionPR(
/**
* Read PR enrichment data from session metadata.
* The CLI lifecycle manager writes this data every 30s (batch enrichment)
* and every 2min (review comments). Returns null if not available.
*/
export function readPREnrichmentFromMetadata(
metadata: Record<string, string>,
): PREnrichmentData | null {
const enrichmentRaw = metadata["prEnrichment"];
if (!enrichmentRaw) return null;
try {
const e = JSON.parse(enrichmentRaw);
let reviewData = {
unresolvedThreads: 0,
unresolvedComments: [] as Array<{ url: string; path: string; author: string; body: string }>,
};
const reviewRaw = metadata["prReviewComments"];
if (reviewRaw) {
try {
reviewData = JSON.parse(reviewRaw);
} catch {
/* use defaults */
}
}
return {
state: e.state ?? "open",
title: e.title ?? "",
additions: e.additions ?? 0,
deletions: e.deletions ?? 0,
ciStatus: e.ciStatus ?? "none",
ciChecks: (e.ciChecks ?? []).map(
(c: { name: string; status: string; url?: string }) => ({
name: c.name,
status: c.status,
url: c.url,
}),
),
reviewDecision: e.reviewDecision ?? "none",
mergeability: {
mergeable: e.mergeable ?? false,
ciPassing: e.ciStatus === "passing" || e.ciStatus === "none",
approved: e.reviewDecision === "approved",
noConflicts: !(e.hasConflicts ?? false),
blockers: e.blockers ?? [],
},
unresolvedThreads: reviewData.unresolvedThreads ?? 0,
unresolvedComments: reviewData.unresolvedComments ?? [],
};
} catch {
return null;
}
}
/**
* Enrich a DashboardSession's PR data from session metadata.
* The CLI lifecycle manager persists batch enrichment data to metadata files.
* No GitHub API calls — reads from disk via the metadata already loaded.
*/
export function enrichSessionPR(
dashboard: DashboardSession,
scm: SCM,
pr: PRInfo,
opts?: { cacheOnly?: boolean },
): Promise<boolean> {
): boolean {
if (!dashboard.pr) return false;
const cacheKey = prCacheKey(pr.owner, pr.repo, pr.number);
const data = readPREnrichmentFromMetadata(dashboard.metadata);
if (!data) return false;
// Check cache first
const cached = prCache.get(cacheKey);
if (cached && dashboard.pr) {
dashboard.pr.state = cached.state;
dashboard.pr.title = cached.title;
dashboard.pr.additions = cached.additions;
dashboard.pr.deletions = cached.deletions;
dashboard.pr.ciStatus = cached.ciStatus;
dashboard.pr.ciChecks = cached.ciChecks;
dashboard.pr.reviewDecision = cached.reviewDecision;
dashboard.pr.mergeability = cached.mergeability;
dashboard.pr.unresolvedThreads = cached.unresolvedThreads;
dashboard.pr.unresolvedComments = cached.unresolvedComments;
dashboard.pr.enriched = true;
refreshDashboardSessionDerivedFields(dashboard);
return true;
}
// Cache miss — if cacheOnly, signal caller to refresh in background
if (opts?.cacheOnly) return false;
// Fetch from SCM
const results = await Promise.allSettled([
scm.getPRSummary
? scm.getPRSummary(pr)
: scm.getPRState(pr).then((state) => ({ state, title: "", additions: 0, deletions: 0 })),
scm.getCIChecks(pr),
scm.getCISummary(pr),
scm.getReviewDecision(pr),
scm.getMergeability(pr),
scm.getPendingComments(pr),
]);
const [summaryR, checksR, ciR, reviewR, mergeR, commentsR] = results;
// Check if most critical requests failed (likely rate limit)
// Note: Some methods (like getCISummary) return fallback values instead of rejecting,
// so we can't rely on "all rejected" — check if majority failed instead
const failedCount = results.filter((r) => r.status === "rejected").length;
const mostFailed = failedCount >= results.length / 2;
if (mostFailed) {
const rejectedResults = results.filter(
(r) => r.status === "rejected",
) as PromiseRejectedResult[];
const firstError = rejectedResults[0]?.reason;
console.warn(
`[enrichSessionPR] ${failedCount}/${results.length} API calls failed for PR #${pr.number} (rate limited or unavailable):`,
String(firstError),
);
// Don't return early — apply any successful results below
}
// Apply successful results
if (summaryR.status === "fulfilled") {
dashboard.pr.state = summaryR.value.state;
dashboard.pr.additions = summaryR.value.additions;
dashboard.pr.deletions = summaryR.value.deletions;
if (summaryR.value.title) {
dashboard.pr.title = summaryR.value.title;
}
}
if (checksR.status === "fulfilled") {
dashboard.pr.ciChecks = checksR.value.map((c) => ({
name: c.name,
status: c.status,
url: c.url,
}));
}
if (ciR.status === "fulfilled") {
dashboard.pr.ciStatus = ciR.value;
}
if (reviewR.status === "fulfilled") {
dashboard.pr.reviewDecision = reviewR.value;
}
if (mergeR.status === "fulfilled") {
dashboard.pr.mergeability = mergeR.value;
} else {
// Mergeability failed — mark as unavailable
dashboard.pr.mergeability.blockers = ["Merge status unavailable"];
}
if (commentsR.status === "fulfilled") {
const comments = commentsR.value;
dashboard.pr.unresolvedThreads = comments.length;
dashboard.pr.unresolvedComments = comments.map((c) => ({
url: c.url,
path: c.path ?? "",
author: c.author,
body: c.body,
}));
}
// Mark as enriched — we attempted SCM API calls and applied whatever succeeded
dashboard.pr.state = data.state;
if (data.title) dashboard.pr.title = data.title;
dashboard.pr.additions = data.additions;
dashboard.pr.deletions = data.deletions;
dashboard.pr.ciStatus = data.ciStatus;
dashboard.pr.ciChecks = data.ciChecks;
dashboard.pr.reviewDecision = data.reviewDecision;
dashboard.pr.mergeability = data.mergeability;
dashboard.pr.unresolvedThreads = data.unresolvedThreads;
dashboard.pr.unresolvedComments = data.unresolvedComments;
dashboard.pr.enriched = true;
// Add rate-limit warning blocker if most requests failed
// (but we still applied any successful results above)
if (
mostFailed &&
!dashboard.pr.mergeability.blockers.includes("API rate limited or unavailable")
) {
dashboard.pr.mergeability.blockers.push("API rate limited or unavailable");
}
// If rate limited, cache the partial data with a long TTL (5 min) so we stop
// hammering the API on every page load. The rate-limit blocker flag tells the
// UI to show stale-data warnings instead of making decisions on bad data.
if (mostFailed) {
const rateLimitedData: PREnrichmentData = {
state: dashboard.pr.state,
title: dashboard.pr.title,
additions: dashboard.pr.additions,
deletions: dashboard.pr.deletions,
ciStatus: dashboard.pr.ciStatus,
ciChecks: dashboard.pr.ciChecks,
reviewDecision: dashboard.pr.reviewDecision,
mergeability: dashboard.pr.mergeability,
unresolvedThreads: dashboard.pr.unresolvedThreads,
unresolvedComments: dashboard.pr.unresolvedComments,
};
prCache.set(cacheKey, rateLimitedData, 60 * 60_000); // 60 min — GitHub rate limit resets hourly
refreshDashboardSessionDerivedFields(dashboard);
return true;
}
const cacheData: PREnrichmentData = {
state: dashboard.pr.state,
title: dashboard.pr.title,
additions: dashboard.pr.additions,
deletions: dashboard.pr.deletions,
ciStatus: dashboard.pr.ciStatus,
ciChecks: dashboard.pr.ciChecks,
reviewDecision: dashboard.pr.reviewDecision,
mergeability: dashboard.pr.mergeability,
unresolvedThreads: dashboard.pr.unresolvedThreads,
unresolvedComments: dashboard.pr.unresolvedComments,
};
prCache.set(cacheKey, cacheData);
refreshDashboardSessionDerivedFields(dashboard);
return true;
}

10
packages/web/src/lib/services.ts
View File

@ -115,10 +115,14 @@ async function initServices(): Promise<Services> {
const sessionManager = createSessionManager({ config, registry });
// Start the lifecycle manager — polls sessions every 30s, triggers reactions
// (CI failure → send fix message, review comments → forward to agent, etc.)
// Lifecycle manager for webhook-triggered checks only — no independent polling.
// The CLI process (`ao`) runs the 30s polling loop and writes PR enrichment
// data to session metadata files. The dashboard reads from metadata instead
// of calling GitHub API directly. This means the dashboard is NOT self-sufficient:
// if the CLI process isn't running, sessions will have no PR enrichment data,
// no state transitions, and no reactions. The SSE endpoint surfaces whatever
// metadata the CLI has written — stale data is expected when CLI is down.
const lifecycleManager = createLifecycleManager({ config, registry, sessionManager });
lifecycleManager.start(30_000);
return { config, registry, sessionManager, lifecycleManager };
}

18
packages/web/src/providers/MuxProvider.tsx
View File

@ -6,7 +6,7 @@ import type { ClientMessage, ServerMessage, SessionPatch } from "@/lib/mux-proto
interface MuxContextValue {
subscribeTerminal: (id: string, callback: (data: string) => void) => () => void;
writeTerminal: (id: string, data: string) => void;
openTerminal: (id: string) => void;
openTerminal: (id: string, tmuxName?: string) => void;
closeTerminal: (id: string) => void;
resizeTerminal: (id: string, cols: number, rows: number) => void;
status: "connecting" | "connected" | "reconnecting" | "disconnected";
@ -71,7 +71,7 @@ function buildMuxWsUrl(runtimeConfig: { directTerminalPort?: string; proxyWsPath
export function MuxProvider({ children }: { children: ReactNode }) {
const wsRef = useRef<WebSocket | null>(null);
const subscribersRef = useRef(new Map<string, Set<(data: string) => void>>());
const openedTerminalsRef = useRef(new Set<string>());
const openedTerminalsRef = useRef(new Map<string, string | undefined>());
const [status, setStatus] = useState<"connecting" | "connected" | "reconnecting" | "disconnected">(
"connecting",
);
@ -105,11 +105,12 @@ export function MuxProvider({ children }: { children: ReactNode }) {
reconnectAttempt.current = 0;
// Re-open previously opened terminals
for (const terminalId of openedTerminalsRef.current) {
for (const [terminalId, tmuxName] of openedTerminalsRef.current) {
const openMsg: ClientMessage = {
ch: "terminal",
id: terminalId,
type: "open",
...(tmuxName && { tmuxName }),
};
ws.send(JSON.stringify(openMsg));
}
@ -136,8 +137,10 @@ export function MuxProvider({ children }: { children: ReactNode }) {
}
}
} else if (msg.type === "opened") {
// Terminal opened successfully
openedTerminalsRef.current.add(msg.id);
// Terminal opened successfully — preserve existing tmuxName
if (!openedTerminalsRef.current.has(msg.id)) {
openedTerminalsRef.current.set(msg.id, undefined);
}
} else if (msg.type === "exited") {
// PTY exited and could not be re-attached — remove so it isn't
// re-opened on reconnect, and surface a terminal-level error chunk
@ -269,13 +272,14 @@ export function MuxProvider({ children }: { children: ReactNode }) {
}
}, []);
const openTerminal = useCallback((id: string) => {
openedTerminalsRef.current.add(id);
const openTerminal = useCallback((id: string, tmuxName?: string) => {
openedTerminalsRef.current.set(id, tmuxName);
if (wsRef.current?.readyState === WebSocket.OPEN) {
const msg: ClientMessage = {
ch: "terminal",
id,
type: "open",
...(tmuxName && { tmuxName }),
};
wsRef.current.send(JSON.stringify(msg));
}