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

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>
This commit is contained in:
Dhruv Sharma 2026-04-16 14:38:41 +05:30
parent 5b0a24138c
commit 7a65c3cb61
2 changed files with 29 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
experiments/a2-baseline-runbook.md
View File

@ -1,7 +1,7 @@
# A2 Baseline Runbook
**Purpose:** Practical execution plan for the Phase A2 scenario x scale x topology matrix.
**Prereq:** A1b blockers 1-4 closed, clean rerun validates tracer visibility.
**Prereq:** A1b blockers 1-4 closed, clean rerun validates tracer visibility. Blocker 5 (`sessionId`/`projectId` threading) must also land before running per-session cells (S2 at scale >1, S3, S4) — without it, per-session attribution is not measurable and the "per-session polling floor" claims in the baseline are not backed by data.
**Output:** `experiments/baseline.md` — the single artifact that gates Track B.
---
@ -26,13 +26,13 @@ Every A2 run must stay inside a single rate-limit reset window (~60 min, resets
Before and after each run, capture a `/rate_limit` snapshot to bracket the coarse subcommand burn (Gap 1 — CLI subcommands are opaque to the tracer):
```bash
# Before run
gh api /rate_limit --jq '.resources.core' > experiments/out/rate-limit-before.json
date -u +%Y-%m-%dT%H:%M:%SZ >> experiments/out/rate-limit-before.json
# Before run — produces valid JSON with embedded timestamp
gh api /rate_limit --jq '{ core: .resources.core, captured_at: now | todate }' \
> experiments/out/rate-limit-before.json
# After run
gh api /rate_limit --jq '.resources.core' > experiments/out/rate-limit-after.json
date -u +%Y-%m-%dT%H:%M:%SZ >> experiments/out/rate-limit-after.json
gh api /rate_limit --jq '{ core: .resources.core, captured_at: now | todate }' \
> experiments/out/rate-limit-after.json
```
### Test repos
@ -80,15 +80,17 @@ Pruning rule from PLAN.md: run full matrix once, keep only cells that show meani
**Priority cells (run first):**
| Cell | Why |
|------|-----|
| S2-T1-1 | Single-session polling floor. Everything else is measured relative to this. |
| S2-T1-5 | Does cost scale linearly with sessions? |
| S2-T1-50 | Target capacity steady state. THE critical cell. |
| S2-T2-50 | Spread vs concentrated at target. Shows detectPR fan-out impact. |
| S1-T1-50 | Cold start at target. Shows cache-miss storm severity. |
| S3-T1-25 | Spawn storm. Shows burst shape. |
| S4-T1-10 | Review burst. Shows reaction-path cost. |
| Cell | Why | Needs blocker 5? |
|------|-----|-------------------|
| S2-T1-1 | Single-session polling floor. Everything else is measured relative to this. | No (1 session) |
| S2-T1-5 | Does cost scale linearly with sessions? | **Yes** (per-session split) |
| S2-T1-50 | Target capacity steady state. THE critical cell. | **Yes** (per-session split) |
| S2-T2-50 | Spread vs concentrated at target. Shows detectPR fan-out impact. | **Yes** (per-session split) |
| S1-T1-50 | Cold start at target. Shows cache-miss storm severity. | **Yes** (per-session split) |
| S3-T1-25 | Spawn storm. Shows burst shape. | **Yes** (per-session split) |
| S4-T1-10 | Review burst. Shows reaction-path cost. | **Yes** (per-session split) |
Only S2-T1-1 (single session) produces meaningful per-session data without blocker 5. All multi-session cells can still measure **total** burn and scorecard metrics, but cannot attribute cost per session.
---
@ -104,7 +106,8 @@ export AO_GH_TRACE_FILE="$PWD/experiments/out/a2-${SCENARIO}-${TOPO}-${SCALE}-$(
# (specific config varies per topology)
# Bracket: capture /rate_limit before
gh api /rate_limit --jq '.resources.core' | tee experiments/out/rl-before-${SCENARIO}-${TOPO}-${SCALE}.json
gh api /rate_limit --jq '{ core: .resources.core, captured_at: now | todate }' \
| tee experiments/out/rl-before-${SCENARIO}-${TOPO}-${SCALE}.json
```
### 2. Run
@ -127,7 +130,8 @@ ao stop <projectId>
```bash
# Bracket: capture /rate_limit after
gh api /rate_limit --jq '.resources.core' | tee experiments/out/rl-after-${SCENARIO}-${TOPO}-${SCALE}.json
gh api /rate_limit --jq '{ core: .resources.core, captured_at: now | todate }' \
| tee experiments/out/rl-after-${SCENARIO}-${TOPO}-${SCALE}.json
# Summarize
node experiments/summarize-gh-trace.mjs "$AO_GH_TRACE_FILE"

12
packages/core/src/gh-trace.ts
View File

@ -82,7 +82,11 @@ function extractOperation(args: string[]): string {
}
continue;
}
return `gh.${args[0]}.${arg}`;
// 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]}`;
}
@ -172,7 +176,7 @@ function buildTraceEntry(
result: GhTraceResult,
durationMs: number,
): GhTraceEntry {
const { statusLine, headers } = parseIncludedHttpResponse(result.stdout);
const { statusLine, headers } = parseIncludedHttpResponse(result.stdout ?? "");
const httpStatus = statusLine
? Number.parseInt(statusLine.replace(/^HTTP\/[0-9.]+\s+/, "").split(" ")[0] ?? "", 10)
: undefined;
@ -191,8 +195,8 @@ function buildTraceEntry(
exitCode: result.exitCode,
signal: result.signal,
durationMs,
stdoutBytes: Buffer.byteLength(result.stdout, "utf-8"),
stderrBytes: Buffer.byteLength(result.stderr, "utf-8"),
stdoutBytes: Buffer.byteLength(result.stdout ?? "", "utf-8"),
stderrBytes: Buffer.byteLength(result.stderr ?? "", "utf-8"),
statusLine,
httpStatus: Number.isFinite(httpStatus) ? httpStatus : undefined,
etag: headers["etag"],