Repository navigation
feat: add execution binding foundation #75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
2ffbcd6
docs(adr): define pg phase 2 ownership model
delete-cloud 86b479b
docs(adr): define execution binding layer
delete-cloud 0b499df
docs(prompt): add execution binding task packet
delete-cloud aea8ecb
feat(ui): add execution binding foundation
delete-cloud d208f36
feat(ui): bind session runtime to execution metadata
delete-cloud 0477524
fix(ui): inject binding resolver
delete-cloud 768724c
test(ui): harden execution binding coverage
delete-cloud File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
56 changes: 56 additions & 0 deletions
56
coding-agent/.opencode/prompts/session-execution-binding-phase2-task-packet.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| Goal: | ||
| Add a first-class execution binding abstraction that separates WHERE a session runs from WHO owns it, then build the thin Phase 2 ownership substrate (session owners, leases, fencing) on top of Phase 1 PostgreSQL persistence. | ||
|
|
||
| Scope: | ||
| - Introduce `ExecutionBinding` dataclass hierarchy in `src/coding_agent/ui/execution_binding.py` with `LocalExecutionBinding` and `CloudWorkspaceBinding` variants, plus typed serialization helpers. | ||
| - Add `BindingResolver` protocol and `DefaultBindingResolver` in `src/coding_agent/ui/binding_resolver.py` that converts bindings into `workspace_root` and tool config for `create_agent_for_session`. | ||
| - Integrate binding into `Session` metadata: store `execution_binding` on the session, round-trip it through `to_store_data` / `from_store_data`, default missing bindings to `LocalExecutionBinding(repo_path)` for backward compatibility, and resolve the binding in `run_agent`, `ensure_session_runtime`, and `_restore_checkpoint`. | ||
| - Add thin Phase 2 ownership substrate: | ||
| - `PGSessionOwnerStore` in `src/agentkit/storage/pg.py` with `session_owners` table schema and atomic acquire/renew/release SQL. | ||
| - App-layer `SessionOwnerStore` in `src/coding_agent/ui/session_owner_store.py` wrapping the PG store. | ||
| - Owner checks in `SessionManager` on `run_agent`, `_restore_checkpoint`, and `close_session` using `_assert_owner`. | ||
|
|
||
| Out of scope: | ||
| - HTTP server local-execution binding integration (endpoint changes and HTTP-level binding tests). | ||
| - Cloud workspace tool implementations or resolver behavior beyond `NotImplementedError`. | ||
| - Sticky routing hardening, brokered event routing, or failover boundary tests. | ||
| - In-flight turn resume after owner loss. | ||
|
|
||
| Context: | ||
| - ADRs: | ||
| - `docs/adr/0014-separate-session-execution-binding-from-runtime-ownership.md` | ||
| - `docs/adr/0013-define-phase2-multi-instance-session-ownership-for-pg-http-sessions.md` | ||
| - `docs/adr/0012-complete-phase1-postgresql-http-session-persistence.md` | ||
| - Relevant files: | ||
| - `src/coding_agent/ui/execution_binding.py` (new) | ||
| - `src/coding_agent/ui/binding_resolver.py` (new) | ||
| - `src/coding_agent/ui/session_owner_store.py` (new) | ||
| - `src/agentkit/storage/pg.py` (append `PGSessionOwnerStore`) | ||
| - `src/coding_agent/ui/session_manager.py` (integrate binding + owner checks) | ||
| - `tests/ui/test_execution_binding.py` (new) | ||
| - `tests/ui/test_session_owner_store.py` (new) | ||
| - `tests/ui/test_session_manager_owner_checks.py` (new) | ||
| - `tests/ui/test_session_manager_public_api.py` (binding round-trip tests) | ||
| - `tests/agentkit/storage/test_pg.py` (PG owner store tests) | ||
| - Implementation plan: | ||
| - `docs/superpowers/plans/2026-04-19-session-execution-binding-and-thin-phase2.md` | ||
|
|
||
| Target tests: | ||
| - `uv run pytest tests/ui/test_execution_binding.py -v` | ||
| - `uv run pytest tests/ui/test_session_owner_store.py -v` | ||
| - `uv run pytest tests/ui/test_session_manager_owner_checks.py -v` | ||
| - `uv run pytest tests/ui/test_session_manager_public_api.py -k "binding" -v` | ||
| - `uv run pytest tests/agentkit/storage/test_pg.py -k "owner" -v` | ||
| - `uv run pytest tests/ui/test_execution_binding.py tests/ui/test_session_owner_store.py tests/ui/test_session_manager_owner_checks.py tests/ui/test_session_manager_public_api.py tests/agentkit/storage/test_pg.py -v` | ||
|
|
||
| Loop policy: | ||
| - Engineer implements the smallest correct change and runs the target tests. | ||
| - Reviewer reviews only the resulting diff and affected tests. | ||
| - Reviewer reports only P1/P2 findings. | ||
| - Engineer fixes only accepted P1/P2 findings and reruns the same target tests. | ||
| - Verifier reruns the exact target tests and reports pass/fail only. | ||
|
|
||
| Stop conditions: | ||
| - At most one review/fix/retest cycle. | ||
| - Escalate architectural redirection or scope expansion to the human. | ||
| - Ignore non-blocking optimization suggestions. |
64 changes: 64 additions & 0 deletions
64
...adr/0013-define-phase2-multi-instance-session-ownership-for-pg-http-sessions.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| # ADR-0013: Define Phase 2 multi-instance session ownership for PG HTTP sessions | ||
|
|
||
| **Status**: Proposed | ||
| **Date**: 2026-04-19 | ||
|
|
||
| ## Context | ||
|
|
||
| ADR-0012 closes the Phase 1 PostgreSQL persistence gap for the HTTP/UI stack by making session metadata, tape history, and checkpoint snapshots durable in PostgreSQL. But it keeps active runtime state (`task`, approval state, event queues, pipeline/context/adapter ownership) single-instance and local-authoritative. | ||
|
|
||
| That Phase 1 boundary is acceptable for restart-safe storage, but it is not sufficient for multi-pod HTTP deployment. Once multiple application instances can read the same persisted session metadata, the repository needs an explicit answer to three questions that Phase 1 intentionally left open: who owns execution for a session right now, how other instances know they are no longer allowed to act, and how approval/event flows reach the active owner. | ||
|
|
||
| Phase 2 is the ownership and coordination layer that sits on top of Phase 1 persistence. It does not primarily add “more PostgreSQL persistence”; it defines how multiple instances safely share a PostgreSQL-backed session/tape/checkpoint substrate without split-brain behavior. | ||
|
|
||
| ## Decision | ||
|
|
||
| Define Phase 2 as a multi-instance coordination layer for HTTP sessions with four core properties: | ||
|
|
||
| - Each active HTTP session has one current runtime **owner** recorded in PostgreSQL. | ||
| - Ownership is time-bounded by a renewable **lease** rather than an infinite claim. | ||
| - Every ownership grant yields a monotonically increasing **fencing token** that must accompany owner-sensitive writes and commands. | ||
| - Runtime execution, approval handling, and event streaming are all **owner-routed** operations rather than implicit local-memory behavior. | ||
|
|
||
| Specifically, Phase 2 will include: | ||
|
|
||
| - A PostgreSQL-backed `session_owner` record keyed by `session_id`, storing `owner_id`, `lease_expires_at`, `fencing_token`, and timestamps. | ||
| - App-layer primitives to `acquire`, `renew`, and `release` session ownership atomically. | ||
| - Owner checks in `run_agent`, checkpoint restore, and close/shutdown flows before they perform runtime-sensitive work. | ||
| - Fencing-token validation so a stale owner cannot continue mutating session-associated state after ownership changes. | ||
| - An explicit routing rule for approval responses and SSE/event delivery so only the active owner drives runtime-visible side effects. | ||
| - A first failover contract of **at-most-once** execution for in-flight turns: after owner loss, a new owner may rebuild cold state from persisted data, but it does not resume partially executed runtime state. | ||
|
|
||
| Phase 2 should initially prefer a minimal operational model: | ||
|
|
||
| - sticky routing is acceptable as the first event/approval routing strategy, | ||
| - in-flight turn replay/resume is out of scope, | ||
| - only owner-sensitive actions need fencing in the first cut. | ||
|
|
||
| ## Alternatives Rejected | ||
|
|
||
| - Keep Phase 2 as “just add more PG-backed state” — rejected because Phase 1 already made durable state explicit; the unresolved risk is multi-instance coordination, not missing tables. | ||
| - Let any instance read persisted session state and optimistically execute if local cache is empty — rejected because it allows concurrent turn execution, stale approval handling, and conflicting close/restore behavior. | ||
| - Solve multi-instance coordination with only sticky load balancing and no persisted owner record — rejected because it provides no correctness guarantee under pod restarts, manual retries, or load balancer drift. | ||
| - Attempt best-effort in-flight turn resume in the first Phase 2 slice — rejected because the repository still treats runtime objects as local-authoritative and not serializable across owners. | ||
|
|
||
| ## Acceptance Criteria | ||
|
|
||
| - [ ] `test_session_owner_acquire_conflicts_with_live_lease` | ||
| - [ ] `test_session_owner_renew_rejects_stale_token` | ||
| - [ ] `test_run_agent_rejects_non_owner_instance` | ||
| - [ ] `test_restore_checkpoint_rejects_stale_owner` | ||
| - [ ] `test_close_session_rejects_stale_owner` | ||
| - [ ] `test_approval_response_is_rejected_after_owner_change` | ||
| - [ ] `test_session_failover_rebuilds_from_persisted_state_without_resuming_inflight_turn` | ||
| - [ ] `uv run pytest tests/agentkit/ tests/ui/ -k "owner or lease or fencing or failover" -v` | ||
|
|
||
| ## References | ||
|
|
||
| - `docs/adr/0012-complete-phase1-postgresql-http-session-persistence.md` | ||
| - `docs/adr/0003-http-sessions-use-one-stable-tape-timeline.md` | ||
| - `src/coding_agent/ui/session_manager.py` | ||
| - `src/coding_agent/ui/http_server.py` | ||
| - `src/coding_agent/ui/session_store.py` | ||
| - `src/agentkit/storage/pg.py` | ||
| - Historical design context: `docs/specs/checkpoint-design-section2a.md` |
66 changes: 66 additions & 0 deletions
66
...gent/docs/adr/0014-separate-session-execution-binding-from-runtime-ownership.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| # ADR-0014: Separate session execution binding from runtime ownership | ||
|
|
||
| **Status**: Proposed | ||
| **Date**: 2026-04-19 | ||
|
|
||
| ## Context | ||
|
|
||
| ADR-0013 defines Phase 2 multi-instance session ownership through owner records, leases, and fencing tokens. That layer answers "which instance may drive this session right now." It does not answer "where do the session's tools and files run," and it does not address how a session moves between execution environments while keeping the same durable identity. | ||
|
|
||
| Today `SessionManager` conflates three concepts inside the `Session` object and its `repo_path` field: | ||
|
|
||
| 1. **Durable session metadata** (id, provider, model, policy, tape_id) that survives restarts. | ||
| 2. **Execution/workspace binding** (`repo_path`, tool filesystem scope, shell working directory) that decides where `file_read`, `file_write`, and `bash_run` operate. | ||
| 3. **Current runtime owner** (`task`, `approval_event`, `event_queues`, `runtime_pipeline`, `runtime_ctx`, `runtime_adapter`) that is local-authoritative and ephemeral. | ||
|
|
||
| These three concepts change on different timelines. Session metadata changes when the user switches models or policies. Execution binding changes when a user moves from a local repository to a cloud workspace, or when a local `repo_path` is remounted. Runtime owner changes every time a lease expires, an instance restarts, or load balancing shifts traffic. | ||
|
|
||
| Because `SessionManager` stores `repo_path` directly on the session and passes it to `create_agent_for_session` as `workspace_root`, there is no place to introduce a cloud workspace binding without overloading `repo_path` with incompatible semantics. The HTTP server also keeps approval state, event queues, and task ownership in local memory, which makes it impossible to reason about failover without first knowing whether the new owner can satisfy the old owner's execution contract. | ||
|
|
||
| ## Decision | ||
|
|
||
| Introduce a first-class **execution binding** abstraction that sits beside the ownership layer defined in ADR-0013. The binding layer decides where a session's tools execute; the ownership layer decides which instance drives the session. The two layers are orthogonal and change independently. | ||
|
|
||
| Specifically: | ||
|
|
||
| - Add an `ExecutionBinding` dataclass/protocol in `coding_agent.ui.execution_binding` with at least two concrete variants: | ||
| - `LocalExecutionBinding(workspace_root: str)` for local filesystem mode. | ||
| - `CloudWorkspaceBinding(workspace_url: str, workspace_id: str)` for remote workspace mode. | ||
| - Store the serialized binding on the session metadata table as `execution_binding`, separate from `repo_path`. Migrate the current `repo_path` field to mean "local workspace root" only. | ||
| - Add a `BindingResolver` interface that turns an `ExecutionBinding` into the `workspace_root` and tool configuration that `create_agent_for_session` expects. The resolver lives in `coding_agent.ui.binding_resolver`. | ||
| - Update `SessionManager` to resolve the binding before calling `create_agent_for_session`, so `run_agent`, `ensure_session_runtime`, and checkpoint restore all use the resolved workspace consistently. | ||
| - Update `Session.to_store_data` and `Session.from_store_data` to round-trip the binding as a typed dict, and default missing bindings to `LocalExecutionBinding(repo_path)` for backward compatibility. | ||
| - Keep the `Session` runtime fields (`task`, `approval_event`, `event_queues`, `runtime_pipeline`, `runtime_ctx`, `runtime_adapter`) untouched. Those belong to the ownership layer (ADR-0013) and remain local-authoritative. | ||
|
|
||
| This ADR does not add cloud workspace tool implementations. It only creates the abstraction and the local binding variant so that future cloud bindings slot in without changing `SessionManager` or `http_server.py`. | ||
|
|
||
| ## Alternatives Rejected | ||
|
|
||
| - Overload `repo_path` to mean both local path and cloud workspace identifier — rejected because it creates a semantic mismatch. A `Path` cannot represent a cloud workspace URL, and conditional parsing based on string prefixes is fragile. | ||
| - Encode binding information inside the `CheckpointSnapshot.extra` field — rejected because binding is session-level metadata, not checkpoint-level state. Restoring a checkpoint should not change where tools execute. | ||
| - Wait until cloud workspace tools exist before adding the abstraction — rejected because `SessionManager` and `http_server.py` already carry local-only assumptions (approval state, event queues, task ownership) that will harden into technical debt without a binding boundary. | ||
| - Merge execution binding and runtime owner into a single "session placement" record — rejected because owner identity and workspace identity change on different timelines. A session can fail over to a new owner while keeping the same binding, and a session can change binding while staying on the same owner. | ||
|
|
||
| ## Acceptance Criteria | ||
|
|
||
| - [ ] `test_local_binding_resolves_to_workspace_root_for_create_agent` | ||
| - [ ] `test_session_metadata_round_trips_local_execution_binding` | ||
| - [ ] `test_session_metadata_defaults_missing_binding_to_local_from_repo_path` | ||
| - [ ] `test_cloud_workspace_binding_serializes_to_typed_dict` | ||
| - [ ] `test_binding_resolver_rejects_unknown_binding_kind` | ||
| - [ ] `test_run_agent_uses_resolved_workspace_root_from_binding` | ||
| - [ ] `test_restore_checkpoint_preserves_execution_binding` | ||
| - [ ] `test_http_server_create_session_stores_local_binding_by_default` | ||
| - [ ] `uv run pytest tests/ui/test_session_manager_public_api.py tests/ui/test_session_manager_runtime.py tests/ui/test_http_server.py -k "binding" -v` | ||
|
|
||
| ## References | ||
|
|
||
| - `docs/adr/0013-define-phase2-multi-instance-session-ownership-for-pg-http-sessions.md` | ||
| - `docs/adr/0012-complete-phase1-postgresql-http-session-persistence.md` | ||
| - `src/coding_agent/ui/session_manager.py` | ||
| - `src/coding_agent/ui/http_server.py` | ||
| - `src/coding_agent/app.py` | ||
| - `src/coding_agent/plugins/core_tools.py` | ||
| - `tests/ui/test_session_manager_public_api.py` | ||
| - `tests/ui/test_session_manager_runtime.py` | ||
| - `tests/ui/test_http_server.py` | ||
168 changes: 168 additions & 0 deletions
168
coding-agent/docs/pg-phase2-multi-instance-checklist.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| # PostgreSQL Phase 2 — Multi-instance checklist | ||
|
|
||
| This document is an implementation-oriented companion to ADR-0013. It does not replace the ADR; it expands the concrete pieces that Phase 2 needs so implementation can be planned incrementally. | ||
|
|
||
| ## Scope Summary | ||
|
|
||
| Phase 1 made these durable in PostgreSQL: | ||
|
|
||
| - HTTP session metadata | ||
| - tape history | ||
| - checkpoint snapshots/catalogs | ||
|
|
||
| Phase 2 defines who is allowed to act on that persisted state when more than one HTTP instance exists. | ||
|
|
||
| ## 1. Ownership data model | ||
|
|
||
| Add a dedicated owner/lease table, for example: | ||
|
|
||
| ```sql | ||
| CREATE TABLE session_owners ( | ||
| session_id TEXT PRIMARY KEY, | ||
| owner_id TEXT NOT NULL, | ||
| lease_expires_at TIMESTAMPTZ NOT NULL, | ||
| fencing_token BIGINT NOT NULL, | ||
| updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() | ||
| ); | ||
| ``` | ||
|
|
||
| Recommended semantics: | ||
|
|
||
| - `owner_id`: stable process/pod identifier | ||
| - `lease_expires_at`: absolute lease expiry | ||
| - `fencing_token`: monotonically increasing per successful acquire | ||
| - `updated_at`: audit/debug support | ||
|
|
||
| ## 2. Ownership primitives | ||
|
|
||
| Define one narrow app-layer interface for ownership: | ||
|
|
||
| ```python | ||
| class SessionOwnerStore(Protocol): | ||
| async def acquire(self, session_id: str, owner_id: str, lease_ttl_seconds: float) -> OwnerLease: ... | ||
| async def renew(self, session_id: str, owner_id: str, fencing_token: int, lease_ttl_seconds: float) -> OwnerLease: ... | ||
| async def release(self, session_id: str, owner_id: str, fencing_token: int) -> None: ... | ||
| async def get(self, session_id: str) -> OwnerLease | None: ... | ||
| ``` | ||
|
|
||
| Acquire rules: | ||
|
|
||
| - succeed if no live lease exists | ||
| - fail if another owner still holds a live lease | ||
| - increment fencing token on every successful acquire | ||
|
|
||
| Renew rules: | ||
|
|
||
| - only current owner + current token may renew | ||
| - stale token must fail | ||
|
|
||
| Release rules: | ||
|
|
||
| - only current owner + current token may release | ||
| - stale release must be ignored/rejected | ||
|
|
||
| ## 3. Runtime owner checks | ||
|
|
||
| Guard these actions behind ownership validation: | ||
|
|
||
| - `SessionManager.run_agent` | ||
| - `SessionManager.capture_checkpoint` | ||
| - `SessionManager.restore_checkpoint` | ||
| - `SessionManager.close_session` | ||
| - `SessionManager.shutdown_session_runtime` | ||
|
|
||
| Minimum rule: | ||
|
|
||
| - if this instance is not the active owner for the session, do not execute runtime-sensitive work | ||
| - return a conflict/error that callers can treat as a retry or routing problem | ||
|
|
||
| ## 4. Fencing points | ||
|
|
||
| Fencing is only useful if stale owners are rejected at side-effect boundaries. | ||
|
|
||
| Phase 2 MVP should require fencing on: | ||
|
|
||
| - turn start (`run_agent`) | ||
| - checkpoint restore | ||
| - session close | ||
| - approval response submission | ||
|
|
||
| Phase 2 later can extend fencing to: | ||
|
|
||
| - event publishing metadata | ||
| - checkpoint capture metadata | ||
| - owner-sensitive session metadata writes | ||
|
|
||
| ## 5. Event routing | ||
|
|
||
| Current HTTP event queues are local in-memory structures. Phase 2 must choose a routing model. | ||
|
|
||
| ### MVP recommendation: sticky routing | ||
|
|
||
| - clients reconnect to the same owner instance | ||
| - only owner instance serves live event stream for a session | ||
| - non-owner instance returns a conflict/redirect-style error or terminates the stream cleanly | ||
|
|
||
| ### Future option: brokered routing | ||
|
|
||
| - owner publishes events to shared event transport | ||
| - any edge instance can proxy SSE | ||
|
|
||
| Do not attempt brokered routing in the first Phase 2 slice unless ownership is already stable. | ||
|
|
||
| ## 6. Approval routing | ||
|
|
||
| Current approval state is runtime-local. Phase 2 needs explicit routing: | ||
|
|
||
| - approval requests are owned by the current session owner | ||
| - approval responses must include session + request identity and be rejected if owner/token changed | ||
|
|
||
| Suggested persisted approval metadata: | ||
|
|
||
| - `request_id` | ||
| - `session_id` | ||
| - `owner_id` | ||
| - `fencing_token` | ||
| - `status` | ||
| - `created_at` | ||
|
|
||
| ## 7. Failover semantics | ||
|
|
||
| Phase 2 MVP should choose **at-most-once** for in-flight turns. | ||
|
|
||
| That means: | ||
|
|
||
| - if the owner dies during a turn, the turn is not resumed mid-flight | ||
| - a new owner may rebuild cold state from persisted session metadata, tape history, and checkpoints | ||
| - clients must retry or reconnect after failover | ||
|
|
||
| This is consistent with the current repository boundary where runtime objects remain local-authoritative and are not serializable. | ||
|
|
||
| ## 8. Restore / close consistency | ||
|
|
||
| Define these invariants: | ||
|
|
||
| - `run_agent`, `restore_checkpoint`, and `close_session` are mutually exclusive owner-sensitive operations | ||
| - non-owner instances cannot execute them locally | ||
| - stale owners must fail before mutating shared state | ||
|
|
||
| ## 9. Suggested implementation order | ||
|
|
||
| 1. Add `session_owners` table + PG store | ||
| 2. Add acquire / renew / release APIs + tests | ||
| 3. Add owner checks to `run_agent` | ||
| 4. Add owner checks to restore / close / shutdown | ||
| 5. Add approval request/response ownership metadata | ||
| 6. Add sticky-routing behavior for SSE and approval endpoints | ||
| 7. Add failover tests and operational metrics | ||
|
|
||
| ## 10. Suggested tests | ||
|
|
||
| - `test_session_owner_acquire_conflicts_with_live_lease` | ||
| - `test_session_owner_renew_rejects_stale_token` | ||
| - `test_session_owner_release_rejects_stale_token` | ||
| - `test_run_agent_rejects_non_owner_instance` | ||
| - `test_close_session_rejects_non_owner_instance` | ||
| - `test_restore_checkpoint_rejects_stale_owner` | ||
| - `test_approval_response_is_rejected_after_owner_change` | ||
| - `test_failover_rebuilds_runtime_from_persisted_tape_without_resuming_inflight_turn` |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ADR-0014 states
LocalExecutionBinding(workspace_root: Path), but the implementation usesworkspace_root: str(and the binding resolver converts it to Path). Please align the ADR wording with the code (or change the code to store a Path) to avoid misleading future implementers about the binding type and serialization expectations.