Stage-bound runtime, capability split with capture checking, and the complexity-review overhaul#22
Open
adamw wants to merge 241 commits into
Open
Stage-bound runtime, capability split with capture checking, and the complexity-review overhaul#22adamw wants to merge 241 commits into
adamw wants to merge 241 commits into
Conversation
ADR 0018 records the full design (resumable committing stages bound to a branch + progress log; FlowContext/FlowControl/InStage capabilities; per-backend session existence probes; idempotent GitHub effects; replaces ADR 0013's Plan-persistence resume). plan-stage-runtime-impl.md is the task-level implementation plan (Epics A-G). ADR 0013 marked superseded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, tuples Adds hand-written `JsonData` instances for `String`, `Int`, `Long`, `Boolean`, `Double`, `Unit`, `Option[A]`, `List[A]`, `(A, B)`, and `(A, B, C)` so that stage-bound flow scripts can use these types as return values without requiring a `Mirror` (which `JsonData.derived` needs). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add two JsonData givens as per Epic A task A2:
- `given [B <: BackendTag]: JsonData[SessionId[B]]` in `BackendTag.scala`,
delegating to `JsonData.given_JsonData_String` (avoids infinite-loop the
compiler detects when `summon[JsonData[String]]` sees this given as a
candidate inside the opaque-alias file).
- `given JsonData[PlanLike]` in `object PlanLike` (Plan.scala): a hand-written
`ConfiguredJsonValueCodec` using a `{"type":"<Name>","value":{…}}` envelope.
`JsonCodecMaker.make[PlanLike]` and `ConfiguredJsonValueCodec.derived` both
fail to add a discriminator at encode time for this sealed trait, so the
codec is written manually using the existing Plan/PlanWithBrief sub-codecs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
JsonData.derived[PlanLike] confirmed inconsistent (encoder omits discriminator, decoder requires it) — hand-rolled codec is retained. Removes unused JsonCodecMaker import, makes the decoder key-order tolerant (buffers value bytes when type arrives later, ignores unknown keys). Removes redundant asInstanceOf brief assertion in tests; adds negative test for unknown discriminator. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Defines the two capability types from ADR 0018 §2.2: - FlowControl (marker trait extending FlowContext): authority to start a stage - InStage (opaque type, private[orca] constructor): in-stage mutation token Includes positive subtyping tests and a negative compile test (package orcacaps) verifying InStage.unsafe is inaccessible outside the orca package. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ests InStage lived in flow but tools cannot import from flow (circular dependency). Move it to tools/src/main/scala/orca/InStage.scala. Keep FlowControl in flow (renamed Capabilities.scala → FlowControl.scala). Move InStage tests to tools; simplify CapabilitiesTest to one focused subtyping test reusing TestFlowContext; strengthen negative test to assert error mentions access/visibility restriction. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sLog) Adds the three immutable case classes for the stage-bound flow runtime progress log (Epic C, ADR 0018 §2.4), each deriving JsonData for JSON round-trip support. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds ProgressStore trait and OsProgressStore implementation (C2 of Epic C / ADR 0018 §2.4). Stores ProgressLog as JSON at <workDir>/.orca/progress-<12hexchars>.json; upserts entries by id; returns None for absent or corrupt files without throwing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds BranchNamingStrategy trait + object with security-critical slug (git-ref-safe, never empty, never starts with '-', SHA-256 fallback), issue(handle, prefix) and fromText(text) factory strategies; 24 munit tests covering all slug security properties and both strategies. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Rewrite `stage` to commit + resume against a progress log, and bind each `flow(...)` run to a feature branch + log (ADR 0018 §2.1/§2.4/§2.5). - stage[T: JsonData](name, commitMessage)(body: InStage ?=> T)(using FlowControl): resume-by-id (decode-or-rerun), run with InStage, then append entry + force-add log + one commit. Add `display` (Step only). - FlowControl gains progressStore / featureBranch / nextOccurrence (ConcurrentHashMap + AtomicInteger counter). - GitTool: forceAdd (git add -f) + resetHard (git reset --hard). - ProgressStore: expose `path` and `hashPrompt`. - flow: mandatory leading `llm`, FlowControl body, setup (stash, branch, header commit) + success/failure teardown. - ReviewLoop internal stages -> display (run under the caller's stage). - Adapt FlowTest/FlowCompilesTest/ReviewLoop tests; add StageRuntimeTest (resume replays once; crash-mid-flow keeps stage 1 committed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1. Robust teardown: body-success teardown now runs OUTSIDE the body's catch so a cleanup-commit failure cannot trigger flowTeardownFailure (resetHard). The log-file removal is best-effort (swallows NoSuchFileException); checkout(startBranch) is in a `finally` so a cleanup error never strands the user on the feature branch. 2. Remove dead featureBranch: FlowControl.featureBranch was never read. Removed from FlowControl trait, DefaultFlowContext (ctor + withDefaults), TestFlowControl, and the CapabilitiesTest stub. 3. Simplify forceAdd: GitTool.forceAdd(paths: Seq[os.Path]) → forceAdd (path: os.Path); both call sites passed a single path. Updated OsGitTool, Flow.recordAndCommit, and flowSetup. 4. Best-effort log removal: os.remove in flowTeardownSuccess now swallows NoSuchFileException (file already gone is harmless). 5. Lifecycle tests: runner/FlowLifecycleTest covers success teardown (branch + log file), failure teardown (branch + clean tree + earlier commit), and setup resume (stage body runs only once across two calls). Note: flow() calls System.exit(1) on body failure, so the failure and resume tests build the aborted-run git state manually. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…it failure Final-review polish on the CORE task: the success-teardown's cleanup commit is now wrapped so a genuine commit failure on an already-successful run is swallowed (the progress file is untracked on the branch we return to), matching the documented "cosmetic" intent. Also drop the stale featureBranch mention from the FlowControl scaladoc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…018 §2.8) Collapse PlanLike/Plan/PlanWithBrief into one always-briefed Plan whose brief comes from the planner structured output (no .briefed turn). Remove the hand-rolled JsonData[PlanLike] codec, the .orca/plan-<hash>.md persistence/recovery API (recover, recoverOrCreate, persistComplete, implementTaskLoop, loadOrGenerate, defaultPath, hashUserPrompt), keeping the generation grid + .reviewed. parse/render now round-trip the brief and are cosmetic-only; the stage log is the sole resume mechanism. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Plan.taskPrompt now returns task.description verbatim when brief is empty, avoiding a stray leading separator line - PlanTest pins both cases: non-empty brief prepends brief+separator, empty brief yields description unchanged - JsonDataGivensTest adds a SessionId[BackendTag.ClaudeCode.type] round-trip, restoring the coverage that was lost when PlanJsonDataTest was deleted in the F1 task Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rack strays Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Whole-branch review follow-ups:
- runner/flow.scala: extract a `private[orca] runFlow` lifecycle seam that
propagates a body failure (exit-free); `flow` keeps the CLI `System.exit(1)`.
Adds genuine end-to-end FlowLifecycleTest coverage of the crash→in-place-resume
path (stage one replays once; body skipped). Resume from an arbitrary branch /
returning to header.startingBranch noted as deferred recovery hardening (R30/R32).
- ProgressStore.appendEntry: document the writeHeader precondition on the trait.
- BranchNamingStrategy.slug: drop dead `replaceAll("-+","-")`, hoist slug(prefix)
in `issue`, clamp maxLen to a lower bound.
- JsonData: note the lenient Unit decoder (Option[Unit] caveat).
- Plan.parse: reconcile docstring with its tolerance of a missing ## Brief.
- Flow.recordAndCommit: comment the deliberate runtime InStage mint.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds two LLM affordances needed by example flows and deferred features: - `LlmTool.cheap: LlmTool[B]` with a default `this` and per-backend overrides: ClaudeTool→haiku, CodexTool→mini, GeminiTool→flash, OpencodeTool→anthropicHaiku, PiTool inherits the default. - `FlowContext.llm: LlmTool[?]` accessor that returns the flow's leading model (the one passed to `flow(args, llm, ...)`). Wiring: `runFlow` passes `leadingLlm = llm` to `DefaultFlowContext.withDefaults`; the context stores and exposes it. All FlowContext implementers updated (DefaultFlowContext, TestFlowContext, TestFlowControl). 7 new tests (6 cheap + 1 llm accessor). Zero warnings. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Review follow-up: ClaudeTool/CodexTool/OpencodeTool/GeminiTool.cheap now return their concrete type (matching haiku/mini/flash/anthropicHaiku) so .cheap chains backend-specific methods without a downcast. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…get-or-create - Add SessionRecord(index, id, seed) to ProgressLog (with lenient codec to tolerate absent sessions field in old logs — strictCodecConfig would reject it) - Add ProgressStore.upsertSession (by index, last wins, no git commit) - Add FlowControl.nextSessionOccurrence() (independent AtomicInteger counter) - Implement nextSessionOccurrence in DefaultFlowContext and TestFlowControl - Add llm.session(seed)(using FlowControl): SessionId[B] extension in Session.scala — pure get-or-create keyed by occurrence index; resumes recorded id on replay Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…purity Review follow-ups: collapse the resume path's exists+find(...).get into one find (no fragile .get); reword the 'pure' doc to 'no LLM call, no commit' since the minted id is a fresh UUID. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds `LlmBackend.sessionExists(session): Boolean` (default `false`) plus non-destructive, best-effort overrides for claude (on-disk `.jsonl` file under `~/.claude/projects/<cwd-slug>/`), codex (glob `rollout-*-<id>.jsonl` under `~/.codex/sessions/`), gemini (`gemini --list-sessions` via the existing `CliRunner` seam), and opencode (`GET /session/<id>` via the existing `OpencodeHttp` seam). Pi inherits the default `false`. Each probe wraps its IO in `try/catch NonFatal => false` and is covered by per-module tests that exercise the present-true / absent-false / unavailable-false paths using injected temp dirs, stub CLI runners, or fake HTTP. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…); best-effort tests + fidelity docs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n not live Expose sessionExists on LlmTool (default false; BaseLlmTool delegates to backend.sessionExists so all five concrete tools automatically relay the D1 probe). Add llm.runSeeded(prompt, session)(using FlowControl): when the backend session isn't live, prepends the recorded seed (from the progress log's SessionRecord) and a progress preamble (completed stage names) before calling llm.autonomous.run; if the session is live, runs prompt verbatim. Missing seed or log → no seed, not a throw. Seven targeted tests (TDD). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ten tests
- composePrimedPrompt: `---` separator emitted ONLY when context (preamble or
seed) is non-empty; no-seed + no-preamble path now returns prompt verbatim
- lookupSeed: filter empty-string seeds so `Some("")` is treated as absent
- "no recorded seed" test: asserts equality to bare prompt (not just contains)
- New test: no seed + completed stages -> preamble present, no stray separator
- Ordering assertion in lost-session test: preamble < seed < prompt by index
- sessionExists delegation tests: replaced tautological stub-override tests
with StubBasedTool (extends BaseLlmTool) backed by StubBackend (extends
LlmBackend), exercising real BaseLlmTool -> backend delegation path
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the old planning/session API across all 6 example scripts:
- remove Plan.briefed, Plan.implementTaskLoop, inner Implementation stage
- use Plan.autonomous/interactive.from(...).value (brief always present)
- seed implementer session with plan.brief via claude.session(seed=)
- run each task in its own stage via claude.runSeeded + reviewAndFixLoop
- implement-enhanced: add .reviewed(claude), plan.taskPrompt(task), push+PR stages
- epic: claude.opus for planning, allReviewers(codex) for cross-backend review
- issue-pr: BranchNamingStrategy.issue, assessThenPlan, Verdict match, Option[Plan] stage result
- issue-pr-bugfix: Triage stage result, R8 push-after-edit in separate stages,
waitForBuild outside stage, PrHandle stage result
API fixes to support stage result types:
- Triage enum: add derives JsonData (needed for stage("Triage") return type)
- PrHandle case class: add derives JsonData (needed for stage("Push + open PR") return type)
FlowCompilesTest: add 6 compile-check defs mirroring each example's distinct
shape (ADR 0018 §3 canaries). All compile clean with zero warnings.
Known gap: `claude` accessor requires FlowContext, so `flow(args, claude)` does
not compile in standalone .sc files. Examples show the intended API; the shapes
are verified via FlowCompilesTest with a `leadModel` stub.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ainst new API The `flow(args, llm: LlmTool[?])` leading-model parameter was uncallable: the only way to name a model is the `claude`/`codex`/… accessor, which needs an in-scope FlowContext not present at the `flow(...)` argument position. Change it to a selector `leadModel: FlowContext => LlmTool[?] = _.claude` resolved against the built context, and reorder the lifecycle so the ProgressStore and DefaultFlowContext are built BEFORE branch setup (which needs `ctx.llm`). `DefaultFlowContext.llm` becomes a lazy val resolving the selector. Teardown / resume (bodySucceeded gate, success checkout-back, failure resetHard+stay) are unchanged. Rewrite the 6 examples + FlowCompilesTest to the real shapes (`flow(OrcaArgs(args))`, `_.codex` selector, issue branchNaming), update the other flow tests to the selector arg, and update ADR 0018 §2.5/§3. Examples compile clean via scala-cli against the local publishLocal snapshot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Model selector warning
- issue-pr.sc: move Rejection gh.writeComment into its own stage("Comment:
rejection") so it is checkpointed and not double-posted on resume; assess
stage returns (Option[Plan], String) tuple (both have JsonData) to carry
the rejection body across stage boundaries.
- FlowCompilesTest: mirror the new two-stage pattern in issuePrFlowShape canary.
- issue-pr-bugfix.sc / implement-enhanced.sc: add code comment near
gh.createPr(...).orThrow noting crash-safe idempotency depends on ADR §2.7
R24 (external-effect-idempotency task).
- Triage.scala: update docstring — Sessioned wrapper is available but flows
start a fresh seeded implementer session (.value discards the triage session);
add comment on Testable.branchName that the runtime (BranchNamingStrategy),
not this field, names the actual feature branch (field kept for LLM wire).
- flow.scala / DefaultFlowContext.scala: add WARNING scaladoc that leadModel
selector must not read ctx.llm (lazy val self-reference = infinite loop);
safe selectors are _.claude, _.codex, _.gemini, _.opencode, _.pi.
- All example version pins bumped to 0.0.14+28-eb1a8993+20260624-0842-SNAPSHOT.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e probes (R22) Persist the learned client→server session mapping in the progress log and rehydrate it on resume, so codex/gemini/opencode (server-id backends) continue the right server thread and probe the server id across a resume — closing D1's documented gap where gemini/opencode probed the client id (always false in prod). - SessionRecord gains serverId: Option[String] = None (back-compat: decodes to None when absent; the lenient ProgressLog codec already tolerates it). - SessionRegistry.serverFor read accessor (ClientToServer → mapped server id; ClaimedOnce → Some(client) iff claimed). - LlmBackend.serverFor (default None; overridden in codex/gemini/opencode); LlmTool.serverSessionId / registerServerSession surfaced via BaseLlmTool. - runSeeded persists the learned server id after a run (upsert only when changed). - flow lifecycle rehydrates the map into the leading model after setup, before the body (multi-tool flows are a documented limitation). - gemini/opencode sessionExists now resolve serverFor(client) and probe the SERVER id; None mapping → false. claude/pi unaffected (client == wire id). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Retype ReviewerSelector to hand selectors the roster as opaque `RosterEntry[?]` handles (`private[review]` ctor) and return a subset/permutation of them as a pure `->` arrow. A foreign reviewer is now unrepresentable at the type level, so four runtime defenses collapse: the name-uniqueness `require`, `resolveAgainstRoster` + foreign-drop warning, and the silent full-roster fallback all delete. Empty selection now honestly means "no reviewers this round" — the shared stop policy converges rather than resurrecting the full roster behind the selector's back. Accidental duplicate entries collapse via identity `.distinct`. The per-reviewer session map becomes an existential `SessionEntry[B]` pairing each `RosterEntry[B]` with its `SessionId[B]` under one backend tag, keyed by entry identity (`eq`). Recovering a session runs the paired entry+session as a unit, so the old `SessionId.Untyped` + `.as[RB]` two-hop cast is gone entirely — `SessionId.Untyped` (sole client) is deleted. CC prerequisite: `ReviewerInfo`/`ReviewerSelectionRequest` (tapir `derives`) move to a sibling non-CC file (the `FixRequest` precedent) so `ReviewerSelector.scala` can carry the `captureChecking` imports and declare the pure arrow. Breaking change to the exported `ReviewerSelector` trait: export `RosterEntry`/`ReviewBatch`, extend the FlowCompilesTest canary with a custom-selector surface. The four foreign-selector tests become inexpressible; rewritten to pin the new contract (roster-bound selection, honest empty selection, identity dedup). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eviewerSelector purity constraint as a CC negative
The 2026-07-06 ADR 0011 amendment described transitional machinery
(resolveAgainstRoster, SessionId.Untyped.as[RB]) that was since deleted.
Add a 2026-07-08 amendment describing the shipped contract: roster-bound
RosterEntry handles, pure-arrow prepare, existential SessionEntry pairing,
and empty-selection-as-honest-stop.
Note the eq/.distinct identity coupling on RosterEntry's class doc (a
case class would split-brain the two lookups), and add a one-line
constraint to ReviewerSelector's trait doc: per-round effects must be
hoisted into prepare, since the returned arrow is pure.
Add CcNegativeCompileTest cases (d)/(e): a ReviewerSelector whose
returned arrow captures an `ev: InStage` capability fails to compile
(capture checking rejects it with "cannot flow into capture set {}",
not "Separation failure" — a different checker pass than the existing
exclusive-capability cases), and its pure history-narrowing twin
compiles clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Seven independent hardening fixes: AgentTurnFailed now carries its cause at both wrap sites; ForkedConversation asserts succeedWith/failWith run on the reader thread (write-side only, cancel()'s cross-thread read untouched); resetHard's doc (+ ADR 0018 §2.5) now states untracked files survive it; ProgressStore's write path routes through loadDetailed() so a corrupted log surfaces its own message instead of a false "before writeHeader"; Pricing's prefix fallback is gated on a date-like suffix so it can't cross model tiers (e.g. flash vs flash-lite); ConversationRenderer's dead showThinking flag is deleted, which also structurally removes the shared-buffer mis-styling risk; and OrcaEvent.TokensUsed gains a role axis (Agent.role/withRole, threaded through every backend) so the review loop tags spend via role instead of baking a "reviewer: " prefix into the agent's name — CostTracker derives the same display text and a perRole/perRoleCost subtotal from the tag. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pic 12) os.temp.dir()'s deleteOnExit is limited to File.deleteOnExit, which no-ops on non-empty dirs, so every GitRepo/workDir fixture leaked permanently -- ~35k dirs filled an 8G tmpfs in a day (Epic 12 blockquote). Route the shared GitRepo chokepoint and every plain os.temp.dir() workDir call site (78 sites, 22 files) through a new orca.testkit.TempDirs registry that recursively os.remove.alls every registered root via one JVM shutdown hook, matching build.sbt's Test/fork := true per-module-JVM boundary. OrcaLog's persistent /tmp/orca-*.log files are untouched by design. Verified: sbt tools/test and full sbt test both show zero new leaked /tmp dirs (numeric-dir count delta = 0), all 775 tests green, zero warnings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…; review finding) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, honest test pair) Drop CostTracker.perRoleCost (zero call sites; summary reads the State field directly). Narrow TempDirs.register to private since only dir calls it. Give the upsertSession half of ProgressStoreTest's two before-writeHeader/corrupt-log pairs a caller-name assertion so it earns its keep instead of duplicating the appendEntry test verbatim. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Must-fix: pin the scala-cli smoke test's generated script to 3.8.4 (literal kept in sync with V.scala, not reachable from test code) and register implement-enhanced.sc/issue-pr.sc in its compile-check list; fix the non-compiling Pi README example (missing _.pi selector); repoint the six examples/*.sc dep coordinates at the unresolvable dynver SNAPSHOT to 0.0.14, and widen UpdateScalaCliVersionInDocs's version regex to include '+' so future release bumps don't re-corrupt a SNAPSHOT coordinate. Touch-ups: annotate the 6.4 done-note; correct the stale Epic 7 residual note about excludeLockFromGit's workDir assumption (it resolves the git common dir and is worktree-safe); export BuildWaitFailed alongside PushFailure; hold FlowSessionCall's resultAs[O] gateway as a val so a hoisted structured session gets JsonSchemaGen's fail-fast at construction; update ADR 0018 §3's illustrative session(...) calls and stale "claude default" comment to the current API; reword the README's nonexistent SlackInteraction reference as a customization point via orca.backend.Interaction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nt layers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nounce, EOF, stderr dedup - lint(): inline small (<=8KB) command output straight into the summariser prompt instead of pointing a read-only agent at a /tmp file — sandboxed autonomous agents (opencode) deny reads outside their worktree, which silently turned the lint gate into a no-op. Large output still spills to a file, now under <workDir>/.orca/ (readable in-sandbox, commit-safe), removed in finally. - codex: attribute TokensUsed to the configured model when thread.started omits it (e.g. resume), so tokens are priced rather than landing under "(unknown)". - Plan announce: drop the branch name — epicId is the plan id, not the flow's git branch (they can differ); the flow announces its branch at setup. Fix the stale epicId scaladoc too. - JLinePrompter.ask: catch EndOfFileException (Ctrl-D / closed stdin) alongside UserInterruptException, mapping both to the graceful Interrupted outcome. - StderrPipeline: suppress consecutive identical stderr lines (claude's ANTHROPIC_API_KEY warning fired ~8x/run). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ing, fence collisions) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…structor Move the `using Ox` capability off the ForkedConversation family's constructors and onto the conversation surface (`events` / `awaitResult`), per the owner's Ox method-scoping rule. Workers now start lazily via an internal `ensureStarted(using Ox)` on first touch of the surface, forking into the per-turn supervised scope the drain shells (`Conversations.runAutonomous` / `AgentCall.runInteractiveOnce`) already open — the same scope construction happens in, so fork lifetime is unchanged. Threads `using Ox` through the `Conversation` trait surface and `Interaction.drive`; the obsolete "forks bind to the Ox captured at construction" caveat is deleted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ms; extract FlowLock
Simplicity sweep, no behaviour change:
- settleSuccess base helper on ForkedConversation wraps WireSessionId[B] +
Model.apply + succeedWith(AgentResult(...)); the five drivers now pass their
synthesised values instead of each spelling out identical scaffolding.
- cleanExitWithoutResult collapses to a terminalMessageNoun hook on the base
(claude included: it doesn't mix StderrPipeline, so its diagnosticContext is
None and the appendContext it "dropped" was a no-op — its message is
byte-identical).
- CliArgs.flag(name, opt)(render) helper routes the ~6 copies of the
opt.toSeq.flatMap(Seq("--x", v)) idiom (modelArgs, claude/codex/pi arg
builders); stale "shared between ClaudeArgs, CodexArgs" doc fixed.
- ReviewLoop's two private emitStep aliases deleted in favour of orca.display.
- flow.scala's lock cluster moved verbatim into runner/FlowLock.scala with an
acquire/release surface; flow.scala keeps entry + orchestration only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…helpers Structure/visibility tidy-up: - ReviewLoop.scala split: `lint` + InlineLintThreshold → review/Lint.scala (kept capture-checked, verbatim); formatIssue/formatReviewerOutcome → review/ReviewFormatting.scala (plain, pure string helpers). The loops, stopPolicy, roster/state types, and extractChangedFiles stay. - TerminalOutput trait + companion → private[terminal] (zero outside-package references). - throwableMessage + pluralize moved from Flow.scala to orca.util.TextUtil (cross-package general helpers, alongside TextWrap); call sites updated. - runner tests' TempRepo one-line alias deleted; call sites use GitRepo.seeded() directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-up to the fork-scoping refactor: with the *Conversation constructors no longer taking `using Ox`, the private `openConversation` builders (pi, opencode) and opencode's `startTurn` no longer use the capability. Incremental compilation hid the resulting unused-implicit warnings; a clean build surfaced them. The public `runInteractive` overrides keep `using Ox` to match `AgentBackend`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`reviewAndFixLoop`'s `reviewerSelection` becomes optional (`Option[ReviewerSelector] = None`): when absent it derives `agentDriven(coderSession.agent.cheap)` — the common case, so a call omits it. A parameter default can't reference `coderSession` (Scala forbids referencing an earlier param), so absence is `None`, resolved in the body. `FlowSession.agent` is exposed `private[orca]` for the derivation. Four backends have a distinct cheap tier; pi has none, so `pi.cheap == pi` and the picker runs on the lead tool (documented). `Reviewer`, `ReviewerPrompts`, and `buildReviewers` become public as the reviewer-customisation surface: compose your own `List[Reviewer]` and build the agents `reviewAndFixLoop` takes, to swap/extend `allReviewers`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
F2: `plan.implementerSession(agent, name = "implementer")` threads `plan.brief` as the seed automatically, collapsing the recurring `agent.session(name, seed = plan.brief)` pairing to one call. The general `agent.session(name, seed)` mechanism is untouched (for non-plan seeds). F4: `openPrFromBranch` (orca.pr, beside summarisePr) bundles the push -> summarise -> create tail as the three resume-safe stages every PR flow otherwise hand-rolls, parameterised by title/body builders (for closes-lines), the summarising agent, and context. Preserves the exact stage names and the diffVsBase(defaultBase) idiom. Both are exported; the FlowCompilesTest canary mirrors the new example shapes and pins the reviewer-customisation surface. Also: a one-line awareness comment on ForkedConversation.ensureStarted's workers-assignment window (benign, practically unreachable — documented, not guarded). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The six example scripts drop the now-redundant `reviewerSelection` arg (all default to agentDriven on the coder's cheap tier); five adopt `plan.implementerSession` and the two clean-fit PR flows adopt `openPrFromBranch`. issue-pr-bugfix keeps its `fixer` session (seeded from the issue body, minted before any plan) and its updatePr two-touchpoint tail, which the createPr-shaped helper deliberately does not cover. README: reviewerSelection now documented as optional (derived default); adds the reviewer-customisation note. Ledger: records comprehensive-review F1 as declined (coder/lead selector stays required — owner, 2026-07-08). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…(F5)
reviewAndFixLoop took lintCommand: Option[String] and lintAgent: Option[Agent[?]]
as a pair, so "a command with no summariser" was a representable-but-invalid
half-set state, guarded only by a runtime require. Bundles both into a single
case class Lint(command, agent) — review/Lint.scala, the lint feature's home
post-split — and a lone `lint: Option[Lint] = None` parameter, deleting the
require entirely.
Lint's companion object is public (not private[review]): a private companion
would carry the case class's synthesized apply down with it, making
`Lint(command, agent)` unreachable from outside the package despite the class
itself being public; InlineLintThreshold stays package-private on its own
member. Exported from orca.exports alongside the rest of the reviewer surface.
Threaded through ReviewLoopConfig/ReviewFixLoop and the ~4 example call sites,
tests, and the FlowCompilesTest canary. `import config.*` inside ReviewFixLoop
now excludes `lint` (config.{lint as _, *}) so the field doesn't shadow the
package-level `lint(command, agent)` summariser function the loop calls.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
README: - Front example: bring in line with the migrated implement.sc — plan.implementerSession, no explicit reviewerSelection arg (shrunk the agentDriven-swap comment to one line), and the new lint = Some(Lint(...)) shape from the previous commit. - git tool row: PushRejected -> PushFailure (NonFastForward/RemoteDeclined). - Branch-naming default, in the front example and the flow-method table: it's a cheap-model-generated label (an LLM call), not a slug of the raw prompt; both spots now point at "The flow lifecycle" for the full branch/teardown story instead of repeating it (previously told 3x). - Plan(epicId, ...) row: epicId is the plan's own kebab-case identifier, NOT its git branch (Batch A already fixed the scaladoc; the README row hadn't caught up), and points at plan.implementerSession for seeding. - Swept for stray lintCommand/lintAgent references (examples/runnable 01-simple's README) now that reviewAndFixLoop takes `lint: Option[Lint]`. - Verified the ReviewerPrompts/Reviewer/buildReviewers customisation paragraph and the reviewerSelection-is-optional wording (both already landed pre-Batch-B); no apology "requires a reviewerSelection" sentence survives. ADR 0014 (OpenCode server driver): status Proposed -> Accepted (shipped long ago; ADRs 0017/0018 build on it) and header normalised to the house `# 00NN.` + inline `Status: Accepted · Date:` format used by every other ADR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
Comprehensive review completed (8 owner questions, 7 parallel review agents including live E2E testing) — 11 follow-up commits pushed ( Live end-to-end results (
|
| Backend | Success | Resume (SIGKILL mid-task) | Concurrent-run guard | Cost |
|---|---|---|---|---|
| claude 2.1.204 | ✅ 1m49s | ✅ stages skipped, no dup commits, stale lock + auto-stash handled | ✅ clear refusal | $0.88 / $0.47 |
| codex 0.133.0 | ✅ 2m17s | — | — | ~$0.02 |
| opencode 1.17.10 | ✅ 4m53s | — | — | $0.29 |
| pi 0.80.2 | ✅ 1m01s | — | — | $0.27 |
Review verdicts
API premise intact · structure sound · docs precise (4 wording fixes) · error handling no silent paths · structured concurrency + Ox scoping pass · simplicity: 5 collapses applied.
What the follow-up commits changed
- Bugs from live testing: lint output now inlined (≤8KB) or spilled inside the worktree — fixes opencode's sandbox silently disabling the lint gate; codex model-attribution fallback; plan announce no longer names a wrong branch; Ctrl-D at prompts is a graceful cancel; consecutive-duplicate stderr deduped.
- Ergonomics (owner-decided):
reviewerSelectiondefaults toagentDrivenfrom the coder's cheap tier;ReviewerPrompts/Reviewer/buildReviewerspublicized;plan.implementerSession(agent)replaces the six-fold session boilerplate;openPrFromBranch(...)bundles the push→summarise→createPr tail;lint = Some(Lint(cmd, agent))replaces the two coupled params + runtime require. Lead selector stays explicit (owner decision). - Simplifications: five-driver settle scaffolding collapsed into
settleSuccess;FlowLock.scalaextracted;ReviewLoop.scalasplit (Lint/ReviewFormatting);(using Ox)moved off conversation constructors ontoevents/awaitResult(method-scoped per the structured-concurrency rule). - Docs: README front example matches the new minimal API;
PushFailurerow, branch-naming honesty,epicIdfix, teardown story de-duplicated; ADR 0014 marked Accepted.
Recorded residuals (owner calls, in complexity-review-2.md): codex default-model token pricing, prompt-fence collisions, examples pin 0.0.14 until next release.
🤖 Generated with Claude Code
…ect.sh --client staleness Live kill-mid-run resume testing (2026-07-08) proved a committed opencode resumeWireId is inert across a process restart: orca spawns a fresh `opencode serve --port 0` per run, so a resumed run's probe finds no live server session and gracefully re-seeds instead of continuing. Qualify the "sessions outlive the process" claim in AGENTS.md's Sessions bullet, README's Sessions section and opencode tool-table row, and the OpencodeBackend/OpencodeServer scaladoc where the probe/server lifecycle is documented. SessionSupport.Durable classification and the probe/graceful-fallback design are unchanged — they're correct; only the prose overclaim is fixed. Record the cross-restart-resume gap as an owner call alongside the other comprehensive-review residuals. Also switch _seed_lib.sh's --local path from `sbt --client` to plain `sbt`, per AGENTS.md's own warning that --client can silently attach to a stale persistent server (reproduced live: a Metals-managed stale server nearly pinned a test flow to old bytecode). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
Resume matrix completed for the remaining backends (follow-up to the live-test comment; commit f0d8055).
No structural failures on any backend: no duplicate commits, wedged sessions, or corrupt logs; the flow-level resume promise holds universally (claude previously verified end-to-end including session continuation). Finding: opencode's committed 🤖 Generated with Claude Code |
opencode persists sessions to a global, cwd-independent on-disk store, and a freshly (lazily) spawned `opencode serve` resumes them — live-verified. The old `server.started && probeSession(...)` guard short-circuited the very first resume probe of a run, before anything had forced the lazy server spawn, so a genuinely resumable session was reported absent and the flow re-seeded instead of resuming. Dropping the guard is safe: SessionSupport.Durable.exists already short-circuits on no mapped wire id, so the forced spawn only fires on a real resume, when the server is about to be needed anyway. Removes OpencodeServerHandle/OpencodeServer.started (now dead — the probe was its only production use) and inverts/extends the OpencodeBackendTest coverage accordingly. Corrects the scaladoc and README/AGENTS.md durability claims that previously (wrongly) documented opencode sessions as durable only within a single run, and resolves the opencode residual in complexity-review-2.md's Epic 12 notes. Follow-up: a live kill-mid-run opencode re-verification should confirm the resume probe returns 200, dispatch is Dispatch.Resume, and no seed preamble is re-injected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…premature live-verified claim Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Live-test finding B: opencode's lint reviewer (claude-haiku, routed
through opencode's own structured-output tool whose sole parameter is
named `input`) echoed that tool-call envelope back as the response —
`{"input":"{\"issues\":[]}"}` instead of the bare `{"issues":[]}` — and
ResponseParser rejected it 2/2 retries, aborting the whole flow.
Unwrap exactly the lone-"input"-key shape (string-encoded or nested
object value), one level, before falling back to the existing failure
path; everything else still goes through the unchanged right-to-left
brace scan. Also nudges the corrective retry prompt to ask for the
JSON directly, not wrapped under any key.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ssion-memory note Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
Multi-task crash/resume matrix + two fixes (commits
Two bugs found and fixed:
Recorded behavior note: a SIGKILL leaving mid-task uncommitted work produces a brief stash-vs-session-memory divergence on continued sessions (the loop churns until the agent re-materialises the code; observed once, completed). Warn-don't-pop remains the deliberate stash policy. 🤖 Generated with Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #21 — this branch contains that PR's entire diff plus the two follow-up passes that substantially rewrote it, so it is reviewed here as the coherent end state.
What this contains (three phases, linear history)
session-identity-fixes, 58 commits): the complexity-review.md tracker — session identity split, enforcement matrix, review-loop restructure, event quarantine, and the rest.complexity-review-2, 87 commits): the full complexity-review-2.md tracker, 13 epics:InStage/WorkspaceWrite+FlowControl) with live capture/separation-checking enforcement (CheckedParfunnel, dotc-in-test negative-compile suite, no consumer taint)SurfacedFlowFailurebracket); successful runs structurally cannot exit 1FlowSession— one durable-session door (probe → seed → run → persist);runSeededand the bare-SessionId trap removedFeatureBranch— protected-branch + ref-shape safety on both setup arms; silent branch adoption removedSessionId.Untypeddeleted), diagnostics batch + test temp-leak fixProcess & verification
Every task was implemented by a dedicated agent with an independent spec+quality review; every epic got a two-lens review round; a final whole-branch review audited the tracker against shipped reality (verdict: ready to merge). Full suite green throughout (~920 tests, zero warnings); example scripts compile-verified against a local publish.
Known follow-ups (documented owner calls, not defects)
0.0.14until the next release publishes the new API (release tooling now heals the pin automatically)ORCA_INTEGRATION=1runs against real CLIs not exercised in this effort🤖 Generated with Claude Code