refactor(runtime): enforce canonical model execution boundary - #736
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 0dea549a
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: drewstone_author · 2026-08-04T02:40:46Z
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 3 (2 low, 1 weak-concern) |
| Heuristic | 0.2s |
| Duplication | 0.0s |
| Interrogation | 196.9s (2 bridge agents) |
| Total | 197.1s |
💰 Value — sound
Collapses every model-execution escape hatch (public OpenAI-compat backend, backend resolver, low-level router clients, direct Python/fetch calls) into one canonical AgentProfile → createExecutor → streamAgentTurn boundary, enforced by a static checker — a coherent, in-grain consolidation with no ex
- What it does: Makes AgentProfile the sole authority for model/provider/prompt/tools/generation/retry across the whole repo. It (1) removes createOpenAICompatibleBackend, resolveAgentBackend, createPrimeIntellectBackend and the low-level routerChat*/routerToolLoop/routerBrain exports from the kernel; (2) demotes Router/PrimeIntellect to transport-only configs (endpoint+auth, no model); (3) adds profileChatClient
- Goals it achieves: One canonical model-execution seam so identity, metering, retries, and cost are owned by Runtime and cannot be bypassed; provider credentials never leave Runtime; eval receives canonical requests, measured receipts, and execution evidence rather than raw credentials; conflicting per-request behavior (model/temp/maxTokens/thinking) is rejected before transport; and the boundary cannot silently regr
- Assessment: Strong change on its merits. It builds directly on machinery the repo already had — streamAgentTurn is already documented as 'the ONE run-a-turn event-stream contract', AgentProfile and the profile-materialization contracts already exist (this PR extends their axes to modelProvider/modelReasoningEffort/modelMetadata rather than inventing a new concept), and createExecutor/streamAgentTurn are alrea
- Better / existing approach: none — this is the right approach. I checked for duplication and a better architecture: (a) verify-static-imports.mjs exists but is a dependency-import analyzer (proper-lockfile etc.), a different concern from model-provider call detection, so the new checker is not a reinvention; (b) profile-materialization.ts already existed and this PR extends rather than forks it; (c) the only alternative to t
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error
🎯 Usefulness — sound
A coherent, heavily-consumed enforcement of the repo's stated AgentProfile-as-sole-authority doctrine: it consolidates scattered provider-call paths into one profile-bound Runtime seam, fails loud on conflict, and closes a real silent-zero dollar accounting hole.
- Integration: Heavily reachable across the whole tree.
runBenchRouterTurn(bench/src/router-turn.ts:101) is consumed by 30+ bench modules (research-shot.ts, david-goliath.mts, humaneval-repair-gate.mts, trata-gate.mts, supervisor-arena.mts, every benchmark adapter, etc.).profileChatClient/profileOptimizerModelCall(src/runtime/profile-chat-client.ts:30,54) are consumed by 6 src paths (strategy.ts:320, ob - Fit with existing patterns: Exactly in the codebase's grain — it makes an existing stated doctrine mechanical rather than inventing a new pattern. CLAUDE.md §1.5 (the 'AgentProfile law', line 36) and docs/canonical-api.md's anti-reinvention table already mandate that behavior changes by authoring the profile and letting the substrate materialize it; this PR removes the escape hatches (low-level
routerChatWithUsage/`RouterC - Real-world viability: Built to fail loud, matching the repo's no-silent-zero house rule.
assertExactExecutorDeclaration(stream-agent-turn.ts:252-322) rejects model/reasoning mismatches between the executor plan and the profile BEFORE transport.assertProfileChatRequest(profile-chat-client.ts:305-336) rejects per-call model/temperature/maxTokens/thinking values that conflict with the profile.profileChatClientp - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
🔎 Heuristic Signals
🟡 Cruft: console debug added bench/src/atom-humaneval.mts
- console.log(
🟡 Cruft: magic number added bench/src/david-attribution.mts
+const MAX_TOKENS = Number(process.env.MAX_TOKENS ?? 1000)
💰 Value Audit
🟡 Boundary allowlist is hand-maintained and silently whitelist-only [maintenance] ``
scripts/check-model-execution-boundary.mjs:13 keeps a 5-entry
directTransportOwnersSet as the only escape from the fence. Adding a legitimate new Runtime-owned transport requires editing this set, and the failure message tells callers to 'use AgentProfile + streamAgentTurn' but does not mention the allowlist for the rare legitimate owner — so a future runtime adapter author will hit the fence and have to discover the allowlist by reading the script. Cheap to live with (the set is deliberately
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 449d76fb
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: drewstone_author · 2026-08-04T04:28:19Z
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 4 (2 low, 2 weak-concern) |
| Heuristic | 0.3s |
| Duplication | 0.0s |
| Interrogation | 141.5s (2 bridge agents) |
| Total | 141.8s |
💰 Value — sound
Removes every parallel model-execution back door and forces all paid calls through one profile-bound Runtime route (AgentProfile -> createExecutor -> streamAgentTurn), backed by a build-time AST gate — a coherent, grain-aligned consolidation of a drift problem the repo had been patching for weeks.
- What it does: Deletes the public low-level model-execution APIs (resolveAgentBackend, createOpenAICompatibleBackend, BackendRetryPolicy, the public routerChatWithUsage/WithTools/ToolLoop/Brain, streamRouterChatWithTools, chatCompletionsTransport, authoredWorker, canonicalizeAuthoredProfile, the ui-auditor in-process client, driver-loop-generator) and demotes the router HTTP client to an internal transport adapt
- Goals it achieves: One model-execution boundary: every paid call enters through one route, so model identity, retry count, usage, and cost can no longer drift across N hand-rolled paths. AgentProfile becomes the sole behavioral authority (model/provider/harness/reasoning/prompt/tools/generation); per-call overrides that conflict are rejected before transport (assertProfileChatRequest, profile-chat-client.ts:316). Ho
- Assessment: Sound and in the grain of the codebase. The git log of src/runtime/supervise/*.ts shows roughly six commits in the prior month each patching a symptom of the same root cause — parallel model-execution paths bypass metering and identity. This PR removes the root cause instead of patching again. The static-analysis gate makes the boundary enforceable rather than aspirational; the scanner handles imp
- Better / existing approach: none — this is the right approach. Searched src/bench/examples for a pre-existing profile->ChatClient bridge (grep profileChatClient|ChatClient|routerChatWithUsage) and for residual createOpenAICompatibleBackend/resolveAgentBackend use; found none. The new profile-chat-client.ts IS the consolidation target. The custom AST scanner fits the repo's existing verify-script pattern (verify-package-expor
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error
🎯 Usefulness — sound
Makes the codebase's already-documented central law (AgentProfile is the sole model-execution authority) statically enforceable and fail-closed, routing every previously-bypassing caller through one profile-checked Runtime port.
- Integration: Strongly wired, not speculative. The new entry points are reached today:
profileChatClientis called by 7+ internal runtime sites (audit-intent.ts:116, strategy.ts:320, strategy-author.ts:156, run-benchmark.ts:172, strategy-evolution.ts:696, observe.ts:146) plus examples;profileOptimizerModelCallis consumed by the official optimizer (bench/src/official-optimizer-config.mts:109); `assertExecu - Fit with existing patterns: This IS the established pattern, not a competitor to one. CLAUDE.md §1.5 states the AgentProfile law as the thing 'we keep forgetting'; docs/canonical-api.md holds the anti-reinvention decision table. The PR converts that doctrine into a CI gate and deletes the actual bypass surfaces (driver-loop-generator.ts, ui-auditor/in-process-client.ts — 2727 lines removed in commit 2ea1c4c) that were making
- Real-world viability: Holds up past the happy path. The static analyzer chases aliases (namespace imports, CommonJS
require, element-accessruntime['routerChatWithUsage']), resolves computed endpoints through template/binop/identifier initializer chains, and crucially flags UNRESOLVED computed URLs outside the non-model allowlist (check-model-execution-boundary.mjs:206-211) — the safe direction. Provider SDKs are b - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
🔎 Heuristic Signals
🟡 Cruft: console debug added bench/scripts/run-package-tests.mjs
- console.log(
🟡 Cruft: magic number added bench/src/david-attribution.mts
+const MAX_TOKENS = Number(process.env.MAX_TOKENS ?? 1000)
💰 Value Audit
🟡 Boundary scanner's directTransportOwners whitelist is manual [maintenance] ``
scripts/check-model-execution-boundary.mjs:21-27 hardcodes 5 files permitted to own provider transport. Adding a new Runtime-owned transport requires editing that Set or the build fails. This is intentional (making accidental new paths fail is the point), but the contract is implicit — a comment or a co-located test asserting the whitelist is closed-and-intentional would make future additions a deliberate act rather than a confusion. Not blocking.
🟡 Generation-field validation lives in agent-runtime, not agent-interface [better-architecture] ``
profileModelExecutionSettings (src/runtime/supervise/model-policy.ts) validates temperature/maxTokens/seed/toolChoice/extraBody/maxTurns/stream and rejects unknown metadata keys. The more canonical home is the AgentProfile schema in @tangle-network/agent-interface, so every consumer gets the same validation. This PR pins agent-interface@0.43.x (package.json:173) and is scoped to agent-runtime, so pushing the schema change upstream is correctly a separate PR — but worth filing as a follow-up so t
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
|
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 449d76fb
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: drewstone_author · 2026-08-04T05:27:14Z
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — bcaa068f
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: drewstone_author · 2026-08-04T05:48:21Z
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 1fbd4050
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: drewstone_author · 2026-08-04T06:48:24Z
|
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 2 (2 low) |
| Heuristic | 0.4s |
| Duplication | 0.0s |
| Interrogation | 298.5s (2 bridge agents) |
| Total | 298.9s |
💰 Value — sound
Converts the documented §1.5 'AgentProfile is the sole behavioral authority' rule into a statically-enforced boundary: removes 22 public execution shortcuts, funnels every paid model call through streamAgentTurn, and adds an AST-based lint that fails the package release gate on any parallel path.
- What it does: Removes every public path that could reach a model without going through an exact AgentProfile (createOpenAICompatibleBackend, resolveAgentBackend, routerChatWithUsage/WithTools/ToolLoop, routerBrain, streamRouterChatWithTools, driverLoopGenerator, chatCompletionsTransport, authoredWorker, canonicalizeAuthoredProfile, createPrimeIntellectBackend, uiAuditorProfile, etc.) and replaces them with thre
- Goals it achieves: Make the existing §1.5 doctrine ('you change an agent's behavior by changing its PROFILE: never by writing orchestration code around it') mechanically enforceable instead of convention. Eliminate the class of bugs where a product-specific if(backend==='router')... branch silently reaches a model outside accounting (the 'phantom 0 reads as a free call' failure mode named in router-client.ts:1-11).
- Assessment: Coherent and in the grain. The codebase has been telegraphing this for many versions: §1.5 of canonical-api.md states the rule as doctrine, #691 ('no harness is special — delete the name-keyed branches'), the bespoke pi-executor deletion, and chat-transport-executor's recent landing all prepared exactly this completion. The PR is net-negative in code: chat-transport-executor drops from ~600 to 212
- Better / existing approach: none — this is the right approach. Searched for existing primitives that could absorb the new adapters' jobs: (1) createIterableBackend and createSandboxPromptBackend in src/backends.ts:20,31 are intentionally narrower (iterable-only and sandbox-only respectively), so a third constructor binding profile+executor to AgentExecutionBackend is a missing piece, not a fork. (2) computeBackoff in src/con
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error
🎯 Usefulness — sound
A coherent enforcement of the codebase's stated AgentProfile-as-sole-model-authority law: every product path is wired through three new adapters, 22 removed shortcuts have a regression scanner and documented replacements, and the design matches the canonical pattern documented in CLAUDE.md and docs/
- Integration: Strongly wired. createProfileExecutionBackend is consumed by the conversation persona loop (src/conversation/run-persona.ts:88) and re-exported from src/conversation/index.ts:17 and src/index.ts:71. profileChatClient is called from five runtime paths — strategy (src/runtime/strategy.ts:320), strategy-author (src/runtime/strategy-author.ts:156), strategy-evolution (src/runtime/strategy-evolution.ts
- Fit with existing patterns: Exactly in the grain. CLAUDE.md:36 (§1.5, the AgentProfile law) already states an agent IS its full profile and behavior is changed by authoring the profile, never by writing harness-specific config or verify-loops. docs/canonical-api.md:134-137 already lists streamAgentTurn/collectAgentTurn as the one normalized turn atom, createProfileExecutionBackend as the AgentExecutionBackend adapter, and pr
- Real-world viability: Fail-loud by design, matching the house rule (CLAUDE.md:97 'No fallbacks; fail loud'). profileChatClient refuses request-vs-profile conflicts on model, temperature, maxTokens, and thinking (src/runtime/profile-chat-client.ts:317-340), rejects maxCostUsd as an unenforceable limit (line 268), and fails the turn if the transport-observed model differs from the profile's required model (line 199). Usa
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
🔎 Heuristic Signals
🟡 Cruft: console debug added bench/scripts/run-package-tests.mjs
- console.log(
🟡 Cruft: magic number added bench/src/david-attribution.mts
+const MAX_TOKENS = Number(process.env.MAX_TOKENS ?? 1000)
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 9b383551
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: drewstone_author · 2026-08-04T07:28:52Z
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 979906a3
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: drewstone_author · 2026-08-04T07:48:29Z
|
| State | Detail |
|---|---|
| Interrupted | webhook restarted |
No review verdict was produced for this run. Trigger a fresh review on the current PR head if the PR is still open.
tangletools · #736 · model: kimi-for-coding · updated 2026-08-04T07:51:08Z
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 37d83656
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: drewstone_author · 2026-08-04T08:07:22Z
tangletools
left a comment
There was a problem hiding this comment.
⚪ Value Audit — audit-incomplete
| Verdict | audit-incomplete |
| Concerns | 2 (2 low) |
| Heuristic | 0.4s |
| Duplication | 0.0s |
| Interrogation | 90.0s (2 bridge agents) |
| Total | 90.4s |
💰 Value — error
value agent produced no parseable value-audit JSON.
- Model: opencode/deepseek/deepseek-v4-pro
- Bridge attempts: 3
- Bridge error: opencode/kimi-for-coding/k2p7: Bridge returned 503: {"error":{"message":"cli-bridge admission timed out after 30000ms","type":"admission_rejected","reason":"queue_timeout","admission":{"active":20,"queued":8,"maxActive":20,"maxQueue":48}}}; opencode/zai-coding-plan/glm-5.2: Bridge returned 503: {"error":{"message":"cli-bridge admission timed out after 30000ms","type":"admission_rejected","reason":
🎯 Usefulness — error
usefulness agent produced no parseable value-audit JSON.
- Model: opencode/deepseek/deepseek-v4-pro
- Bridge attempts: 3
- Bridge error: opencode/zai-coding-plan/glm-5.2: Bridge returned 503: {"error":{"message":"cli-bridge admission timed out after 30000ms","type":"admission_rejected","reason":"queue_timeout","admission":{"active":20,"queued":9,"maxActive":20,"maxQueue":48}}}; opencode/kimi-for-coding/k2p7: Bridge returned 503: {"error":{"message":"cli-bridge admission timed out after 30000ms","type":"admission_rejected","reason":
🔎 Heuristic Signals
🟡 Cruft: console debug added bench/scripts/run-package-tests.mjs
- console.log(
🟡 Cruft: magic number added bench/src/david-attribution.mts
+const MAX_TOKENS = Number(process.env.MAX_TOKENS ?? 1000)
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
What
AgentProfilethe only authority for model, provider, prompt, tools, resources, harness, and generation behaviorcreateProfileExecutionBackendAgentProfile.model.metadata.retryand composes agent-core 0.4.35Direct proof
origin/main: tree752f489d4180487299339129e4befb9ca195077fThe final lint/package/Bench/Python optimizer jobs are being rerun on this exact head; GitHub CI is authoritative before merge.