feat(dsl): provider-independent DSL with type-safe capabilities

[arcaputo3]

Summary

Introduces a provider-independent DSL layer for Scalagent that decouples agent definitions from any specific LLM provider (Claude, Codex, etc.).

• Core type system: TypedAgent[In, Out], Capability, Budget, Depth (Peano-encoded delegation limits), ExecutionPolicy, Utility scoring, TraceLogger/TraceSummary, Classification-gated review
• Interpreters: ClaudeInterpreter, CodexInterpreter, A2AInterpreter — same agent definition runs on any backend
• Protocol support: First-class A2A and MCP capability types with McpSurface tool loading
• Safety layers: Capture-checked capabilities (experimental), ScopedCapabilities with runtime enforcement, AgenticReview with explainable scoring and classified segregation
• Builder API: Fluent AgentBuilder for constructing agents without boilerplate
• Examples: 8 runnable examples (dsl-basic, dsl-builder, dsl-delegation, dsl-review, dsl-cells, dsl-codex, dsl-cross, capture)
• Tests: 19 new test suites covering core types, interpreters, event mappers, and experimental features
• Docs: docs/dsl/ with foundations, protocols, mapping guide, examples, roadmap, and next steps

Test plan

• ./mill examples.compile — all examples compile
• ./mill agent.test — full test suite passes
• Manual: ./mill examples.run dsl-basic end-to-end with API key
• Manual: ./mill examples.run dsl-codex with Codex API key
• Review docs in docs/dsl/ for accuracy

🤖 Generated with Claude Code

[claude]

PR Review: feat(dsl): provider-independent DSL with type-safe capabilities

This is a substantial and well-architected PR. The core type system design (Peano-encoded Depth, capability phantom types, Agent[P, I, O] variance) is excellent — these are the right abstractions for a provider-independent DSL. Below is a structured review organized by severity.

---

🔴 Critical

1. Race condition in ClaudeInterpreter.scala — shared state without synchronization

The getShared method uses a mutable var state that is read and written without any synchronization:

var state: SharedState[O] = SharedState.Empty()

def getShared: UIO[SharedRun[O]] =
  ZIO.suspendSucceed {
    state match
      case SharedState.Empty() =>
        for
          promise <- Promise.make[Nothing, SharedRun[O]]
          _ <- ZIO.succeed { state = SharedState.Initializing(promise) }  // not atomic
          ...

Two concurrent fibers can both observe SharedState.Empty() and both try to initialize, creating two independent underlying executions — breaking the single-execution contract. Fix: replace var state with Ref[SharedState[O]] and use Ref.modify for atomic state transitions.

---

🟠 High

2. Budget subtraction semantics are incorrect for the Unlimited - Unlimited case

def -(other: Budget): Budget = (this, other) match
  case (Unlimited, _)  => Unlimited
  case (_, Unlimited)  => Usd(0.0)  // <-- Unlimited - Unlimited hits this, returning $0
  case (Usd(a), Usd(b)) => Usd(math.max(0, a - b))

Unlimited - Unlimited resolves to the second case and returns Usd(0.0). This is also a precision concern: using Double for financial values can cause epsilon drift over many slice() operations. Consider BigDecimal or at minimum document the precision trade-off. Also clarify whether Usd(0.0).isExhausted should return true or false (currently true via <= 0).

---

🟡 Medium

3. AgentBuilder.withReadOnlyTools — validation happens after combining

val combined = tools ++ surface
require(combined.isReadOnlyCompatible, ...)

The ToolSurface is combined before the read-only check, meaning tools is already mutated in the new builder instance if a downstream check fails. Additionally, withReadOnlyTools silently injects ToolSurface.readOnlyBuiltins — callers may not expect implicit tools to be added. Consider either making this explicit or documenting it clearly in the method's scaladoc.

4. ToolSurface.++ doesn't deduplicate

def ++(other: ToolSurface): ToolSurface =
  ToolSurface(tools = tools ++ other.tools, allowedTools = allowedTools ++ other.allowedTools)

Multiple with* calls on AgentBuilder can produce duplicate tool entries. distinctAllowedTools only deduplicates at query time — not at construction. isReadOnlyCompatible and size can return inconsistent results. Consider deduplicating in ++ by tool name.

5. isReadOnlyCompatible validates the allowlist, not the tools themselves

def isReadOnlyCompatible: Boolean = allowedTools.forall(ToolName.isReadOnly)

A ToolDef can have side-effects while being registered under a read-only ToolName. The current check only validates the allowlist, not the actual tool behavior. This is a documentation gap at minimum.

6. TypedAgent.delegateTyped — runtime check direction is confusing

require(child.maxRuntimeDepth < maxRuntimeDepth, ...)

The assertion message says "child depth must be < parent depth" but this compares the integer values of depth levels, not delegation capacity. With Peano encoding, Depth2 has maxRuntimeDepth = 2 and Depth1 has maxRuntimeDepth = 1, so the check is functionally correct — but the mental model in the message conflicts with the Peano semantics described in Depth.scala. Add a comment explaining the Int vs. Peano mapping.

7. CodexClient.jsToJson silently swallows errors

private def jsToJson(value: js.Any): Json =
  try { ... zio.json.ast.Json.decoder.decodeJson(str).getOrElse(Json.Str(str)) }
  catch case _: Throwable => Json.Null

Failed MCP tool argument parsing is silently dropped as Json.Null. This makes debugging Codex tool calls very difficult. Consider returning a structured error value or at least logging the failure.

8. AgenticReview.enrich — permit consumed even if review fails

The permit.consume() call is not monadic — if it throws, the effect chain still proceeds to Evaluation.withReview(...). Should be permit.consume() *> Evaluation.withReview(evaluation, reviewer) to enforce sequencing.

---

🔵 Low / Suggestions

9. Missing experimental module documentation

The experimental/ package has no package.scala or README explaining:

• That capture checking requires -language:experimental.captureChecking and is Scala 3 experimental
• That ScopedCapabilities (zio-blocks/scope) is the stable alternative
• Migration guidance if the feature stabilizes or is removed

This matters because CaptureCheckedOps.scala is in examples/ where users will encounter it first.

10. Utility.weightedNamed vs Utility.weighted — redundant APIs

Both methods solve the same problem (weighted combination). The weighted variant auto-generates names while weightedNamed requires explicit names. This is confusing. Consider deprecating weighted or documenting why both exist.

11. ExecutionPolicy.childPolicy appears unused

def childPolicy(fraction: Double): ExecutionPolicy = ...

This method doesn't appear to be called anywhere in the codebase. If it's intended for delegation, document it; if not, remove it to avoid API bloat.

12. CodexClient — Double.toInt on exit codes

item.exit_code.toOption.map(_.toInt)

JS exit codes are Double. For values outside Int range this silently truncates. Add bounds checking or document the assumption.

13. Test coverage gaps

The DSL test suite is solid, but several critical paths are untested:

• CodexClient.startThread() / resumeThread() / runStreamed() (zero coverage)
• CodexInterpreter integration with ExecutionPolicy and AgentBuilder
• Error event routing through CodexEventMapper to AgentRun.result
• AgentBuilder.build() with conflicting options (e.g., structured output + base options already having outputFormat)

---

✅ What's Well Done

• Capability.scala and Depth.scala: The phantom type system is clean and correct. Inductive given instances for HasSpawn and DepthLTE are textbook-quality.
• Agent.scala variance (-P, -I, +O) is correctly specified.
• Security: No hardcoded keys anywhere; environment variable pattern is consistently used across all examples.
• CaptureCheckSpec: Both positive and negative compile-time tests via typeChecks — this is the right way to test type-level enforcement.
• build.mill ExampleRunner refactor: The EXAMPLE env-var dispatch via ExampleRunner is a significant UX improvement over the previous mainClass as Task.Input pattern.
• A2A and MCP interop types: A2ARemoteAgent and McpSurface are well-factored protocol adapters with clear separation from the core DSL.

---

The foundation is strong. Addressing the race condition in ClaudeInterpreter and the Budget subtraction edge cases would be the highest-priority fixes before merge.

[claude]

Code Review: Provider-Independent DSL Layer

This is a substantial, well-designed PR. The core ideas — phantom-type capability tracking, Peano-encoded delegation depth, provider-independent Agent[P, I, O] with interpreter pattern — are architecturally sound. The 19 test suites show solid coverage of the type-level properties. Below are issues ranging from security bugs to design nits.

---

Security / Critical

Capabilities.scala — non-atomic mutations in BudgetSlice and ReviewPermit

// BudgetSlice.spend()
_remaining -= amount          // not atomic

// ReviewPermit.consume()
_remaining -= 1               // not atomic

Both classes are SharedCapability (implying shared ownership), yet state mutations use plain var fields without synchronization. Concurrent calls to spend() can overdraw the budget; concurrent consume() calls can exceed the review allowance. Fix: use AtomicReference or java.util.concurrent.atomic.AtomicDouble if targeting JVM, or a ZIO Ref if this ever runs in a ZIO fiber context.

Capabilities.scala — symlink escape edge case

catch
  case e: SecurityException => throw e
  case _: Throwable => resolved // root doesn't exist yet; lexical check passed

If the root directory is unreadable (permission denied on realpathSync), the whole symlink check is silently skipped and the code falls back to lexical resolution. A symlink planted alongside the root could escape the sandbox in this case. Consider making realpathSync failure on the root a hard error rather than a fallback.

Capabilities.scala — Node.js platform assumption

private val nodePath = scala.scalajs.js.Dynamic.global.require("node:path")
private val nodeFs   = scala.scalajs.js.Dynamic.global.require("node:fs")

This is Scala.js code that requires a Node.js runtime. The code will throw at construction time (not compile time) in any non-Node environment (browser, JVM, Deno). This should be documented prominently in the class Scaladoc, or the constructor should call require lazily and fail with a descriptive UnsupportedOperationException before any capability is used.

---

Bugs

TypedAgent.scala — depth comparison is off-by-one

// Line 75-76
require(
  child.maxRuntimeDepth < maxRuntimeDepth,
  s"Child depth ${child.maxRuntimeDepth} must be < parent depth $maxRuntimeDepth"
)

The type-level DepthLTE[CD, PD] evidence at call-site is defined as "child depth <= parent depth - 1", i.e., a strict predecessor relationship. But the runtime guard uses strict < on raw integers, so parent=2, child=2 would pass the type check (if DepthLTE is <=) but fail the runtime check, or vice-versa. Verify that DepthValue[D].value for Depth2 and the meaning of "predecessor" match the runtime comparison. A comment explaining the intended invariant would prevent this confusion.

ClaudeInterpreter.scala — daemon fiber leak

_ <- runner.forkDaemon   // no cleanup semantics

The runner fiber is forked but never interrupted if the result is abandoned (e.g., consumer calls .result and ignores the stream, or the ZIO runtime is shut down). This leaks a running fiber for each abandoned AgentRun. Use ZIO.scoped + Fiber.toManagedWith or wrap the fork in a ZIO.acquireRelease so the fiber is interrupted when the scope closes.

CodexEventMapper.scala — mutable shared state without isolation

class MapperState:
  var lastAgentMessage: Option[String] = None
  var startTimeMs: Long = System.currentTimeMillis()

MapperState is a mutable class. If the same MapperState is inadvertently shared across two concurrent streams, the fields corrupt each other. Consider making it immutable and threading state through ZStream.scan or ZStream.mapAccum.

---

Design Issues

Agent.scala — AgentRun[Any, O] loses ZIO environment type

def run(principal: P, input: I, policy: ExecutionPolicy): AgentRun[Any, O]

The Any environment erases any ZIO service requirements the underlying interpreter may need (e.g., ClaudeAgent, Scope). This forces all interpreters to either bake in their own ZLayer or use Unsafe escapes. Consider threading an R type parameter through Agent[R, P, I, O] and AgentRun[R, O] so callers can see and satisfy service requirements.

ExecutionPolicy.scala — no input validation

Retry(maxAttempts = 0) and maxTurns = Some(-1) are silently accepted. Add require guards:

case Retry(maxAttempts) => require(maxAttempts > 0, "maxAttempts must be positive")
// and in ExecutionPolicy constructor:
maxTurns.foreach(t => require(t > 0, "maxTurns must be positive"))

DelegationPolicy.scala — maxChildTurns not validated

maxChildTurns: Option[Int] = None

Some(-5) is accepted. Add maxChildTurns.foreach(t => require(t > 0, "maxChildTurns must be positive")).

AgentEvent.scala — DelegationStarted/DelegationFinished asymmetry

case DelegationStarted(childId: String, label: String)
case DelegationFinished(childId: String, status: String, summary: Option[RunSummary])

label is present on Started but missing on Finished, making it hard to correlate events in traces without joining on childId. status: String should be a sealed type (e.g., enum DelegationStatus { Success, Failed(error), Cancelled }) to prevent typos and enable exhaustive pattern matching.

Utility.scala — successGated breakdown double-counts the gate

The successGated wrapper appends a success_gate component to the breakdown, but the effect of the gate is already visible in the zero-ed out components. Downstream analysis code summing component contributions will over-attribute the impact of gating.

---

Minor / Quality

• Budget.scala: Budget.usd(0.0) is allowed and silently produces a budget that immediately exhausts. Either document this as intentional (a no-op budget for dry-run scenarios) or add require(amount > 0).
• ToolSurface.scala: isReadOnlyCompatible checks allowlist names, not handler behavior. A handler registered under ToolName.Read could still write. This limitation should be noted in the Scaladoc so users understand the check is name-based only.
• TraceLogger.scala: The console logger uses print() directly; concurrent fiber output will interleave. Consider funneling through Console.print (ZIO's thread-safe version) or buffer per-run.
• Capability.scala: CanReadMemory, CanWriteMemory, and CanEscalateHuman have no corresponding type-class extractors (HasReadMemory etc.), so they can be declared in phantom types but never queried at runtime. They're currently dead type-level information.
• build.mill: The --no-server flag was removed from README examples. Make sure this is intentional and not a regression for environments where the Mill server causes issues (CI, Docker).

---

Test Coverage

The test suite is thorough for compile-time type checking (negative tests via typeChecks, builder accumulation, depth constraints). A few gaps worth considering:

• No test verifies that BudgetSlice.spend() is safe under concurrent access.
• ClaudeInterpreter tests (if any) appear to mock out the Claude SDK entirely — a runtime integration test with a mock that exercises the fiber/queue lifecycle would catch the resource-leak issue.
• CodexEventMapper tests should include a case where MapperState is shared (even accidentally) to document/enforce isolation.

---

Overall this is high-quality exploratory work. The type-level safety story (Peano depth, phantom capabilities, DepthLTE) is genuinely novel and the builder pattern is clean. The main blockers before production use are the non-atomic mutations in Capabilities, the fiber leak in ClaudeInterpreter, and the Any environment erasure in AgentRun. The rest can be addressed incrementally.

[claude]

Code Review: Provider-independent DSL with type-safe capabilities

This is a substantial, well-designed PR. The core ideas are sound and the implementation is generally clean. Below is a mix of blocking issues, design questions, and minor observations.

---

Architecture — overall impression

The phantom-type capability system (CanUseTools[T], CanSpawn[D], HasBudget, etc.), Peano-encoded delegation depth, and the Agent[-P, -I, +O] trait variance annotations are all appropriate choices. The shared-run pattern in ClaudeInterpreter (one provider invocation shared between .events and .result) is correctly implemented using an atomic Ref.modify + Promise, avoiding a double-execution race. Good work on that.

---

Issues worth fixing before merge

1. Budget.usd should guard against NaN/Infinity (Budget.scala)

Budget.usd(Double.PositiveInfinity) and Budget.usd(Double.NaN) both pass the current >= 0 check and silently create meaningless budget values that break arithmetic downstream. Add:

def usd(amount: Double): Budget =
  require(amount >= 0 && amount.isFinite, s"Budget must be a finite non-negative number: $amount")

Also the (_, Unlimited) arm in - deserves a comment ("spending an unlimited amount from a finite budget exhausts it") or an error, since this case is unlikely and confusing.

2. Utility.weightedNamed doesn't normalize weights (Utility.scala)

If caller passes weights summing to 2.0, the composite score can exceed 1.0. Either normalize internally, add a require that weights sum to approximately 1.0, or document the invariant. The factory docstring currently says nothing about this.

3. Runtime.default inside claudeTransform creates a detached runtime (ClaudeInterpreter.scala)

val runtime = Runtime.default

This runs at agent construction time and creates a new default ZIO runtime detached from any surrounding runtime context. If the caller's ZIO runtime has a custom layer (tracing, executor, etc.), the MCP tool server won't inherit it. Consider threading the runtime from the call site or using ZIO.runtime[Any] inside a ZIO effect.

4. Agent.run hardcodes R=Any (Agent.scala)

def run(principal: P, input: I, policy: ExecutionPolicy = ExecutionPolicy.unbounded): AgentRun[Any, O]

The return type is hardcoded to AgentRun[Any, O], preventing implementations from expressing environment requirements at the trait level. If intentional (all env deps resolved at interpreter construction), document this constraint clearly.

---

Design questions

5. Ref.unsafe.make in ClaudeInterpreter.make

val stateRef: Ref[SharedState[O]] = Unsafe.unsafe { implicit u =>
  Ref.unsafe.make[SharedState[O]](SharedState.Empty())
}

This works (ZIO Ref is a pure value so eager allocation is fine), but it's unusual and may surprise readers. Hoisting into a for-comprehension using Ref.make would be more idiomatic.

6. parseEvent fallthrough in CodexClient.scala discards unknown events

case other =>
  CodexEvent.Error(s"Unknown event type: $other")

If the Codex SDK adds a new event type, you'd see a synthetic Error event instead of structured unknown data. Consider a Native(tag: String, raw: js.Dynamic) case in CodexEvent (mirroring AgentEvent.Native).

7. CanReadMemory / CanWriteMemory defined but not wired

The capability markers exist in Capability.scala but the agentTransform only wires tool surfaces and MCP surfaces. If stubs for future work, a // TODO comment would make the gap explicit.

---

Minor observations

• var opts in overlayPolicy — mutable variable pattern could be replaced with a fold chain for clarity, not wrong.
• dsl-cells CLI key maps to DslClandestineCellExample — name mismatch is a minor discoverability issue.
• --no-server flag removal — original ./mill --no-server in the README was likely there for CI reproducibility. Removed without explanation; worth confirming this is intentional.
• Stale branch reference — docs/dsl/EXAMPLES.md line 1 says "All examples use the actual DSL API on dsl/core-exploration". Should be cleaned up before merge.
• Tests — compile-time DepthLTE tests using typeChecks/typeCheckErrors are excellent. Would be good to add a BudgetSpec covering the Unlimited - Usd(x) arithmetic edge cases.

---

Summary

Severity	Item
Fix	Budget.usd should guard against NaN/Infinity
Fix	Utility.weightedNamed — document or enforce weight normalization
Fix	Runtime.default in claudeTransform creates detached runtime
Clarify	Agent.run R=Any hardcoding — intentional or oversight?
Minor	CanReadMemory/CanWriteMemory stubs need TODO annotations
Minor	Stale branch reference in EXAMPLES.md
Minor	--no-server removal from README needs explanation

Overall the design is solid and the type-safety story (phantom capabilities, Peano depth, DepthLTE compile-time checks) is well executed. The main concerns are Budget edge cases, missing weight validation in Utility, and the runtime-context issue in claudeTransform.