docs(plan): approvals incident fixes — root cause, Zed comparison, and execution plan

Author: mmabrouk

docs/design/agent-workflows/projects/approvals-incident-fixes/README.md

@@ -0,0 +1,50 @@
+# Concurrent approvals incident fixes
+
+This workspace plans the fixes for the concurrent human-approval failure observed in live
+session `db58551b` on 2026-07-19, plus the session-turns counter bug found in the same logs.
+The incident and its seven defects are documented and log-verified in
+`docs/design/agent-workflows/scratch/debug-concurrent-approvals-db58551b.md`; the design
+principles we adopt from Zed's handling of the same problem are in
+`docs/design/agent-workflows/scratch/zed-acp-approvals-comparison.md`; the counter bug is in
+`docs/design/agent-workflows/scratch/debug-session-turns-append-500.md`.
+
+## Reading order
+
+1. `context.md` explains why this work exists, what it must achieve, and what is
+   deliberately out of scope.
+2. `research.md` holds the code-verified findings (R1 through R8) that the plan is built
+   on, with file and line evidence for every claim, and a prominent list of open risks.
+3. `plan.md` is the implementation plan: five ordered steps, each with exact files,
+   behavioral contracts, acceptance criteria, and test commands. Each step is independently
+   landable. The implementer is expected to work from this file without having read the
+   incident conversation.
+4. `qa.md` is the live QA script that reproduces the incident shape against the dev stack
+   and states the expected correct behavior at each point.
+5. `status.md` tracks where the project stands.
+
+## Glossary
+
+Every domain term used in this workspace, one line each.
+
+- **Runner**: the Node/TypeScript sidecar under `services/runner/` that drives a coding-agent
+  harness inside a sandbox and streams events to the web UI.
+- **Harness**: the agent program that implements the model-and-tool loop, such as Pi
+  (`pi_core`) or Claude Code (`claude`). The runner talks to it over ACP, the Agent Client
+  Protocol.
+- **Gate**: a human-approval request. When the harness wants to run a permission-gated tool,
+  the runner shows the user an approval card and waits for allow or deny.
+- **Park**: ending the browser-facing turn while an unanswered gate keeps the live harness
+  process waiting. The answer arrives in a later request.
+- **Warm resume**: continuing a parked session in place. The runner checks the live process
+  out of the keepalive pool and answers the original permission request by its id, so the
+  approved tool runs with its original arguments.
+- **Keepalive pool**: the runner's bounded collection of live parked sessions, each with a
+  time-to-live limit after which it is destroyed.
+- **Records**: the durable per-session event stream. The runner posts every agent event to
+  the API's record-ingest endpoint; the frontend rebuilds a conversation from these rows
+  (that rebuild is called hydration).
+- **Interactions**: the durable table of human-in-the-loop requests. Each gate creates one
+  interaction row with a lifecycle status (pending, responded, resolved, cancelled).
+- **Turns**: the append-only `session_turns` table. One row per completed conversation turn,
+  carrying the harness's native session id so a restarted runner can resume the conversation
+  natively instead of replaying text.

docs/design/agent-workflows/projects/approvals-incident-fixes/context.md

@@ -0,0 +1,58 @@
+# Context: why this work exists
+
+## The incident in three sentences
+
+During live QA of two parallel permission-gated shell commands (session `db58551b`,
+2026-07-19), both commands executed exactly once and only after their approvals, but the
+system misreported both, flipped an already-approved card back to "waiting", and then went
+silent because the user's final approval was never sent to the runner. The root causes are
+that the durable session record stores every approval request but never any approval answer,
+that the post-pause cleanup invents tool results (a false "not executed" error for an
+approved running call, and a false success for a call that never started), and that the
+frontend only dispatches approvals once every visible card looks settled, a condition that
+can never hold after a state rebuild. A seventh, unrelated defect found in the same logs
+makes the new `session_turns` ingestion fail with HTTP 500 on every warm turn, because the
+turn index is computed once per environment acquire instead of once per turn.
+
+## Goals
+
+1. Fix the session-turns counter so `turn_index` is a true conversation-turn counter, and
+   make the API answer a duplicate append with 409 Conflict instead of 500.
+2. Stop the pause cleanup from inventing tool results: an approved, executing call keeps its
+   real result, and a never-started call is recorded as deferred, never as success.
+3. Persist the answer half of every gate (a new `interaction_response` record event plus the
+   allow/deny verdict in the interaction row's existing `resolution` field), and make
+   frontend hydration overlay answers onto requests, so a rebuilt conversation shows
+   answered cards as answered.
+4. Dispatch each approval per card the moment the user answers it, and make the runner
+   accept a resume that answers only part of the parked gates while remaining parked on the
+   rest.
+5. Add a regression test that reproduces the exact incident shape, then verify with live QA
+   on the dev stack.
+
+## Non-goals
+
+Two related fixes are deliberately out of scope here.
+
+- **Real turn cancellation** (the Zed-style cancel-then-restart when new user text arrives
+  while gates are parked, which fixes defect 6 of the incident report) goes to Arda after
+  JP's sessions PRs merge, because it depends on the session-streams control plane those PRs
+  introduce and is partly a product decision.
+- **Audit-hardening of record ids** (scoping stable record ids by turn so contradictory
+  results append instead of overwriting, defect 5) is queued separately, because the answer
+  persistence in this project already fixes the user-facing rebuild problem and the id
+  rescope touches the record identity contract end to end.
+
+## Constraints already decided
+
+- Work lands in this order, on these lanes: the counter fix on the existing rebase lanes for
+  PR #5376 (runner, branch `sessions-rebase/runner`) and PR #5375 (backend, branch
+  `sessions-rebase/backend`), with a wipe of the wrongly numbered dev rows and an
+  explanatory note for JP; everything else on the PR #5382 lane (branch
+  `plan/concurrent-approvals`).
+- `turn_index` is confirmed to be a true conversation-turn counter, not an acquire counter.
+  The implementation must carry a code comment stating this invariant, and the PR body must
+  explain what was done and what was assumed.
+- The implementation will be done by an agent that has not read the incident conversation,
+  so every step in `plan.md` is specified with exact files, behavioral contracts, and
+  acceptance criteria.