Files
OmniRoute/docs/sessions/20260714-issue-agent-executable-triage/04_IMPLEMENTATION_STRATEGY.md
KooshaPari f657c7865a feat(issue-agent): surface RecordedTriageTimeoutError as 504 (#7315)
* chore(ci): add .mergify.yml to main — Mergify only reads config from the default branch (#7168)

* feat: scaffold issue agent and router eval provenance

* feat: wire recorded issue triage runner

* feat: ingest recorded issue context

* feat: persist issue agent audit log

* feat: import recorded github issue exports

* docs: document issue agent env toggle

* fix(issue-agent): validate run requests

* fix(ci): add the auto-enqueue pull_request_rule to the Mergify config (queue_conditions alone are eligibility-only) (#7179)

* fix: validate issue agent run requests

* docs: add issue agent execution traceability

* feat(issue-agent): route recorded triage through chat

* test(issue-agent): verify recorded triage through chat route

* docs(issue-agent): add executable triage session artifacts

* fix(ci): migrate Mergify auto-enqueue to merge_protections_settings.auto_merge_conditions (rules-based path is EOL 2026-07-16) (#7216)

* fix(ci): drop Mergify batch settings (batching is a paid-tier feature; free plan queue is serial) (#7220)

* feat(issue-agent): surface RecordedTriageTimeoutError as 504

When the recorded-triage chat completion times out, the AbortController
fires an AbortError that previously surfaced as a generic 400 to the
caller. This change:

  * Adds a `RecordedTriageTimeoutError` that wraps the AbortError
    with the timeoutMs context.
  * Re-throws it from `executeRecordedTriageChatCompletion` so the
    caller can distinguish timeouts from other failures.
  * In the runs route, catches it and returns a 504 with code
    `ISSUE_AGENT_TIMEOUT` so clients can render a useful error.

Tests:
  * issue-agent-execution.test.ts        — verifies the typed error
  * issue-agent-route-execution.test.ts  — covers timeout path
  * issue-agent-runs-route.test.ts       — verifies 504 mapping

* fix(ci): merge queue tolerates the advisory dast-smoke failure (its GH-hosted build hang dequeued every attempt) (#7225)

* test(ci): make the #6634 selfref guard hermetic — main's copy hard-fails every PR (#7341)

main's copy of this test still does git I/O inside a unit test:

    const baseSrc = git(['show', 'origin/main:' + FILE]);

Runners check out a shallow single ref, so origin/main does not resolve and the
test dies with 'fatal: invalid object name origin/main'. Every PR into main
fails Unit Tests (7/8) on it — today that is #7313, #7315, #7316, #7334, #7336
and #7337, six PRs red on a defect none of them introduced. #7313 has no other
red at all.

release/v3.8.49 already carries a fix (2e42b8efc, #7174: try/catch, fetch
origin/main on demand, t.skip() when unreachable), but it only reaches main at
release time — so main stays broken for the whole cycle. Cherry-picking it would
also import a new problem: PR Test Policy classifies t.skip() as a silenced
assertion, which we watched it correctly catch on #7300 today.

This is the hermetic version instead (ported from #7327, which does the same for
the release branch): read the file straight off disk, compare against an empty
base so baseTaut/baseExtTaut are 0 — the strictest possible comparison point —
and call evaluateMasking() directly. No git ref, no fetch, no skip, nothing the
runner's checkout depth can break.

The #6634 regression stays covered: the guard's logic lives in
SELF_TEST_FIXTURE_RE (check-test-masking.mjs:337), not in the test. Proven both
ways on main before committing — neutralise SELF_TEST_FIXTURE_RE to /$^/ and
the test FAILS; restore it and it passes 2/2, with check-test-masking.mjs left
byte-identical.

Co-authored-by: growab <nekron@icloud.com>

* chore(quality): tighten main's coverage baseline to the CI's real numbers (#7347)

main's ratchet had been failing --require-tighten on every PR: 11 metrics
improved but the baseline was never tightened. Same class as the #6634
selfref guard — an infra fix that lands only on the release branch leaves
main red for the whole cycle, and every PR into main pays for it.

Values are the merged-coverage numbers from a run on main itself (a local
run measures ~68% vs CI's ~80%; the baseline's own note warns about that
gap). Only the 11 coverage values change — gitleaks and semgrepFindings
keep main's own state.

No changelog fragment: #7326 carries it on release/v3.8.49, and a second
one here would double the entry at release time.

* fix(issue-agent): dryRun-explicit test fixture + sanitize the generic error catch

Two independent Hard Rule #12/#18 fixes on the recorded-triage runs route:

1. tests/unit/issue-agent-route-execution.test.ts's "preserves the normal
   chat route provider failure response" test omitted dryRun from its
   request body. createRecordedTriageRun() computes `dryRun: input.dryRun
   !== false`, so an omitted dryRun defaults to true (dry-run mode) and the
   route returns the deterministic dry-run summary WITHOUT ever calling
   executeRecordedTriageChatCompletion() -- the mocked 429 fetch was never
   invoked, so the test always observed the 200 dry-run response instead.
   Add the missing `dryRun: false`, matching the sibling test above it. The
   normal chat-completions route also enriches upstream errors with a
   "[provider/model] [status]:" prefix and a connection-cooldown hint
   (RESILIENCE_GUIDE.md) rather than passing them through byte-for-byte, so
   the assertion now checks the original message survives (status +
   substring) instead of exact-matching the mocked JSON shape.

2. src/app/api/issue-agent/runs/route.ts's generic (non-timeout) catch
   returned raw `error.message` straight to the client. Validation failures
   thrown by this module (bad issue URL, malformed GitHub export) are safe,
   curated messages -- but appendIssueAgentAuditRecord()'s mkdir/appendFile
   under DATA_DIR can throw a real Node fs error (ENOENT/EACCES/EEXIST, ...)
   whose raw `.message` embeds the server's absolute filesystem path. Add
   isNodeSystemError() (keys off NodeJS.ErrnoException's `.code`, which only
   Node's own fs/system errors set) to replace that class of error with a
   generic message, and route everything else through sanitizeErrorMessage()
   per Hard Rule #12. Add a regression test that forces a real audit-write
   failure (pre-creating a file where audit.ts expects to mkdir) and asserts
   the response contains neither the errno code nor the DATA_DIR path.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com>
Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: growab <nekron@icloud.com>
2026-07-18 15:13:35 -03:00

3.1 KiB

Issue-Agent Executable Triage: Implementation Strategy

Machine status: in_progress

Phase Plan

phase work package dependency exit evidence status
P1 verify/finish routing-policy contract and failure semantics existing AC1 seam actual chat route test proves policy consumption and non-2xx mapping in_progress
P2 introduce durable execution lifecycle persistence P1 records transitions, request/prompt, output, accounting, terminal error pending
P3 normalize actionable triage result P2 stable API result schema derived from completion pending
P4 implement terminal outcome controls P2 provider failure, timeout, budget stop transition tests pending
P5 release validation and PR review P1-P4 focused tests, route gate, relevant typecheck/CI evidence pending

Architecture

  1. Keep src/app/api/issue-agent/runs/route.ts as the API adapter: validation, feature gate, and response formatting only.
  2. Keep the standard chat POST as the routing boundary; do not add a parallel provider invocation path.
  3. Extract lifecycle persistence and result normalization into focused src/lib/issueAgent/ modules. Do not overload the existing pre-execution audit writer with unrelated transport behavior.
  4. Use typed execution outcomes so provider failure, abort/timeout, and budget termination are distinguishable before HTTP mapping and persistence.
  5. Add tests from the actual route down to a mocked external provider boundary; use unit tests for pure normalization and lifecycle state transitions.

Quality Controls

control command or review threshold
route contract npm run check:route-validation:t06 pass
AC1 route behavior focused bun test issue-agent route/execution suites policy and provider/model assertions pass
AC2-AC4 lifecycle/result/terminal-outcome suites all required states persist and API matches
static safety npm run typecheck:core distinguish new failures from existing omniglyph blocker
patch integrity git diff --check origin/main...HEAD pass