Skip to content

[audit] Side-effecting operations retried/replayed without idempotency keys #6556

Description

@7jrxt42BxFZo4iAnN4CX

Source: static audit of main@384439634. Candidates are static and not reproduced — confirm each before changing code.
Template: agent-task (agent-ready, docs/ISSUE_TRIAGE.md). Labels: bug, agent-ready.

Goal / Why

Tool calls, MCP calls, HTTP turns, bridge messages, and forge actions are retried or replayed on ambiguous failure without an operation_key/CAS. When a request times out or the connection drops the operation may already have been applied, so the retry applies it a second time. Duplicate side effects range from noisy (a double comment) to costly (double billing, divergent remote state, a duplicate PR). The fix is an operation key (or compare-and-swap) on every side-effecting call on the retry path, so replay can detect an already-applied operation.

Confirmed static findings in this class: 20 (reassessed severity noted per line).

Scope / Plan

  1. Re-read each candidate below at the cited file:line on current main; drop anything already fixed or misread.
  2. Introduce an operation key (or CAS) for every side-effecting call on the retry path; make replay detect an already-applied operation.
  3. Add the smallest regression/gate that would catch a re-introduction (focused test, budget script, or grep guard).

Key files

  • crates/tui/src/core/engine/turn_loop.rs
  • crates/tui/plugins/computer-use/src/app-socket.mjs
  • crates/tui/plugins/computer-use/src/backends/darwin-accessibility.m
  • crates/tui/src/computer_meter.rs
  • crates/tui/src/cloud_dispatch.rs
  • crates/tui/src/mcp.rs
  • crates/tui/src/tui/widgets/agent_card.rs
  • crates/tui/src/fleet/store.rs
  • crates/tui/src/plugins/marketplace/parsers/codewhale.rs
  • crates/tui/src/llm_client/mod.rs
  • crates/tui/src/tui/control_socket.rs
  • crates/tui/src/runtime_api.rs
  • crates/tui/src/runtime_mobile.html
  • crates/tui/src/runtime_threads.rs

Acceptance criteria

  • Every listed candidate is either fixed with a test, or downgraded with a written reason in the PR.
  • No new instance of this class is introduced (guarded by the test/budget).
  • Existing behavior for unrelated paths is unchanged.

Verification

cargo check -p codewhale-tui
cargo test  -p codewhale-tui --lib
cargo clippy --workspace --all-targets --locked -- -D warnings
cargo deny check bans advisories   # if cargo-deny is installed; see .github/workflows/cargo-deny.yml
( cd pet && npm run check )
( cd web && npm run check )

Out of scope

  • Findings that belong to another systemic class or to the localized backlog.
  • Re-architecture beyond the listed sites.

Related existing work (do not duplicate)

#5199 (stack R5), #6252 (sticky route binding), #6096 (session-export slice)

Candidate findings (20)

  • CU01-07 · CU01 — Computer-use · crates/tui/plugins/computer-use/src/app-socket.mjs:64 — timeout/cancel after dispatch returns cancelled without outcome_unknown, encouraging duplicate action.
    audit severity: high; reassessed: major
  • CU01-11 · CU01 — Computer-use · crates/tui/plugins/computer-use/src/backends/darwin-accessibility.m:867 — duplicate app name/bundle first-match resolver can send input to the wrong Chrome/profile/window.
    audit severity: high; reassessed: major
  • T01-08 · T01 — TUI entry, process lifecycle, support · crates/tui/src/computer_meter.rs:474 — meter interval can cross expiry, replay digest checks only a subset of fields.
    audit severity: high; reassessed: major
  • T06-01 · T06 — Remote control, MCP, OAuth, cloud dispatch · crates/tui/src/cloud_dispatch.rs:291 — cancel/phase-save race resurrects a canceled job; fixed shared .tmp races writers.
    audit severity: high; reassessed: major
  • T06-03 · T06 — Remote control, MCP, OAuth, cloud dispatch · crates/tui/src/mcp.rs:4932 — ambiguous failed tools/call automatically replayed without idempotency.
    audit severity: high; reassessed: major
  • U05-04 · U05 — Work surface, widgets, agent cards/panels · crates/tui/src/tui/widgets/agent_card.rs:574 — delayed mailbox event after terminal reopens delegate/fanout and duplicates counts.
    audit severity: high; reassessed: major
  • C02-04 · C02 — TUI engine and turn loop · crates/tui/src/core/engine/turn_loop.rs:1443 — reasoning-only retry nudge visible to model but absent from session log; needs durable runtime event/replay.
    audit severity: major; reassessed: major
  • C02-14 · C02 — TUI engine and turn loop · crates/tui/src/core/engine/turn_loop.rs:2906 — tool admission does not bound cardinality/batch and duplicate IDs before side effects.
    audit severity: major; reassessed: major
  • F01-04 · F01 — Fleet and plugins · crates/tui/src/fleet/store.rs:290, crates/tui/src/plugins/marketplace/parsers/codewhale.rs:161 — schema/identity validation is incomplete; unbounded control-containing values and duplicate names are preserved.
    audit severity: major; reassessed: major
  • T02-07 · T02 — Model HTTP/SSE clients · crates/tui/src/llm_client/mod.rs:1246 — ambiguous POST transport failures are retried without idempotency, creating duplicate billing/side effects.
    audit severity: major; reassessed: major
  • T05-02 · T05 — Runtime API, ACP, control socket, web/mobile · crates/tui/src/tui/control_socket.rs:824 — response timeout does not cancel a queued command; retry can duplicate message/relaunch.
    audit severity: major; reassessed: major
  • T05-07 · T05 — Runtime API, ACP, control socket, web/mobile · crates/tui/src/runtime_api.rs:6231 — omitted replay limit/full recovery replay unbounded.
    audit severity: major; reassessed: major
  • T05-08 · T05 — Runtime API, ACP, control socket, web/mobile · crates/tui/src/runtime_mobile.html:575 — reconnect always since_seq=0, duplicate/stale events can overwrite activeTurnId.
    audit severity: major; reassessed: major
  • T08-04 · T08 — Runtime threads, tasks, automation, goals · crates/tui/src/runtime_threads.rs:2144 — concurrent Complete/Blocked use only goal_id, terminal state is overwritten without revision CAS.
    audit severity: major; reassessed: major
  • U02-03 · U02 — Event loop, dispatch/apply · crates/tui/src/tui/ui/dispatch.rs:91 — hook-replaced queued message remains echoed, then a duplicate User history cell is added.
    audit severity: major; reassessed: major
  • U04-m2 · U04 — History, transcript, markdown, diff, streaming · crates/tui/src/tui/history/latex_render.rs:995 — Sqrt root duplicates/misaligns the radical and reprocesses the argument.
    audit severity: medium; reassessed: medium
  • W02-02 · W02 — Website API, Cloudflare/KV, generators · web/app/api/admin/post/route.ts:78 — GitHub side effect before KV mark, no idempotency/CAS; duplicate posts/race discard.
    audit severity: high; reassessed: high
  • W02-05 · W02 — Website API, Cloudflare/KV, generators · web/lib/community-agent-tasks.ts:100, web/lib/community-agent.ts:371, web/lib/facts-drift.ts:320 — KV read-modify-write/check-LLM-write non-atomic, duplicates/lost usage/stale overwrite.
    audit severity: major; reassessed: major
  • P01-07 · P01 — Cross-platform Pet · pet/scripts/pet.mjs:58 — Catch-up after suspension sequentially replays every missed interval without bound.
    audit severity: major; reassessed: major
  • O02-11 · O02 — Policy, dev/install tooling, Unicode patch · deny.toml:20 — Duplicate dependency versions only warn; name-only bans skip all versions.
    audit severity: major; reassessed: major

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageNew external report awaiting maintainer triage; repro, logs and version output help

    Projects

    • Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions