Skip to content

feat(candidate): execute through exact process providers#574

Merged
drewstone merged 4 commits into
mainfrom
feat/environment-candidate-executor
Jul 21, 2026
Merged

feat(candidate): execute through exact process providers#574
drewstone merged 4 commits into
mainfrom
feat/environment-candidate-executor

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

Problem

Candidate execution was coupled to a Sandbox-specific adapter and could not reuse the exact process contract across environment providers.

Change

  • replace the Sandbox-only executor with a provider-neutral exact-process executor
  • expose a thin Intelligence adapter for measured candidate experiments
  • require @tangle-network/agent-interface 0.32.0 for the exact process contract
  • publish agent-runtime 0.102.0

Checks

  • pnpm test: 151 files, 1,582 passed, 2 skipped
  • pnpm typecheck
  • pnpm lint
  • pnpm docs:check
  • pnpm verify:package

No product activation path is changed in this PR.

tangletools
tangletools previously approved these changes Jul 20, 2026

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — ceb6f747

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-07-20T22:45:00Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Value Audit — sound-with-nits

Verdict sound-with-nits
Concerns 1 (1 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 293.1s (2 bridge agents)
Total 293.1s

💰 Value — sound

Clean swap of the Sandbox-only candidate executor for a provider-neutral one over the new exactProcess contract; old code fully removed, new code is the single AgentCandidateExecutorPort implementation and matches the codebase's stated direction.

  • What it does: Deletes src/intelligence/sandbox-approved-candidate.ts (681 lines, SandboxClient-coupled) and its test, and replaces them with src/candidate-execution/exact-process-executor.ts (730 lines, new) implementing AgentCandidateExecutorPort against provider.exactProcess (the slot added in @tangle-network/agent-interface 0.32.0), plus a 117-line Intelligence-layer wrapper (src/intelligence/exact-process-c
  • Goals it achieves: Decouple the 'measure a candidate before promoting it' executor from the @tangle-network/sandbox SDK so any provider that declares exactProcess + egress capability can host a candidate cell. Aligns the candidate-execution seam with the existing AgentEnvironmentProvider abstraction already driving runtime/environment-provider.ts and runtime/supervise/runtime.ts (where SandboxClient is the legacy su
  • Assessment: Sound and in-grain. (1) It is a true replacement, not a parallel path: git grep shows ExactProcessAgentCandidateExecutor is the only AgentCandidateExecutorPort implementation in src/; the old SandboxAgentCandidateExecutor and the createSandboxCandidateExperimentExecutor / sandboxCandidateExperimentExecutionSupport exports are gone from src, tests, docs, and scripts/verify-package-exports.mjs. (2)
  • Better / existing approach: none — this is the right approach. Searched for and ruled out: (a) other AgentCandidateExecutorPort implementations to extend — none exist, the deleted Sandbox one was the only other; (b) reuse of providerAsExecutor from src/runtime/environment-provider.ts:278 — different port (the runtime Executor for multi-turn agents vs. the one-shot candidate cell port), different lifecycle, not a candidate fo
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound-with-nits

Clean refactor of the candidate executor from a Sandbox-SDK-coupled adapter to a provider-neutral exact-process adapter, matching the repo's documented migration plan; structurally isomorphic to the deleted predecessor and wired through the same Intelligence experiment cell.

  • Integration: The new exactProcessProviderAsCandidateExecutor (src/candidate-execution/exact-process-executor.ts:61) implements the existing AgentCandidateExecutorPort (execute/stop/capture/dispose) and is consumed by createExactProcessCandidateExperimentExecutor in src/intelligence/exact-process-candidate.ts:73, which in turn drives the unchanged executeAgentCandidateExperimentCell (src/intelligence/im
  • Fit with existing patterns: Fits the established grain precisely. The predecessor src/intelligence/sandbox-approved-candidate.ts (deleted in 57fb0b6) was the only Sandbox-only outlier in an otherwise provider-neutral stack: docs/research/environment-provider-adapter-spec.md:28-48 explicitly calls for lifting execution above the SandboxInstance SDK shape so third-party providers implement one contract. The new executor pr
  • Real-world viability: The new executor's error and concurrency paths mirror the deleted predecessor almost line-for-line: signal-linked cancellation (exact-process-executor.ts:198-218), idempotency keys derived from executionId+plan digest (line 673), fresh-environment invariant check (line 175-178), terminal-status assertion across wait() vs status() (line 632-642), output size enforcement with cleanup-on-exceed (
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🎯 Usefulness Audit

🟡 In-tree sandbox provider adapter does not yet expose exactProcess [integration] ``

exactProcessProviderAsCandidateExecutor requires options.provider.exactProcess and the executor throws at construction if absent (src/candidate-execution/exact-process-executor.ts:77-81). The only in-tree AgentEnvironmentProvider adapter, sandboxClientAsProvider (src/runtime/environment-provider.ts:218), does not populate capabilities.exactProcess nor wrap SandboxInstance.process.spawnExact into the AgentExactProcessProvider shape — so until either that adapter is extended in a fol


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.

value-audit · 20260720T225150Z

@drewstone

Copy link
Copy Markdown
Contributor Author

Follow-up to the failed agent-bench check: the Pier candidate fixture now declares its public PATH, and agent-bench is released as 0.3.7 with agent-interface 0.32.0 so its packed manifest matches agent-runtime 0.102.0. Local proof: pnpm run verify:bench (build, public types, 25 benchmark files plus Pier, packed consumer on TypeScript 5.9.3 and 6.0.3); pnpm test (151 files, 1,582 passed, 2 skipped); pnpm typecheck; pnpm lint; pnpm verify:package; pnpm docs:check. The in-tree sandboxClientAsProvider is deliberately not the product integration target: products will use @tangle-network/agent-provider-tangle@0.3.0, which owns the Sandbox SDK exact-process mapping. Extending the in-tree adapter would duplicate that provider.

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 711833ba

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-07-20T23:27:50Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Value Audit — sound-with-nits

Verdict sound-with-nits
Concerns 1 (1 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 273.5s (2 bridge agents)
Total 273.5s

💰 Value — sound

Cleanly decouples Intelligence's measured-candidate experiment executor from the Sandbox SDK by routing it through the existing AgentEnvironmentProvider's new exact-process contract — a coherent, mostly 1:1 port that matches the codebase's grain.

  • What it does: Deletes src/intelligence/sandbox-approved-candidate.ts (681 lines) and src/intelligence/sandbox-approved-candidate.test.ts (598 lines) — an executor that called sandbox.process.spawnExact directly on @tangle-network/sandbox's SandboxInstance. Replaces them with src/candidate-execution/exact-process-executor.ts (730 lines) + src/intelligence/exact-process-candidate.ts (117 lines) + tests/exact-proc
  • Goals it achieves: Read from the change itself: (1) Make the measured-candidate experiment path provider-neutral so any environment provider that declares exact-process capability can host it, not only the Tangle Sandbox. (2) Collapse the last Intelligence-side bypass of the repo's existing AgentEnvironmentProvider abstraction (src/runtime/environment-provider.ts:178-264 already defines sandboxClientAsProvider / pro
  • Assessment: Coherent and in-grain. The new ExactProcessAgentCandidateExecutor preserves the old invariants almost line-for-line — execute/stop/capture/dispose lifecycle, immutable candidate material + idempotency key, recovery via list-and-match-by-metadata, fresh-environment pre-check, signed-PATH requirement, output-byte cap, cancellation-then-await ordering, trace-once append. The extensions it adds (cpu/m
  • Better / existing approach: none — this is the right approach. I searched for an existing equivalent (grep AgentExactProcessProvider / spawnExact across src/, tests/, bench/) and confirmed: (a) nothing in this repo previously implemented AgentExactProcessProvider — the contract is new in agent-interface 0.32.0; (b) the existing sandboxClientAsProvider (src/runtime/environment-provider.ts:218) only adapts provider.create/get/
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound-with-nits

Replaces a Sandbox-coupled candidate executor with a provider-neutral one that matches Runtime's established AgentEnvironmentProvider pattern, fully wired into the existing experiment-cell pipeline with the bench already updated for the new contract.

  • Integration: Reachable and correctly wired. exactProcessProviderAsCandidateExecutor (src/candidate-execution/exact-process-executor.ts:61) produces an AgentCandidateExecutorPort consumed by executeAgentCandidateExperimentCell (src/intelligence/improvement-cycle.ts:226) via executePreparedAgentCandidate. createExactProcessCandidateExperimentExecutor (src/intelligence/exact-process-candidate.ts:73) is the produc
  • Fit with existing patterns: Excellent. The deleted src/intelligence/sandbox-approved-candidate.ts (681 lines) was a parallel Sandbox-specific executor that duplicated the same lifecycle (idempotency, recovery, evidence capture, output bounds) the runtime layer already standardized on. This PR collapses that fork onto the shared provider contract. No competing pattern remains: providerAsExecutor is the established way Runtime
  • Real-world viability: Robust. The executor handles: abort linkage via addEventListener('abort', cancel, {once:true}) plus an explicit re-check (src/candidate-execution/exact-process-executor.ts:203-204); idempotency keys derived from executionId+plan digest (line 152-155, 673-679); AggregateError when validation AND cleanup both fail (line 165-169); output byte-cap enforcement with kill-on-overflow (collectOutput, line
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🎯 Usefulness Audit

🟡 Stale reference to removed createSandboxCandidateExperimentExecutor in canonical-api.md [integration] ``

docs/canonical-api.md:114 still tells products to 'use createSandboxCandidateExperimentExecutor(...) for fresh Tangle sandboxes', but that symbol was deleted in this PR (src/intelligence/sandbox-approved-candidate.ts removed) and replaced by createExactProcessCandidateExperimentExecutor. The auto-generated docs (docs/api/intelligence.md:4410, docs/api/primitive-catalog.md:323) were regenerated correctly; only the hand-curated canonical-api.md is stale. One-line fix to swap the symbol name and up


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.

value-audit · 20260720T235312Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 711833ba

Review health 100/100 · Reviewer score 52/100 · Confidence 95/100 · 12 findings (1 medium, 11 low)

glm: Correctness 52 · Security 52 · Testing 52 · Architecture 52

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 8/8 planned shots over 19 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM capture's fallback re-reads process.stdout() after execute already drained it — src/candidate-execution/exact-process-executor.ts

In execute(), outputPromise = collectOutput(process, ...) starts consuming stdout at line 196 and the consumer only stops when the iterator closes. If execute throws after spawn (e.g., signal aborted at awaitOutput line 212, or oversized-output rejection at line 583), state.output is never assigned. The runtime always calls capture() afterwar

🟡 LOW executor writes profileActivation.files but ignores request.inputs.profile.files — src/candidate-execution/exact-process-executor.ts

materializeRequest iterates request.profileActivation.files (string-content form) at line 467 but never reads request.inputs.profile.files (Uint8Array form, declared at types.ts:461-464). Both should be byte-equivalent (content is UTF-8 of bytes), but the executor never asserts they agree. If a future caller constructs a request where they diverge (e.g., profileActivation.files has stale content), the executor silently writes one representation. Either drop inputs.profile.files from the request type (it's dead input) or add an equality assertion in assertSupportedRequest so divergence fails loudly.

🟡 LOW path-traversal defenses in beneath() and assertKnowledgeExecutionBinding() have no unit tests — src/candidate-execution/exact-process-executor.ts

beneath() (line 719) and the reserved-path overlap check (lines 536-547) are the primary guards against a signed-but-malicious input path escaping its execution root or colliding with knowledge files. They handle the obvious cases (absolute paths, '../' prefixes, prefix-based directory collisions), but the test suite never exercises them: grep for 'input path must', 'escapes its execution', 'knowledge paths overlap' returns zero hits under tests/. Edge cases worth covering: empty string

🟡 LOW recovery path trusts provider-controlled outputMaxBytes over signed plan — src/candidate-execution/exact-process-executor.ts

When capture() runs without in-memory state (fresh-worker recovery), the byte cap comes from environment.metadata?.outputMaxBytes (line 662) — a value the provider persisted. The runtime originally signed outcome.maxBytes into the createMaterial (line 132) and metadata (line 143), but on recovery the executor never re-derives or re-verifies t

🟡 LOW redundant removeEventListener in execute's try block shadows the finally cleanup — src/candidate-execution/exact-process-executor.ts

Line 208 calls context.signal.removeEventListener('abort', cancel) inside the try block, and line 217 does the same in finally. The first call is dead code — finally always runs. Cosmetic, but the intent appears to be 'stop observing abort once process.wait() resolves so cancellation doesn't race with processExited', which the finally already covers. Remove [line 208](https://github.com/tangle-network/agent-runtime/blob/711833ba0d64a07efe02ceee630deade0c9f5b18/src/candidate-execution/exact-p

🟡 LOW stop() re-lists processes after kill but no longer enforces the count bound — src/candidate-execution/exact-process-executor.ts

The first list() at line 229 throws when statuses.length > 1. After stopProcess() at line 237, the second list() at line 240 only checks statuses.some(entry => entry.running) (line 241) — it

🟡 LOW Redundant keys in executeAgentCandidateExperimentCell call — src/intelligence/exact-process-candidate.ts

The call spreads ...input and then re-assigns executionId, executionRoots, stagingRoots, attempt, and conditionally preparation — every one of those keys is already part of input (ExactProcessCandidateExperimentExecution extends CandidateExperimentExecutionInput plus placement fields). The duplicates overwrite with identical values so behavior is correct, but the explicit re-assignment of executionId, executionRoots, stagingRoots, and the conditional preparation add noise without effect (only attempt: input.attempt ?? 1 does work by normalising undefined). Drop the redundant lines and keep the attempt default + the closure-sourced ports/execution. Stylistic, matches the file's explicit-over-spread idiom, not a behavior bug.

🟡 LOW execute() ignores input.attempt when it is explicitly 0 — src/intelligence/exact-process-candidate.ts

attempt: input.attempt ?? 1 only defaults undefined/null; if a caller passes attempt: 0 it flows through. improvement-cycle.ts:232-234 then computes attempt > options.task.attempt.maxAttempts (0 > max is false) and uses attempt=0 for the run-cell digest, while the canonical schema elsewhere treats attempt as 1-indexed (see candidateCell helper at tests/exact-process-candidate.test.ts:644 passing seed=101 but never attempt=0). Unverified whether the upstream type rejects 0 — ExactProcessCandidateExperimentExecution.attempt is number | undefined. Defensive fix: attempt: input.attempt ?? 1 → validate Number.isInteger(input.attempt) && input.attempt >= 1 before forwarding, or rely on the schema. Low severity because no caller in-repo passes 0.

🟡 LOW PATH literal duplicated across fixture helper and two tests — tests/candidate-execution-execute.test.ts

The fixture helper (tests/helpers/candidate-execution-fixture.ts:141) already declares the same PATH default, but each test passes a full env object that replaces it (spread order in candidateBundle). Tests then re-state the literal /usr/local/bin:/usr/bin:/bin. Cosmetic only; not a defect. If desired, expose a PUBLIC_ENV_DEFAULT constant from the helper and spread it in, so the contract lives in one place.

🟡 LOW Real timers used for time-sensitive kill/deadline behavior — tests/exact-process-candidate.test.ts

Tests at lines 467 (setTimeout 5ms in kill mock), 552 (timeoutMs:100 deadline), and 520 (kill-on-oversize) use real wall-clock timers instead of vi.useFakeTimers(). Under CI load these can race: e.g., test 10's 100ms deadline must fire AFTER the abort listener calls cancel() → stopProcess, which itself awaits an async kill mock. If the runtime stalls >100ms before the deadline fires, behavior is correct, but if the kill mock's microtask drains slowly the assertTerminalStatus could see inconsistent state. Recommend fake timers for determinism, but current code is acceptable.

🟡 LOW Test 1 couples to hashKnowledgeBase's internal algorithm — tests/exact-process-candidate.test.ts

The test materializes the runbook at profileRoot/knowledge/runbook.md, calls hashKnowledgeBase(hashRoot), then rmSyncs the directory to compute candidateKnowledgeHash. This is correct for the contract but tightly couples the test to hashKnowledgeBase's directory-walking algorithm — any future change to how @tangle-network/agent-knowledge hashes a base will break this test even if the executor itself is unchanged. Acceptable as a contract test, but worth a comment explaining the indirection.

🟡 LOW Unused resultTimeoutMs option in recovery-capture test — tests/exact-process-candidate.test.ts

The test sets resultTimeoutMs: 100 in createExactProcessCandidateExperimentExecutor options, but only invokes adapter.executor.capture and adapter.executor.dispose?. — never adapter.execute(). resultTimeoutMs is threaded exclusively through the execute() path (src/intelligence/exact-process-candidate.ts:110-112 → executeAgentCandidateExperimentCell), so it has no effect here. Cosmetic; remove or move into a test that exercises execute().


tangletools · 2026-07-21T00:42:59Z · trace

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 12 non-blocking findings — 711833ba

Full multi-shot audit completed 8/8 planned shots over 19 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-07-21T00:42:59Z · immutable trace

@drewstone
drewstone merged commit ccc4570 into main Jul 21, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants