port: sync with @openrouter/agent upstream (EVAL FAILED — do not merge)

[openrouter-port-bot]

Automated Upstreamer port of @openrouter/agent into this repo.

• Contract: .upstreamer/upstreamer.md
• Run log: .upstreamer/logs/
• Parity eval: .upstreamer/eval-report.md
• Parity eval passed: false

Review the diff as a port, not as a normal PR: check behavioral parity
against the TypeScript reference, not just that it compiles. If
.upstreamer/state.yaml did not advance, the eval did not pass and this
PR must not be merged as-is.

[perry-the-pr-reviewer]

Perry's Review

Verdict: 🔁 Needs changes

This is a large, high-quality port of the doom-loop detection system, usage accounting, tool-choice relaxation, strict serialization, and several other features from @openrouter/agent. The ported code is faithful — the RFC 8785 canonical JSON serializer, fingerprint vectors, and graduated action ladder all match upstream — and the test coverage is strong (32 monitor + 30 integration + 5 fingerprint conformance tests). However, two blockers prevent merge:

1. CI lint is red — staticcheck SA4006 on doom_loop_engine.go:179 (keyMaterial := any(args) is a dead initial assignment, overwritten in both branches of the if/else). A one-line fix, but it must be green.
2. The parity eval failed — upstream commit e8d7d6d (#90, the entire async tool lifecycle: background/deferred tools, ToolTask, the universal task tool, per-tool concurrency/timeout, subagents) is not ported at all. The PR title and eval report both declare this. The eval report correctly blocks state advancement and recommends pinning .upstreamer/state.yaml to the commit immediately before #90 to bank the verified work.

The non-blocker findings below are suggestions and observations folded into this summary.

Details

Blockers

1. Staticcheck SA4006 — dead assignment in checkDoomLoopBeforeExecution

doom_loop_engine.go:179: keyMaterial := any(args) assigns a value that is overwritten in both branches of the if !parsed { ... } else { ... } before being read. Staticcheck flags this as SA4006, which fails the lint CI job. Replace with var keyMaterial any so the intent (assign in both branches, read once after) is clear to both the reader and the linter.

2. Parity eval FAIL — upstream #90 not ported

The eval report (.upstreamer/eval-report.md) declares FAIL because upstream e8d7d6d (#90) has no Go counterpart. This is ~2,000 lines of Go across 7 new files plus ~86 upstream test cases — a port cycle of its own. The eval report's recommendation is sound: pin state to the commit immediately before #90 (1ac98c8) to bank this run's verified work, leaving e8d7d6d..5a7ed03 as the next delta. This needs a human decision (the contract is repo-owned and cannot be amended by a port run).

Suggestions (non-blocking)

• Malformed tool-call arguments retained (stream_transformers.go): Previously, a function_call with unparseable JSON arguments was silently dropped from ExtractToolCallsFromResponse, hiding it from the model's history and doom-loop detection. Now it's surfaced with RawArgs only and produces an error tool output. This matches upstream and closes a real gap — a model stuck emitting invalid JSON would bounce off the parse error forever. Good fix.
• Strict tool serialization (tool.go): strict was unconditionally true for every function tool; now it's absent unless declared via ToolConfig.Strict *bool. This is a documented breaking change but correct — upstream serializes strict: tool.function.strict ?? null. Tests cover the three cases (absent, true, false). Note that schemas with optional properties may be rejected by providers in strict mode; this is documented.
• Forced tool-choice relaxation (model_result.go): A forced ToolChoice (required, specific tool, allowed_tools with mode: required) is now one-shot, relaxed to auto after the first turn emits a tool call. The consumed key is persisted across pauses. Engine-owned overrides (final none, doom-loop advisor) still win. 9 tests cover the core cases. The multi-follow-up and manual-client-resume sub-cases remain uncovered — worth backfilling in the #90 port cycle.
• TurnContext.ToolCall populated on streaming path (tool_context.go): Previously only the non-streaming orchestrator set Turn.ToolCall, so hooks and Execute functions reading it saw nil on the streaming path. Now BuildToolExecuteContext threads the executed call via toFunctionCallItem, preferring RawArgs (byte-exact wire string). Good fix.
• ModelResult.Usage(ctx) (model_result.go): Aggregate token/cost totals across every model call. Cost is nil when never reported. The divergence from upstream's getUsage() (which doesn't advance the tool loop on approval-resume reads) is documented and correct for Go's single-loop-to-completion model.

Observations

• Doom-loop detection (doom_loop.go + doom_loop_engine.go, ~2,400 lines): The largest piece is a faithful, deterministic, fail-safe translation. The RFC 8785 canonical JSON serializer is hand-rolled (Go's encoding/json HTML-escapes and formats numbers differently) and well-tested against upstream conformance vectors. The graduated action ladder (observe → steer → escalate → block → stop), round-scoped streaks with per-call detection, state persistence/resume, text repetition detection, and server-tool fingerprinting are all correct. The KNOWN RESIDUAL (HITL mid-round phantom members) is documented and fail-safe.
• mcpBrand ToolLoopKey forwarding (tool.go): MarkMcp now accepts McpMarkOptions{LoopKey: ...} and mcpBrand.ToolLoopKey() correctly checks the override first, then delegates to the wrapped tool. The loopKey/hasLoopKey fields are on the value-receiver struct, which is fine since MarkMcp returns a new mcpBrand value.
• Concurrency safety: The doom-loop monitor has its own mutex. ModelResult's doom-loop fields are accessed only from run() (single goroutine). m.mu guards the public-facing fields (resp, toolCalls, steps). No data races observed.

Risk: 🟡 Medium

Risk assessment:

Dimension	Severity	Risk	Reasoning
Implementation risk	🟨🟨	Medium	The ported code is high quality, but the lint failure indicates a dead-code path, and the uncovered tool-choice-relaxation sub-cases are a regression risk.
Premise risk	🟩	Low	The PR honestly declares the eval failure and does not advance state — the premise is sound. The eval report's recommendation (pin to pre-#90) is the right call.
Estimated impact	🟨🟨	Medium	If merged as-is, the async tool lifecycle gap is effectively permanent (state advances past #90, and the gap never appears in a future delta). The ported features themselves are low-impact additions (doom-loop is opt-in, strict is a documented behavior change).

Risk Factor	Severity	Risk	Reasoning
Reversibility	🟩	Low	The ported code is additive; reverting is straightforward.
Detectability	🟩	Low	Doom-loop detection is opt-in and deterministic; bugs surface immediately in tests.
Blast radius	🟩	Low	New features behind opt-in flags; existing API paths are unaffected.
Data integrity	🟩	Low	No persisted data is mutated beyond the new DoomLoop state blob, which round-trips correctly.
Financial exposure	🟩	Low	Doom-loop detection reduces spend, not increases it.
Security and privacy exposure	🟩	Low	No auth, credential, or tenant-isolation surface touched.
Propagation	🟩	Low	The ported features are consumed only by callers who opt in.
Availability	🟩	Low	The doom-loop stop action halts a run, but only when detection is enabled and a loop is confirmed.
Recovery cost	🟩	Low	A revert or fix is a standard rollback.
Time to correct	🟨🟨	Medium	The lint fix is trivial (1 line); the #90 port is a separate cycle (~2,000 lines + tests).

Estimated impact: Medium — the worst case is that the async tool lifecycle gap becomes permanent if state is advanced past #90, silently dropping the capability from every future port delta. The eval report correctly identifies this and blocks state advancement.

[perry-the-pr-reviewer]

▶ Fix the SA4006 lint failure (CI blocker)

keyMaterial := any(args) assigns a value that is overwritten in both branches of the if !parsed { keyMaterial = call.RawArgs } else { ... keyMaterial = resolution.KeyMaterial } before being read — staticcheck flags this as SA4006: this value of keyMaterial is never used, which fails the lint CI job.

Replace with var keyMaterial any so the intent (assign in both branches, read once after) is clear:

var keyMaterial any

This is the only CI failure — check, e2e, and verify-port all pass.

[perry-the-pr-reviewer]

▶ Do not advance state past #90 — pin to the pre-#90 commit instead

The eval report correctly declares FAIL and leaves state.yaml un-advanced. The blocking finding (C1) is that upstream e8d7d6d (#90, the async tool lifecycle) has no Go counterpart. The report's recommendation to pin state to the commit immediately before #90 (1ac98c8) is the right call: it banks this run's verified doom-loop / usage / tool-choice / strict work while leaving e8d7d6d..5a7ed03 as the next delta, so #90 is never silently dropped.

This needs a human decision — the scope contract (.upstreamer/upstreamer.md) is repo-owned and cannot be amended by a port run. Until either #90 is ported (~2,000 lines of Go + ~86 upstream test cases) or the contract is amended to declare the async-tool surface out of scope, this PR should not be merged as-is.