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>
This commit is contained in:
adil
parent
08d7be7ef1
commit
a339ccef94
|
|
@ -0,0 +1,507 @@
|
|||
# D4 AO-Side Reduction Plan
|
||||
|
||||
## Goal
|
||||
|
||||
Reduce redundant GitHub traffic from AO-owned code paths (lifecycle polling, SCM batch, CLI commands) without changing the user-visible lifecycle model.
|
||||
|
||||
Agent-side `gh` call reduction is covered separately in [D4-wrapper-cache-plan.md](./D4-wrapper-cache-plan.md). That plan also includes the session-manager changes needed to make the `~/.ao/bin/gh` wrapper universal for all agents (including Claude Code).
|
||||
|
||||
## Scope
|
||||
|
||||
This plan covers four changes:
|
||||
|
||||
1. **Phase 2 skip**: if a PR is auto-detected in the current poll, persist it and skip same-poll live SCM fallback.
|
||||
2. **Phase 4/5 cache-first**: prefer cached PR enrichment data before making individual SCM calls.
|
||||
3. **PR-scoped ETag**: replace repo-scoped Guard 1 with PR-scoped ETag checks and refresh only changed PRs.
|
||||
4. **Metadata-first PR reuse**: AO-owned paths (`ao status`, lifecycle) should prefer known PR metadata before calling `detectPR()`.
|
||||
|
||||
## Current Architecture
|
||||
|
||||
### Persistent SCM cache
|
||||
|
||||
The real cross-poll cache lives in `packages/plugins/scm-github/src/graphql-batch.ts`.
|
||||
|
||||
- `etagCache` at [graphql-batch.ts:69](/Users/adilshaikh/Desktop/ao/packages/plugins/scm-github/src/graphql-batch.ts:69)
|
||||
- `prMetadataCache` at [graphql-batch.ts:131](/Users/adilshaikh/Desktop/ao/packages/plugins/scm-github/src/graphql-batch.ts:131)
|
||||
- `prEnrichmentDataCache` at [graphql-batch.ts:150](/Users/adilshaikh/Desktop/ao/packages/plugins/scm-github/src/graphql-batch.ts:150)
|
||||
|
||||
These survive across polls and decide whether GraphQL can be skipped.
|
||||
|
||||
### Per-poll lifecycle cache
|
||||
|
||||
Lifecycle manager keeps a temporary cache only for the current poll:
|
||||
|
||||
- `prEnrichmentCache` at [lifecycle-manager.ts:325](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:325)
|
||||
- cleared at [lifecycle-manager.ts:342](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:342)
|
||||
- repopulated from SCM batch results in [lifecycle-manager.ts:442](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:442)
|
||||
|
||||
This cache is reused later in the same poll by:
|
||||
|
||||
- Phase 2 status decisions at [lifecycle-manager.ts:746](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:746)
|
||||
- Phase 4 CI details at [lifecycle-manager.ts:1352](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:1352)
|
||||
- Phase 5 merge conflict details at [lifecycle-manager.ts:1479](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:1479)
|
||||
|
||||
### Metadata-backed PR identity
|
||||
|
||||
Session metadata already persists `pr`:
|
||||
|
||||
- metadata contract at [types.ts:1532](/Users/adilshaikh/Desktop/ao/packages/core/src/types.ts:1532)
|
||||
- read path at [metadata.ts:69](/Users/adilshaikh/Desktop/ao/packages/core/src/metadata.ts:69)
|
||||
- lifecycle auto-detect writeback at [lifecycle-manager.ts:726](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:726)
|
||||
- agent `gh pr create` wrapper writeback at [agent-workspace-hooks.ts:232](/Users/adilshaikh/Desktop/ao/packages/core/src/agent-workspace-hooks.ts:232)
|
||||
|
||||
`session.pr` is reconstructed from `lifecycle.pr.url ?? meta["pr"]` in [session-from-metadata.ts:46](/Users/adilshaikh/Desktop/ao/packages/core/src/utils/session-from-metadata.ts:46).
|
||||
|
||||
---
|
||||
|
||||
## Change 1: Phase 2 skip same-poll fallback after PR auto-detect
|
||||
|
||||
### Current behavior
|
||||
|
||||
Inside `determineStatus()`:
|
||||
|
||||
- PR auto-detect happens at [lifecycle-manager.ts:708](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:708)
|
||||
- detected PR is assigned to `session.pr` and metadata is updated at [lifecycle-manager.ts:718](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:718) and [lifecycle-manager.ts:726](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:726)
|
||||
- immediately after that, the same check can enter the per-PR fallback block at [lifecycle-manager.ts:743](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:743) and call:
|
||||
- `getPRState()`
|
||||
- `getCISummary()`
|
||||
- `getReviewDecision()`
|
||||
- `getMergeability()`
|
||||
|
||||
That fallback is redundant because batch enrichment already ran earlier in the poll and the newly detected PR will not be in `prEnrichmentCache` until the next poll.
|
||||
|
||||
### Planned change
|
||||
|
||||
Add one ephemeral local flag in `determineStatus()`:
|
||||
|
||||
- `let detectedPRThisCycle = false;`
|
||||
|
||||
When `detectPR()` succeeds:
|
||||
|
||||
- keep `session.pr = detectedPR`
|
||||
- keep `updateMetadata(..., { pr: detectedPR.url })`
|
||||
- keep `sessionManager.invalidateCache()`
|
||||
- keep `lifecycle.pr.number`, `lifecycle.pr.url`, and `lifecycle.pr.lastObservedAt`
|
||||
- do **not** mark canonical PR state as open in that same branch
|
||||
- set `detectedPRThisCycle = true`
|
||||
|
||||
Then gate the per-PR fallback block:
|
||||
|
||||
- if `session.pr && scm && !detectedPRThisCycle`, run the current fallback logic
|
||||
- if `detectedPRThisCycle`, skip live SCM fallback for this poll
|
||||
|
||||
### Why lifecycle still needs a partial PR update
|
||||
|
||||
`updateSessionMetadata()` later writes lifecycle-derived fields back to metadata via:
|
||||
|
||||
- [lifecycle-manager.ts:1072](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:1072)
|
||||
- [lifecycle-state.ts:449](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-state.ts:449)
|
||||
|
||||
If we stop touching lifecycle PR fields entirely, the final lifecycle metadata patch can wipe the just-written `pr` URL in the same check. The safe approach is:
|
||||
|
||||
- update `lifecycle.pr.number`
|
||||
- update `lifecycle.pr.url`
|
||||
- update `lifecycle.pr.lastObservedAt`
|
||||
- defer `lifecycle.pr.state` / `reason` until next poll
|
||||
|
||||
### Next-poll inclusion path
|
||||
|
||||
This optimization is safe because the next poll:
|
||||
|
||||
1. reloads sessions via `sessionManager.list()` at [lifecycle-manager.ts:1870](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:1870)
|
||||
2. rebuilds `session.pr` from metadata in [session-from-metadata.ts:46](/Users/adilshaikh/Desktop/ao/packages/core/src/utils/session-from-metadata.ts:46)
|
||||
3. collects those PRs for batch enrichment in [lifecycle-manager.ts:345](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:345)
|
||||
|
||||
### Tests
|
||||
|
||||
Update or add lifecycle tests:
|
||||
|
||||
- extend the existing detect-PR test near [lifecycle-manager.test.ts:700](/Users/adilshaikh/Desktop/ao/packages/core/src/__tests__/lifecycle-manager.test.ts:700) to assert same-poll auto-detect persists metadata but does **not** call:
|
||||
- `getPRState`
|
||||
- `getCISummary`
|
||||
- `getReviewDecision`
|
||||
- `getMergeability`
|
||||
- add a `pollAll()` regression test proving:
|
||||
- poll 1 auto-detects the PR
|
||||
- poll 2 includes it in batch enrichment
|
||||
- later PR classification comes from batch data, not immediate fallback
|
||||
|
||||
---
|
||||
|
||||
## Change 2: Phase 4 and Phase 5 cache-first behavior
|
||||
|
||||
### Phase 4 current behavior
|
||||
|
||||
Phase 4 already prefers cache when `ciChecks` is present:
|
||||
|
||||
- [lifecycle-manager.ts:1352](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:1352)
|
||||
|
||||
Current rule:
|
||||
|
||||
- if `cachedEnrichment?.ciChecks !== undefined`, use it
|
||||
- else fall back to `scm.getCIChecks()`
|
||||
|
||||
This should stay, because GraphQL batch intentionally omits `ciChecks` when contexts are truncated:
|
||||
|
||||
- [graphql-batch.ts:847](/Users/adilshaikh/Desktop/ao/packages/plugins/scm-github/src/graphql-batch.ts:847)
|
||||
|
||||
### Phase 4 plan
|
||||
|
||||
Treat Phase 4 as already mostly correct.
|
||||
|
||||
Implementation work:
|
||||
|
||||
- keep the existing `ciChecks !== undefined` contract
|
||||
- document it in code comments as the canonical fallback rule
|
||||
- add tests for the negative case:
|
||||
- if cached batch data omits `ciChecks`, Phase 4 must still call `getCIChecks()`
|
||||
|
||||
### Phase 5 current behavior
|
||||
|
||||
Phase 5 partially prefers cache for conflicts:
|
||||
|
||||
- [lifecycle-manager.ts:1479](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:1479)
|
||||
|
||||
But it currently does:
|
||||
|
||||
- `hasConflicts = cachedData.hasConflicts ?? false`
|
||||
|
||||
That treats "field missing" as "no conflicts," which is too aggressive.
|
||||
|
||||
### Phase 5 plan
|
||||
|
||||
Change conflict derivation to:
|
||||
|
||||
1. if `cachedData.hasConflicts` is a boolean, use it
|
||||
2. else if `cachedData.blockers` exists, derive conflicts from:
|
||||
- `cachedData.blockers.includes("Merge conflicts")`
|
||||
3. else fall back to `scm.getMergeability()`
|
||||
|
||||
Relevant data already exists in:
|
||||
|
||||
- `PREnrichmentData` in [types.ts:933](/Users/adilshaikh/Desktop/ao/packages/core/src/types.ts:933)
|
||||
- batch extraction at [graphql-batch.ts:865](/Users/adilshaikh/Desktop/ao/packages/plugins/scm-github/src/graphql-batch.ts:865)
|
||||
|
||||
### Tests
|
||||
|
||||
Existing tests already cover the cache-first happy path:
|
||||
|
||||
- Phase 5 near [lifecycle-manager.test.ts:2364](/Users/adilshaikh/Desktop/ao/packages/core/src/__tests__/lifecycle-manager.test.ts:2364)
|
||||
- Phase 4 near [lifecycle-manager.test.ts:2415](/Users/adilshaikh/Desktop/ao/packages/core/src/__tests__/lifecycle-manager.test.ts:2415)
|
||||
|
||||
Add fallback coverage:
|
||||
|
||||
- cache entry with no `ciChecks` still calls `getCIChecks()`
|
||||
- cache entry with no `hasConflicts` and no `blockers` still calls `getMergeability()`
|
||||
- cache entry with `blockers` containing `"Merge conflicts"` skips `getMergeability()`
|
||||
|
||||
---
|
||||
|
||||
## Change 3: PR-scoped ETag guard and selective refresh
|
||||
|
||||
### Current behavior
|
||||
|
||||
Guard 1 is repo-scoped:
|
||||
|
||||
- `ETagCache.prList` at [graphql-batch.ts:57](/Users/adilshaikh/Desktop/ao/packages/plugins/scm-github/src/graphql-batch.ts:57)
|
||||
- `checkPRListETag()` at [graphql-batch.ts:366](/Users/adilshaikh/Desktop/ao/packages/plugins/scm-github/src/graphql-batch.ts:366)
|
||||
|
||||
`shouldRefreshPREnrichment()` groups by repo and sets one repo-wide `guard1DetectedChanges`:
|
||||
|
||||
- [graphql-batch.ts:187](/Users/adilshaikh/Desktop/ao/packages/plugins/scm-github/src/graphql-batch.ts:187)
|
||||
|
||||
Once any repo change is seen, `enrichSessionsPRBatch()` refreshes all input PRs for that path instead of only changed PRs:
|
||||
|
||||
- [graphql-batch.ts:925](/Users/adilshaikh/Desktop/ao/packages/plugins/scm-github/src/graphql-batch.ts:925)
|
||||
|
||||
### Planned change
|
||||
|
||||
Replace repo-scoped Guard 1 with PR-resource Guard 1:
|
||||
|
||||
- current endpoint: `GET /repos/{owner}/{repo}/pulls?...per_page=1`
|
||||
- new endpoint: `GET /repos/{owner}/{repo}/pulls/{number}`
|
||||
|
||||
Cache key changes:
|
||||
|
||||
- from `owner/repo`
|
||||
- to `owner/repo#number`
|
||||
|
||||
Recommended changes in `graphql-batch.ts`:
|
||||
|
||||
- rename `MAX_PR_LIST_ETAGS` to `MAX_PR_RESOURCE_ETAGS`
|
||||
- replace `ETagCache.prList` with `ETagCache.prResource`
|
||||
- replace `get/setPRListETag()` with `get/setPRResourceETag(owner, repo, number)`
|
||||
- replace `checkPRListETag()` with `checkPRResourceETag(owner, repo, number)`
|
||||
|
||||
Guard 2 stays the same in principle:
|
||||
|
||||
- commit-status ETag check using `owner/repo#sha`
|
||||
- [graphql-batch.ts:424](/Users/adilshaikh/Desktop/ao/packages/plugins/scm-github/src/graphql-batch.ts:424)
|
||||
|
||||
### New guard result shape
|
||||
|
||||
`shouldRefreshPREnrichment()` should stop returning only a boolean and instead return a refresh plan:
|
||||
|
||||
```ts
|
||||
interface PREnrichmentRefreshPlan {
|
||||
shouldRefresh: boolean;
|
||||
prsToRefresh: PRInfo[];
|
||||
cachedPRKeys: string[];
|
||||
details: string[];
|
||||
}
|
||||
```
|
||||
|
||||
### New decision logic
|
||||
|
||||
For each tracked PR:
|
||||
|
||||
1. Run PR-resource Guard 1.
|
||||
2. If Guard 1 returns changed or errors:
|
||||
- mark only that PR for refresh
|
||||
3. If Guard 1 returns 304:
|
||||
- if full enrichment cache is missing, refresh that PR
|
||||
- if metadata cache is missing or `headSha === null`, refresh that PR
|
||||
- otherwise run Guard 2 commit-status ETag
|
||||
4. If Guard 2 returns 304:
|
||||
- reuse cached enrichment for that PR
|
||||
5. If Guard 2 returns changed:
|
||||
- refresh that PR
|
||||
|
||||
### `enrichSessionsPRBatch()` changes
|
||||
|
||||
Update [graphql-batch.ts:925](/Users/adilshaikh/Desktop/ao/packages/plugins/scm-github/src/graphql-batch.ts:925) so it:
|
||||
|
||||
- seeds the result map from cached unchanged PRs
|
||||
- batches only `prsToRefresh`
|
||||
- returns cached unchanged PRs even if the GraphQL fetch for changed PRs partially fails
|
||||
|
||||
This is the key structural change: selective refresh instead of all-or-nothing refresh.
|
||||
|
||||
### Edge cases
|
||||
|
||||
- treat missing full enrichment cache or missing metadata cache as "refresh required"
|
||||
- do not reuse stale cache if a changed PR disappears from GraphQL response
|
||||
- old commit-status ETags keyed by obsolete SHAs can be left for LRU eviction
|
||||
- optional hardening: dedupe `PRInfo[]` by `owner/repo#number` before guarding
|
||||
|
||||
### Tests
|
||||
|
||||
Update `packages/plugins/scm-github/test/graphql-batch.test.ts`:
|
||||
|
||||
- rename and update PR-resource ETag helpers
|
||||
- replace repo-level Guard 1 expectations with PR-resource expectations
|
||||
- add guard-planner tests:
|
||||
- one changed PR and one unchanged PR in the same repo → refresh only changed PR
|
||||
- Guard 1 304 + missing full data → refresh that PR
|
||||
- Guard 1 304 + missing metadata or `headSha === null` → refresh that PR
|
||||
- Guard 1 304 + Guard 2 304 → reuse cache only
|
||||
- Guard 1 304 + Guard 2 200 → refresh only that PR
|
||||
- add `enrichSessionsPRBatch()` tests:
|
||||
- merge cached unchanged PRs with fetched changed PRs
|
||||
- GraphQL query includes only `prsToRefresh`
|
||||
- unchanged cached PRs are still returned if the changed subset fails
|
||||
|
||||
---
|
||||
|
||||
## Change 4: Metadata-first PR reuse (AO-owned paths only)
|
||||
|
||||
Agent-side wrapper caching for `gh pr list --head` and `gh issue view` is covered in [D4-wrapper-cache-plan.md](./D4-wrapper-cache-plan.md). This change covers only the AO-owned Node.js code paths.
|
||||
|
||||
### Lifecycle hardening
|
||||
|
||||
Lifecycle already calls `detectPR()` only when `!session.pr`:
|
||||
|
||||
- [lifecycle-manager.ts:708](/Users/adilshaikh/Desktop/ao/packages/core/src/lifecycle-manager.ts:708)
|
||||
|
||||
But to make this resilient even if a caller hands lifecycle a stale `Session`, harden it by hydrating from metadata first:
|
||||
|
||||
- if `session.pr` is null and `session.metadata["pr"]` exists, reconstruct a minimal PR object from metadata before calling `scm.detectPR()`
|
||||
|
||||
Reuse existing parser:
|
||||
|
||||
- [packages/core/src/utils/pr.ts](/Users/adilshaikh/Desktop/ao/packages/core/src/utils/pr.ts:1)
|
||||
|
||||
### `ao status` fix
|
||||
|
||||
The main AO-owned rediscovery bug is in status:
|
||||
|
||||
- [packages/cli/src/commands/status.ts:121](/Users/adilshaikh/Desktop/ao/packages/cli/src/commands/status.ts:121)
|
||||
|
||||
Current behavior:
|
||||
|
||||
- if `branch` exists, it calls `scm.detectPR(session, project)` even if metadata already has `pr`
|
||||
|
||||
Plan:
|
||||
|
||||
- prefer `session.pr` for `prUrl`, `prNumber`, and SCM follow-up calls
|
||||
- only fall back to `scm.detectPR()` when there is no known PR object or URL
|
||||
|
||||
### Tests
|
||||
|
||||
- add lifecycle test: metadata `pr` present, `session.pr` absent or stale, `detectPR()` should not run
|
||||
- update/add status tests in `packages/cli/__tests__/commands/status.test.ts` to assert:
|
||||
- existing PR metadata avoids `detectPR()`
|
||||
- CI/review lookups use known PR identity
|
||||
|
||||
---
|
||||
|
||||
## Recommended Implementation Order
|
||||
|
||||
1. Phase 2 skip-same-poll fallback (Change 1)
|
||||
2. Phase 5 conflict cache fallback hardening (Change 2)
|
||||
3. Status command metadata-first reuse (Change 4)
|
||||
4. PR-scoped ETag planner and selective refresh (Change 3)
|
||||
5. Test expansion and trace validation
|
||||
|
||||
The agent-side wrapper cache ([D4-wrapper-cache-plan.md](./D4-wrapper-cache-plan.md)) can be implemented in parallel with steps 1-3 above since it touches different files (wrapper scripts + session-manager environment, vs. lifecycle-manager + graphql-batch + CLI).
|
||||
|
||||
---
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
### Lifecycle (Change 1)
|
||||
|
||||
- A PR auto-detected in Phase 2 is persisted immediately.
|
||||
- Same-poll fallback SCM calls are skipped for that newly detected PR.
|
||||
- Next poll includes the PR in batch enrichment and classifies it there.
|
||||
|
||||
### Cache-first Phase 4/5 (Change 2)
|
||||
|
||||
- Phase 4 uses cached `ciChecks` when present and falls back only when missing.
|
||||
- Phase 5 uses cached conflict data when present and falls back only when conflict information is incomplete.
|
||||
|
||||
### PR-scoped ETag (Change 3)
|
||||
|
||||
- Unrelated PR activity in the same repo no longer refreshes all tracked session PRs.
|
||||
- Unchanged tracked PRs are served from cache while only changed PRs are refreshed.
|
||||
|
||||
### Metadata-first PR reuse (Change 4)
|
||||
|
||||
- `ao status` does not call `detectPR()` when a known PR already exists in metadata/session state.
|
||||
- Lifecycle hydrates `session.pr` from metadata before attempting discovery.
|
||||
|
||||
---
|
||||
|
||||
## Expected Impact
|
||||
|
||||
### Baseline (D4 10-session / 22-minute run)
|
||||
|
||||
AO-side trace: **890 calls** total.
|
||||
|
||||
| AO-side bucket | Count | % | Rate limit pool |
|
||||
|----------------|-------|---|-----------------|
|
||||
| `scm-github gh.pr.list` (detectPR) | 598 | 67.2% | REST core |
|
||||
| `tracker-github gh.issue.view` | 75 | 8.4% | REST core |
|
||||
| `scm-github gh.pr.view` | 47 | 5.3% | REST core |
|
||||
| `scm-github-batch gh.api.guard-pr-list` | 37 | 4.2% | REST core (304s free) |
|
||||
| `scm-github-batch gh.api.graphql-batch` | 31 | 3.5% | GraphQL |
|
||||
| `scm-github gh.api.graphql` | 30 | 3.4% | GraphQL |
|
||||
| `scm-github gh.pr.checks` | 30 | 3.4% | REST core |
|
||||
| Other | 42 | 4.7% | Mixed |
|
||||
|
||||
Rate limit burn: **2502 GraphQL pts/hr**, **16 REST core pts** (over 22 min).
|
||||
|
||||
### Per-change reduction estimates
|
||||
|
||||
#### Change 1: Phase 2 skip same-poll fallback
|
||||
|
||||
When a PR is auto-detected, 4 redundant SCM calls are skipped (getPRState, getCISummary, getReviewDecision, getMergeability). With 10 sessions each detecting once:
|
||||
|
||||
| Bucket | Before | After | Saved |
|
||||
|--------|--------|-------|-------|
|
||||
| `gh.pr.view` (getPRState) | +10 | 0 | ~10 |
|
||||
| `gh.pr.checks` (getCISummary) | +10 | 0 | ~10 |
|
||||
| `gh.api.graphql` (getReviewDecision) | +10 | 0 | ~10 |
|
||||
| `gh.pr.view` (getMergeability) | +10 | 0 | ~10 |
|
||||
| **Total** | | | **~40 calls** |
|
||||
|
||||
Small in absolute terms but eliminates a code path that is completely wasted work — the batch enrichment in the next poll covers all of this.
|
||||
|
||||
#### Change 2: Phase 4/5 cache-first
|
||||
|
||||
When batch enrichment already fetched CI checks and merge status, individual follow-up calls are skipped. The batch query covers most polls; individual calls only fire when batch data is incomplete (truncated CI contexts, missing mergeability).
|
||||
|
||||
Estimated batch coverage rate: ~60-70% of polls have complete data.
|
||||
|
||||
| Bucket | Before | After | Saved |
|
||||
|--------|--------|-------|-------|
|
||||
| `gh.pr.checks` (Phase 4 fallback) | 30 | ~10 | ~20 |
|
||||
| `gh.pr.view` (Phase 5 getMergeability) | ~15 | ~5 | ~10 |
|
||||
| **Total** | | | **~30 calls** |
|
||||
|
||||
Also avoids individual GraphQL review queries when batch data already has review decision.
|
||||
|
||||
#### Change 3: PR-scoped ETag guard
|
||||
|
||||
This is the highest-leverage change for **GraphQL points**. Currently, any change to any PR in the repo triggers a full GraphQL batch refresh of all tracked PRs. With PR-scoped guards, only the changed PR is refreshed.
|
||||
|
||||
In a 10-session run against the same repo, a single PR getting a new CI status currently refreshes all 10 PRs. With PR-scoped guards, only 1 is refreshed; the other 9 serve from cache.
|
||||
|
||||
| Metric | Before | After | Saved |
|
||||
|--------|--------|-------|-------|
|
||||
| Guard 1 REST calls | 37 (repo-scoped) | ~370 (per-PR, but most return 304 = free) | Net ~0 cost change |
|
||||
| GraphQL batch calls | 31 | ~10-12 | ~19-21 |
|
||||
| GraphQL points consumed | ~2502 pts/hr | ~800-1200 pts/hr | **~50-60%** |
|
||||
|
||||
The guard calls increase in count (one per PR instead of one per repo) but 304 responses don't cost rate limit. The GraphQL savings are substantial because batches are only run for PRs that actually changed.
|
||||
|
||||
#### Change 4: Metadata-first PR reuse
|
||||
|
||||
The 598 `detectPR()` calls are the largest single AO-side bucket. Once a PR is known for a session (via wrapper writeback or lifecycle auto-detect), all subsequent polls should skip detectPR(). With 10 sessions over ~44 poll cycles (22min / 30s), each discovering their PR within the first 2-3 polls:
|
||||
|
||||
- Calls with PR already known: ~598 - (10 × 3) = ~568 wasted calls
|
||||
- With metadata-first hydration, these become zero
|
||||
|
||||
| Bucket | Before | After | Saved |
|
||||
|--------|--------|-------|-------|
|
||||
| `gh.pr.list` (detectPR) | 598 | ~30 (initial discovery only) | **~568 (95%)** |
|
||||
|
||||
This also benefits from the wrapper plan: when `gh pr create` writes `pr=` to metadata (via the wrapper), the very next lifecycle poll sees it and never calls detectPR().
|
||||
|
||||
### Combined AO-side projection
|
||||
|
||||
| Bucket | Before | After (est.) | Saved |
|
||||
|--------|--------|-------------|-------|
|
||||
| `gh.pr.list` (detectPR) | 598 | ~30 | ~568 |
|
||||
| `gh.pr.view` | 47 | ~22 | ~25 |
|
||||
| `gh.pr.checks` | 30 | ~10 | ~20 |
|
||||
| `gh.api.graphql` | 30 | ~15 | ~15 |
|
||||
| `gh.api.graphql-batch` | 31 | ~12 | ~19 |
|
||||
| `gh.api.guard-pr-list` → `guard-pr-resource` | 37 | ~100 (304s, free) | N/A |
|
||||
| `gh.issue.view` | 75 | 75 (unchanged, covered by wrapper plan) | 0 |
|
||||
| Other | 42 | 42 | 0 |
|
||||
| **Total calls** | **890** | **~306** | **~584 (66%)** |
|
||||
|
||||
| Rate limit | Before | After (est.) | Reduction |
|
||||
|------------|--------|-------------|-----------|
|
||||
| GraphQL burn | 2502 pts/hr | ~800-1200 pts/hr | **50-60%** |
|
||||
| REST core calls/hr | ~1630/hr | ~400/hr | **~75%** |
|
||||
|
||||
### Combined with wrapper plan
|
||||
|
||||
The wrapper plan ([D4-wrapper-cache-plan.md](./D4-wrapper-cache-plan.md)) targets agent-side calls (916 in the same run). Together:
|
||||
|
||||
| Source | Before | After (est.) | Saved |
|
||||
|--------|--------|-------------|-------|
|
||||
| AO-side (this plan) | 890 | ~306 | ~584 (66%) |
|
||||
| Agent-side (wrapper plan) | 916 | ~273 | ~643 (70%) |
|
||||
| **Total `gh` calls** | **1806** | **~579** | **~1227 (68%)** |
|
||||
|
||||
The dominant remaining cost after both plans is the legitimate work: initial PR discovery (once per session), batch GraphQL enrichment for PRs that genuinely changed, and issue context fetches (covered by wrapper TTL cache).
|
||||
|
||||
---
|
||||
|
||||
## Non-goals
|
||||
|
||||
- Changing canonical lifecycle precedence so `metadata.pr` overrides `statePayload.pr.url = null`
|
||||
- New reactions for blocker types other than current merge conflict handling
|
||||
- Agent-side `gh` caching (covered in [D4-wrapper-cache-plan.md](./D4-wrapper-cache-plan.md))
|
||||
|
||||
---
|
||||
|
||||
## Companion Plans
|
||||
|
||||
| File | Scope |
|
||||
|------|-------|
|
||||
| [D4-wrapper-cache-plan.md](./D4-wrapper-cache-plan.md) | Agent-side `gh` wrapper cache (PR discovery + issue context intercepts), session-manager universalization for all agents |
|
||||
| This file | AO-side lifecycle, SCM batch, and CLI reduction |
|
||||
|
|
@ -0,0 +1,515 @@
|
|||
# D4 Wrapper Cache Implementation Plan
|
||||
|
||||
## Scope
|
||||
|
||||
Two intercepts only, targeting the two largest agent-side waste buckets from the D4 analysis:
|
||||
|
||||
| Pattern | D4 count (10-session run) | % of agent-side traffic |
|
||||
|---------|--------------------------|------------------------|
|
||||
| `gh pr list --repo R --head B --json ... --limit 1` | 598 | 65.3% |
|
||||
| `gh issue view N --json ...` | 75 | 8.2% |
|
||||
|
||||
Combined: **673 / 916 = 73.5%** of all agent-side `gh` calls.
|
||||
|
||||
Everything else passes through unchanged.
|
||||
|
||||
---
|
||||
|
||||
## Storage
|
||||
|
||||
### Location
|
||||
|
||||
```
|
||||
$AO_DATA_DIR/.ghcache/$AO_SESSION/
|
||||
```
|
||||
|
||||
- `$AO_DATA_DIR` = sessions directory (already available in agent env)
|
||||
- `$AO_SESSION` = session ID (already available in agent env)
|
||||
- `.ghcache` is invisible to `listMetadata()` (`metadata.ts:366` filters `name.startsWith(".")`)
|
||||
|
||||
### File format
|
||||
|
||||
Each cache entry = two files:
|
||||
|
||||
```
|
||||
{key}.stdout # exact gh stdout (byte-identical to what real gh returned)
|
||||
{key}.ts # single line: epoch seconds when entry was written
|
||||
```
|
||||
|
||||
Example:
|
||||
|
||||
```
|
||||
$AO_DATA_DIR/.ghcache/issue-30-fix/
|
||||
pr-discovery-feat--issue-30.stdout # [{"number":57,"url":"...","title":"..."}]
|
||||
pr-discovery-feat--issue-30.ts # 1713567890
|
||||
issue-ctx-31.stdout # {"number":31,"title":"...","body":"..."}
|
||||
issue-ctx-31.ts # 1713567850
|
||||
```
|
||||
|
||||
### Why two files instead of embedding timestamp
|
||||
|
||||
The `.stdout` file is returned to the agent as-is via `cat`. No parsing, no stripping, no risk of corrupting JSON output. The `.ts` file is a one-line epoch used only by the cache-freshness check.
|
||||
|
||||
---
|
||||
|
||||
## Intercept 1: PR Discovery
|
||||
|
||||
### What agents are doing
|
||||
|
||||
From D4 traces, every agent session repeatedly runs:
|
||||
|
||||
```
|
||||
gh pr list --repo iamasx/api-test --head feat/issue-X --json number,url,title,headRefName,baseRefName,isDraft --limit 1
|
||||
```
|
||||
|
||||
The 10-session run showed:
|
||||
|
||||
- 75 identical calls for `feat/issue-30`
|
||||
- 74 identical calls for `feat/issue-27`
|
||||
- 74 identical calls for `feat/issue-26`
|
||||
- 73 identical calls for `feat/issue-22`
|
||||
- ...
|
||||
|
||||
Once branch `feat/issue-30` has PR #57, that answer never changes. Yet the agent keeps asking.
|
||||
|
||||
### Match conditions
|
||||
|
||||
ALL must be true:
|
||||
|
||||
- `$1` = `pr`, `$2` = `list`
|
||||
- `--head <branch>` flag present
|
||||
- `--limit 1` present
|
||||
- None of these flags present: `--search`, `--state`, `--assignee`, `--label`, `--jq`, `--template`
|
||||
|
||||
If any condition fails → passthrough to real `gh`.
|
||||
|
||||
### Cache key
|
||||
|
||||
```
|
||||
pr-discovery-{sanitized_branch}
|
||||
```
|
||||
|
||||
Where `sanitized_branch` = `--head` value with non-alphanumeric chars (except `.`, `_`, `-`) replaced by `-`:
|
||||
|
||||
```bash
|
||||
safe_branch=$(printf '%s' "$head_val" | tr -c 'a-zA-Z0-9._-' '-')
|
||||
```
|
||||
|
||||
### TTL
|
||||
|
||||
- **Positive result** (non-empty JSON array, i.e., stdout is not `[]`): **infinite** within session.
|
||||
- **Negative result** (`[]` or empty): **not cached**. The PR might be created at any moment.
|
||||
|
||||
### Cache-read flow
|
||||
|
||||
```
|
||||
1. Parse args, extract --head value and --repo value
|
||||
2. Compute cache_key = "pr-discovery-{safe_branch}"
|
||||
3. If cache file exists (positive = infinite TTL):
|
||||
cat "$cache_dir/$cache_key.stdout"
|
||||
exit 0
|
||||
4. Else:
|
||||
call real gh, capture stdout to tmpfile
|
||||
if exit_code == 0 AND stdout is not "[]" and not empty:
|
||||
write tmpfile → $cache_dir/$cache_key.stdout
|
||||
write epoch → $cache_dir/$cache_key.ts
|
||||
cat tmpfile
|
||||
exit $exit_code
|
||||
```
|
||||
|
||||
### Integration with `gh pr create` write intercept
|
||||
|
||||
The existing `pr/create` intercept (wrapper lines 212-244) already captures the PR URL and number after a successful `gh pr create`. Extend it to also populate the PR discovery cache:
|
||||
|
||||
After the existing metadata writes:
|
||||
|
||||
```bash
|
||||
# Populate PR discovery cache so subsequent gh pr list --head hits cache
|
||||
_branch="$(read_ao_metadata branch)"
|
||||
if [[ -n "$_branch" && -n "$pr_url" && -n "$pr_number" ]]; then
|
||||
_safe_branch=$(printf '%s' "$_branch" | tr -c 'a-zA-Z0-9._-' '-')
|
||||
_cache_key="pr-discovery-${_safe_branch}"
|
||||
_cache_dir="$(ao_cache_dir)"
|
||||
if [[ -n "$_cache_dir" ]]; then
|
||||
# Write a minimal but valid gh pr list JSON response.
|
||||
# Contains only the fields we know. If the agent later asks for
|
||||
# fields not present here, those fields will be null/missing in
|
||||
# the cached response. This is acceptable because:
|
||||
# - The most common query shape includes these exact fields
|
||||
# - Any missing-field issue self-corrects on the next real call
|
||||
# if the agent retries or asks for different fields
|
||||
_draft_val="${report_draft:-false}"
|
||||
printf '[{"number":%s,"url":"%s","headRefName":"%s","isDraft":%s}]\n' \
|
||||
"$pr_number" "$pr_url" "$_branch" "$_draft_val" \
|
||||
> "$_cache_dir/${_cache_key}.stdout.tmp.$$"
|
||||
mv "$_cache_dir/${_cache_key}.stdout.tmp.$$" "$_cache_dir/${_cache_key}.stdout"
|
||||
date +%s > "$_cache_dir/${_cache_key}.ts"
|
||||
fi
|
||||
fi
|
||||
```
|
||||
|
||||
This means: the moment `gh pr create` succeeds, the very next `gh pr list --head` for that branch hits cache with zero API calls.
|
||||
|
||||
### Projected reduction
|
||||
|
||||
From D4 10-session data: 598 calls → ~10 (one real call per session, the first discovery). **~588 calls eliminated (98%).**
|
||||
|
||||
---
|
||||
|
||||
## Intercept 2: Issue Context
|
||||
|
||||
### What agents are doing
|
||||
|
||||
From D4 traces, agents repeatedly fetch the same issue:
|
||||
|
||||
```
|
||||
gh issue view 31 --json number,title,body,state,labels,assignees
|
||||
```
|
||||
|
||||
- issue 31 viewed 9 times
|
||||
- issue 30 viewed 9 times
|
||||
- issue 29 viewed 9 times
|
||||
- issues 28, 27, 26, 24, 23, 22 viewed 7 times each
|
||||
- issue 25 viewed 6 times
|
||||
|
||||
Issue metadata (title, body, labels) does not change during a typical agent work session. Fetching once is sufficient.
|
||||
|
||||
### Match conditions
|
||||
|
||||
ALL must be true:
|
||||
|
||||
- `$1` = `issue`, `$2` = `view`
|
||||
- Third positional arg is a number (the issue identifier)
|
||||
- None of these flags present: `--web`, `--comments`, `--jq`, `--template`
|
||||
|
||||
If any condition fails → passthrough.
|
||||
|
||||
### Cache key
|
||||
|
||||
```
|
||||
issue-ctx-{issue_id}
|
||||
```
|
||||
|
||||
The issue ID is the numeric identifier extracted from the positional arg. No sanitization needed (it's a number).
|
||||
|
||||
### TTL
|
||||
|
||||
**300 seconds (5 minutes).** Issue content can change externally (someone edits the title or adds a label), but this is rare during an active agent session. A 5-minute window is conservative enough to avoid serving truly stale data while eliminating most repeated fetches.
|
||||
|
||||
### Cache-read flow
|
||||
|
||||
```
|
||||
1. Parse args, extract issue ID (first positional arg after "issue view")
|
||||
2. Compute cache_key = "issue-ctx-{issue_id}"
|
||||
3. If cache file exists AND age < 300 seconds:
|
||||
cat "$cache_dir/$cache_key.stdout"
|
||||
exit 0
|
||||
4. Else:
|
||||
call real gh, capture stdout to tmpfile
|
||||
if exit_code == 0:
|
||||
write tmpfile → $cache_dir/$cache_key.stdout
|
||||
write epoch → $cache_dir/$cache_key.ts
|
||||
cat tmpfile
|
||||
exit $exit_code
|
||||
```
|
||||
|
||||
### Age check
|
||||
|
||||
```bash
|
||||
ao_cache_fresh() {
|
||||
local cache_key="$1" max_age="$2"
|
||||
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
|
||||
|
||||
# max_age=0 means infinite TTL
|
||||
[[ "$max_age" -eq 0 ]] && return 0
|
||||
|
||||
local cached_ts now
|
||||
cached_ts=$(cat "$ts_file" 2>/dev/null) || return 1
|
||||
now=$(date +%s)
|
||||
(( now - cached_ts < max_age ))
|
||||
}
|
||||
```
|
||||
|
||||
### Projected reduction
|
||||
|
||||
From D4 10-session data: 75 calls → ~20 (one real call per issue per 5-minute window). **~55 calls eliminated (73%).**
|
||||
|
||||
---
|
||||
|
||||
## Helper Functions
|
||||
|
||||
Added to `AO_METADATA_HELPER` (sourced by both `gh` and `git` wrappers):
|
||||
|
||||
### `read_ao_metadata()`
|
||||
|
||||
```bash
|
||||
read_ao_metadata() {
|
||||
local key="$1"
|
||||
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
|
||||
|
||||
local metadata_file="$ao_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#*=}"
|
||||
}
|
||||
```
|
||||
|
||||
### `ao_cache_dir()`
|
||||
|
||||
```bash
|
||||
ao_cache_dir() {
|
||||
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
|
||||
|
||||
local d="$ao_dir/.ghcache/$ao_session"
|
||||
mkdir -p "$d" 2>/dev/null || return 1
|
||||
printf '%s' "$d"
|
||||
}
|
||||
```
|
||||
|
||||
### `ao_cache_fresh()`
|
||||
|
||||
As defined in the issue-context section above.
|
||||
|
||||
### `ao_cache_read()`
|
||||
|
||||
```bash
|
||||
ao_cache_read() {
|
||||
local cache_key="$1"
|
||||
local cache_dir
|
||||
cache_dir="$(ao_cache_dir)" || return 1
|
||||
cat "$cache_dir/${cache_key}.stdout"
|
||||
}
|
||||
```
|
||||
|
||||
### `ao_cache_write()`
|
||||
|
||||
```bash
|
||||
# Stdin is piped in. Writes atomically.
|
||||
ao_cache_write() {
|
||||
local cache_key="$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"
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## New Wrapper Structure
|
||||
|
||||
The `GH_WRAPPER` dispatch changes from:
|
||||
|
||||
```bash
|
||||
# Current:
|
||||
log_gh_invocation "$@"
|
||||
case "$1/$2" in
|
||||
pr/create) ... ;;
|
||||
*) exec "$real_gh" "$@" ;;
|
||||
esac
|
||||
```
|
||||
|
||||
To:
|
||||
|
||||
```bash
|
||||
log_gh_invocation "$@"
|
||||
|
||||
# ── Cacheable reads ──────────────────────────────────────
|
||||
|
||||
# 1. PR discovery: gh pr list --head <B> --limit 1
|
||||
if [[ "$1" == "pr" && "$2" == "list" ]]; then
|
||||
<parse args, check match conditions>
|
||||
if <matched>; then
|
||||
cache_key="pr-discovery-${safe_branch}"
|
||||
if ao_cache_fresh "$cache_key" 0; then
|
||||
ao_cache_read "$cache_key"
|
||||
exit 0
|
||||
fi
|
||||
<call real gh, capture stdout>
|
||||
if [[ $exit_code -eq 0 ]] && <stdout is not empty/[]>; then
|
||||
<write to cache>
|
||||
fi
|
||||
<output stdout>
|
||||
exit $exit_code
|
||||
fi
|
||||
fi
|
||||
|
||||
# 2. Issue context: gh issue view <N> --json ...
|
||||
if [[ "$1" == "issue" && "$2" == "view" ]]; then
|
||||
<parse args, check match conditions>
|
||||
if <matched>; then
|
||||
cache_key="issue-ctx-${issue_id}"
|
||||
if ao_cache_fresh "$cache_key" 300; then
|
||||
ao_cache_read "$cache_key"
|
||||
exit 0
|
||||
fi
|
||||
<call real gh, capture stdout>
|
||||
if [[ $exit_code -eq 0 ]]; then
|
||||
<write to cache>
|
||||
fi
|
||||
<output stdout>
|
||||
exit $exit_code
|
||||
fi
|
||||
fi
|
||||
|
||||
# ── Write intercepts (existing, enhanced) ────────────────
|
||||
|
||||
case "$1/$2" in
|
||||
pr/create)
|
||||
<existing pr create logic>
|
||||
<NEW: populate pr-discovery cache after success>
|
||||
exit $exit_code
|
||||
;;
|
||||
*)
|
||||
exec "$real_gh" "$@"
|
||||
;;
|
||||
esac
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Session-Manager Changes
|
||||
|
||||
### Make PATH wrappers universal for all agents
|
||||
|
||||
**`packages/core/src/session-manager.ts`**
|
||||
|
||||
At the `setupWorkspaceHooks` call site (~line 1524), add wrapper installation:
|
||||
|
||||
```typescript
|
||||
// Setup agent hooks for automatic metadata updates
|
||||
try {
|
||||
if (plugins.agent.setupWorkspaceHooks) {
|
||||
await plugins.agent.setupWorkspaceHooks(workspacePath, { dataDir: sessionsDir });
|
||||
}
|
||||
// Always install shared wrappers — every agent gets gh/git interception
|
||||
await setupPathWrapperWorkspace(workspacePath);
|
||||
} catch (err) {
|
||||
await cleanupWorktreeAndMetadata();
|
||||
throw err;
|
||||
}
|
||||
```
|
||||
|
||||
At all 3 `runtime.create()` sites (lines ~1257, ~1608, ~2697), inject PATH and GH_PATH after the agent environment spread:
|
||||
|
||||
```typescript
|
||||
environment: {
|
||||
...environment,
|
||||
PATH: buildAgentPath(environment["PATH"] ?? process.env["PATH"]),
|
||||
GH_PATH: PREFERRED_GH_PATH,
|
||||
AO_SESSION: sessionId,
|
||||
AO_DATA_DIR: sessionsDir,
|
||||
// ... rest unchanged
|
||||
},
|
||||
```
|
||||
|
||||
### Remove per-plugin boilerplate
|
||||
|
||||
Remove `buildAgentPath()` and `setupPathWrapperWorkspace()` calls from:
|
||||
|
||||
- `packages/plugins/agent-codex/src/index.ts`
|
||||
- `packages/plugins/agent-aider/src/index.ts`
|
||||
- `packages/plugins/agent-opencode/src/index.ts`
|
||||
- `packages/plugins/agent-cursor/src/index.ts`
|
||||
|
||||
These plugins no longer need to import or call these functions. The session manager handles it.
|
||||
|
||||
### Bump wrapper version
|
||||
|
||||
`packages/core/src/agent-workspace-hooks.ts:35`:
|
||||
|
||||
```typescript
|
||||
const WRAPPER_VERSION = "0.4.0";
|
||||
```
|
||||
|
||||
Existing workspaces auto-refresh when the version marker doesn't match.
|
||||
|
||||
---
|
||||
|
||||
## Cache Cleanup
|
||||
|
||||
### On session delete/archive
|
||||
|
||||
In `deleteMetadata()` (`metadata.ts:263`), add cleanup of the session's cache directory:
|
||||
|
||||
```typescript
|
||||
// Clean up gh cache for this session
|
||||
const cachePath = join(dataDir, ".ghcache", sessionId);
|
||||
try { rmSync(cachePath, { recursive: true, force: true }); } catch { /* best effort */ }
|
||||
```
|
||||
|
||||
### No cross-session leakage
|
||||
|
||||
Each session has its own cache subdirectory keyed by `$AO_SESSION`. A restored session gets a fresh directory. No stale data from previous runs.
|
||||
|
||||
---
|
||||
|
||||
## Safety
|
||||
|
||||
| Scenario | Handling |
|
||||
|----------|---------|
|
||||
| `$AO_DATA_DIR` or `$AO_SESSION` unset | All cache functions return 1 (failure). Wrapper falls through to real `gh`. Agent sees no difference. |
|
||||
| Cache directory creation fails (permissions, disk full) | `ao_cache_dir` returns 1. Cache write silently skipped. Real `gh` is called. |
|
||||
| `gh pr list --head` returns `[]` (no PR yet) | Not cached. Every subsequent call goes to real `gh` until a PR exists. |
|
||||
| Agent uses `--jq` or `--template` | Match conditions reject these flags. Passthrough to real `gh`. |
|
||||
| Agent uses different `--json` field sets for the same branch/issue | Cached stdout is the exact response from the first call. If the first call had `--json number,url,title,...` and a later call asks for `--json number,url` only, the cached response has _more_ fields than requested — `gh` clients tolerate this. If the later call asks for fields _not_ in the cached response, it gets a response without those fields. This is a theoretical edge case — the D4 traces show agents use the same `--json` field set consistently. |
|
||||
| Concurrent cache writes from parallel commands | Atomic write pattern: write to `$file.tmp.$$`, then `mv`. Same pattern used by existing metadata writes. |
|
||||
| Clock skew (container, VM) | Only affects TTL accuracy. A few seconds of drift on a 300s TTL is negligible. |
|
||||
|
||||
---
|
||||
|
||||
## Files Changed
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `packages/core/src/agent-workspace-hooks.ts` | Add cache helpers to `AO_METADATA_HELPER`. Add read intercepts + cache-write-on-create to `GH_WRAPPER`. Bump `WRAPPER_VERSION`. |
|
||||
| `packages/core/src/session-manager.ts` | Add `setupPathWrapperWorkspace()` call. Add `buildAgentPath()` + `GH_PATH` to all 3 environment sites. |
|
||||
| `packages/core/src/metadata.ts` | Add `.ghcache` cleanup in `deleteMetadata()`. |
|
||||
| `packages/plugins/agent-codex/src/index.ts` | Remove `buildAgentPath()`, `setupPathWrapperWorkspace()`, `GH_PATH` from plugin. |
|
||||
| `packages/plugins/agent-aider/src/index.ts` | Same removal. |
|
||||
| `packages/plugins/agent-opencode/src/index.ts` | Same removal. |
|
||||
| `packages/plugins/agent-cursor/src/index.ts` | Same removal. |
|
||||
| `packages/core/src/__tests__/agent-workspace-hooks.test.ts` | Tests for cache helpers, PR discovery intercept, issue context intercept, `gh pr create` cache population, passthrough on unsupported flags, TTL expiry. |
|
||||
|
||||
---
|
||||
|
||||
## Projected Impact
|
||||
|
||||
From the D4 10-session / 22-minute run (916 agent-side `gh` calls):
|
||||
|
||||
| Pattern | Before | After | Saved |
|
||||
|---------|--------|-------|-------|
|
||||
| `gh pr list --head` | 598 | ~10 | ~588 (98%) |
|
||||
| `gh issue view` | 75 | ~20 | ~55 (73%) |
|
||||
| **Subtotal (in scope)** | **673** | **~30** | **~643 (96%)** |
|
||||
| Everything else (unchanged) | 243 | 243 | 0 |
|
||||
| **Total** | **916** | **~273** | **~643 (70%)** |
|
||||
|
||||
The two intercepts eliminate ~70% of all agent-side `gh` traffic and ~96% of the traffic they target.
|
||||
|
|
@ -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.2") // version marker matches
|
||||
.mockResolvedValueOnce("0.4.0") // version marker matches
|
||||
.mockRejectedValueOnce(new Error("ENOENT")); // AGENTS.md doesn't exist
|
||||
|
||||
await setupPathWrapperWorkspace("/workspace");
|
||||
|
|
@ -95,3 +100,91 @@ 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-discovery-");
|
||||
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("contains issue context cache intercept with 300s TTL", () => {
|
||||
expect(GH_WRAPPER).toContain('$1" == "issue" && "$2" == "view"');
|
||||
expect(GH_WRAPPER).toContain("issue-ctx-");
|
||||
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("populates PR discovery cache after gh pr create", () => {
|
||||
expect(GH_WRAPPER).toContain("pr/create)");
|
||||
expect(GH_WRAPPER).toContain("read_ao_metadata branch");
|
||||
expect(GH_WRAPPER).toContain("ao_cache_write");
|
||||
expect(GH_WRAPPER).toContain("pr-discovery-");
|
||||
});
|
||||
|
||||
it("still passes through unmatched commands", () => {
|
||||
expect(GH_WRAPPER).toContain('exec "$real_gh" "$@"');
|
||||
});
|
||||
|
||||
it("uses current wrapper version in trace logging", () => {
|
||||
expect(GH_WRAPPER).toContain("0.4.0");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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.2";
|
||||
const WRAPPER_VERSION = "0.4.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,71 @@ 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"
|
||||
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
|
||||
# max_age=0 means infinite TTL
|
||||
[[ "\$max_age" -eq 0 ]] 2>/dev/null && return 0
|
||||
local cached_ts now
|
||||
cached_ts=\$(cat "\$ts_file" 2>/dev/null) || return 1
|
||||
now=\$(date +%s)
|
||||
(( now - cached_ts < max_age ))
|
||||
}
|
||||
|
||||
ao_cache_read() {
|
||||
local cache_key="\$1"
|
||||
local cache_dir
|
||||
cache_dir="\$(ao_cache_dir)" || return 1
|
||||
cat "\$cache_dir/\${cache_key}.stdout"
|
||||
}
|
||||
|
||||
ao_cache_write() {
|
||||
local cache_key="\$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,7 +245,7 @@ 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
|
||||
|
||||
# Best-effort JSONL tracing for agent-side gh invocations.
|
||||
|
|
@ -206,8 +286,102 @@ log_gh_invocation() {
|
|||
|
||||
log_gh_invocation "\$@"
|
||||
|
||||
# Only capture output for commands we need to parse (pr/create).
|
||||
# All other commands pass through transparently without stream merging.
|
||||
# =============================================================================
|
||||
# Cacheable reads
|
||||
# =============================================================================
|
||||
|
||||
# ── 1. PR discovery: gh pr list --head <B> --limit 1 ────────────────────────
|
||||
# Infinite TTL for positive results (non-empty array). Never caches [].
|
||||
if [[ "\$1" == "pr" && "\$2" == "list" ]]; then
|
||||
_ao_head="" _ao_limit="" _ao_cacheable=true
|
||||
_ao_saved_args=("\$@")
|
||||
shift 2
|
||||
while [[ \$# -gt 0 ]]; do
|
||||
case "\$1" in
|
||||
--head) _ao_head="\$2"; shift 2 ;;
|
||||
--limit) _ao_limit="\$2"; shift 2 ;;
|
||||
--repo|--json) shift 2 ;; # consumed but not needed for cache key
|
||||
--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
|
||||
_ao_safe_branch=\$(printf '%s' "\$_ao_head" | tr -c 'a-zA-Z0-9._-' '-')
|
||||
_ao_cache_key="pr-discovery-\${_ao_safe_branch}"
|
||||
|
||||
if ao_cache_fresh "\$_ao_cache_key" 0 2>/dev/null; then
|
||||
ao_cache_read "\$_ao_cache_key"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Cache miss — call real gh, cache positive results
|
||||
_ao_tmpout="\$(mktemp)"
|
||||
trap 'rm -f "\$_ao_tmpout"' EXIT
|
||||
"\$real_gh" "\$@" > "\$_ao_tmpout" 2>&1
|
||||
_ao_exit=\$?
|
||||
cat "\$_ao_tmpout"
|
||||
if [[ \$_ao_exit -eq 0 ]]; then
|
||||
_ao_out=\$(cat "\$_ao_tmpout")
|
||||
_ao_trimmed=\$(printf '%s' "\$_ao_out" | tr -d '[:space:]')
|
||||
# Only cache non-empty positive results
|
||||
if [[ -n "\$_ao_trimmed" && "\$_ao_trimmed" != "[]" ]]; then
|
||||
printf '%s' "\$_ao_out" | ao_cache_write "\$_ao_cache_key" 2>/dev/null || true
|
||||
fi
|
||||
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_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 ;;
|
||||
--repo|--json) shift 2 ;;
|
||||
-*) 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_cache_key="issue-ctx-\${_ao_issue_id}"
|
||||
|
||||
if ao_cache_fresh "\$_ao_cache_key" 300 2>/dev/null; then
|
||||
ao_cache_read "\$_ao_cache_key"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
_ao_tmpout="\$(mktemp)"
|
||||
trap 'rm -f "\$_ao_tmpout"' EXIT
|
||||
"\$real_gh" "\$@" > "\$_ao_tmpout" 2>&1
|
||||
_ao_exit=\$?
|
||||
cat "\$_ao_tmpout"
|
||||
if [[ \$_ao_exit -eq 0 ]]; then
|
||||
cat "\$_ao_tmpout" | ao_cache_write "\$_ao_cache_key" 2>/dev/null || true
|
||||
fi
|
||||
exit \$_ao_exit
|
||||
fi
|
||||
fi
|
||||
|
||||
# =============================================================================
|
||||
# Write intercepts
|
||||
# =============================================================================
|
||||
|
||||
case "\$1/\$2" in
|
||||
pr/create)
|
||||
tmpout="\$(mktemp)"
|
||||
|
|
@ -239,6 +413,20 @@ case "\$1/\$2" in
|
|||
update_ao_metadata agentReportedState "\$report_state"
|
||||
update_ao_metadata agentReportedAt "\$(date -u +%Y-%m-%dT%H:%M:%SZ)"
|
||||
update_ao_metadata agentReportedPrIsDraft "\$report_draft"
|
||||
|
||||
# Populate PR discovery cache so subsequent gh pr list --head hits cache
|
||||
_branch="\$(read_ao_metadata branch 2>/dev/null)" || true
|
||||
if [[ -n "\$_branch" && -n "\$pr_url" && -n "\$pr_number" ]]; then
|
||||
_safe_branch=\$(printf '%s' "\$_branch" | tr -c 'a-zA-Z0-9._-' '-')
|
||||
_cache_key="pr-discovery-\${_safe_branch}"
|
||||
_cache_dir="\$(ao_cache_dir 2>/dev/null)" || true
|
||||
if [[ -n "\$_cache_dir" ]]; then
|
||||
_draft_val="\${report_draft:-false}"
|
||||
printf '[{"number":%s,"url":"%s","headRefName":"%s","isDraft":%s}]\\n' \
|
||||
"\$pr_number" "\$pr_url" "\$_branch" "\$_draft_val" \
|
||||
| ao_cache_write "\$_cache_key" 2>/dev/null || true
|
||||
fi
|
||||
fi
|
||||
fi
|
||||
|
||||
exit \$exit_code
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ import {
|
|||
existsSync,
|
||||
mkdirSync,
|
||||
unlinkSync,
|
||||
rmSync,
|
||||
readdirSync,
|
||||
statSync,
|
||||
openSync,
|
||||
|
|
@ -273,6 +274,14 @@ export function deleteMetadata(dataDir: string, sessionId: SessionId, archive =
|
|||
}
|
||||
|
||||
unlinkSync(path);
|
||||
|
||||
// Clean up per-session gh cache directory (.ghcache/<sessionId>/)
|
||||
const cachePath = join(dataDir, ".ghcache", sessionId);
|
||||
try {
|
||||
rmSync(cachePath, { recursive: true, force: true });
|
||||
} catch {
|
||||
/* best effort */
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -77,6 +77,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;
|
||||
|
|
@ -1256,6 +1261,8 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
|
|||
launchCommand,
|
||||
environment: {
|
||||
...environment,
|
||||
PATH: buildAgentPath(environment["PATH"] ?? process.env["PATH"]),
|
||||
GH_PATH: PREFERRED_GH_PATH,
|
||||
AO_SESSION: sessionId,
|
||||
AO_DATA_DIR: sessionsDir, // Pass sessions directory (not root dataDir)
|
||||
AO_SESSION_NAME: sessionId, // User-facing session name
|
||||
|
|
@ -1520,10 +1527,12 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
|
|||
};
|
||||
|
||||
// Setup agent hooks for automatic metadata updates
|
||||
// Also install shared ~/.ao/bin wrappers (gh/git intercept + cache) for all agents
|
||||
try {
|
||||
if (plugins.agent.setupWorkspaceHooks) {
|
||||
await plugins.agent.setupWorkspaceHooks(workspacePath, { dataDir: sessionsDir });
|
||||
}
|
||||
await setupPathWrapperWorkspace(workspacePath);
|
||||
} catch (err) {
|
||||
await cleanupWorktreeAndMetadata();
|
||||
throw err;
|
||||
|
|
@ -1607,6 +1616,8 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
|
|||
launchCommand,
|
||||
environment: {
|
||||
...environment,
|
||||
PATH: buildAgentPath(environment["PATH"] ?? process.env["PATH"]),
|
||||
GH_PATH: PREFERRED_GH_PATH,
|
||||
AO_SESSION: sessionId,
|
||||
AO_DATA_DIR: sessionsDir,
|
||||
AO_SESSION_NAME: sessionId,
|
||||
|
|
@ -2696,6 +2707,8 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
|
|||
launchCommand,
|
||||
environment: {
|
||||
...environment,
|
||||
PATH: buildAgentPath(environment["PATH"] ?? process.env["PATH"]),
|
||||
GH_PATH: PREFERRED_GH_PATH,
|
||||
AO_SESSION: sessionId,
|
||||
AO_DATA_DIR: sessionsDir,
|
||||
AO_SESSION_NAME: sessionId,
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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.2");
|
||||
}
|
||||
// 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.2");
|
||||
|
||||
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.2");
|
||||
}
|
||||
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.2");
|
||||
}
|
||||
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) : "";
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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";
|
||||
|
||||
|
|
@ -744,8 +737,8 @@ 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> {
|
||||
|
|
@ -761,8 +754,7 @@ function createCodexAgent(): Agent {
|
|||
resolvingBinary = null;
|
||||
}
|
||||
}
|
||||
if (!session.workspacePath) return;
|
||||
await setupPathWrapperWorkspace(session.workspacePath);
|
||||
// PATH wrappers are re-ensured by session-manager.
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue