Skip to content

feat(workers): persist fenced delivery results (W1-03f-a) - #301

Merged
mergify[bot] merged 3 commits into
mainfrom
codex/w1-03f-a-delivery-state
Oct 4, 2026
Merged

mergify[bot] merged 3 commits into
mainfrom
codex/w1-03f-a-delivery-state

Conversation

@Cuarroc

@Cuarroc Cuarroc commented Oct 4, 2026 •

Copy link
Copy Markdown
Owner

Dieser Teil speichert die Abschlussmeldung eines Worker-Laufs dauerhaft im vorhandenen Laufdatensatz. Ein wiederholtes „done“ verändert nichts, während falsche Läufe und veraltete Fences abgewiesen werden. Eine Blockierung wird zusammen mit ihrem konkreten Grund gespeichert.

Report

What changed

  • Added the worker-layer WorkerDelivery::{Done, Blocked} contract and record_worker_delivery in src-tauri/src/workers/delivery_state.rs:8-65.
  • Reused the existing durable development_runs terminal states; no schema change or store-lane edit was needed.
  • Made identical terminal reports idempotent only after the existing serialized store transition has validated the live owner/fence. Stale fences and another run retain their refusal.
  • Persisted a trimmed non-empty blocked reason as the failed run terminal detail.
  • Existing delivery anchors inspected: src-tauri/src/workers.rs:645-688, src-tauri/src/workers.rs:831, src-tauri/src/store/development_delivery.rs:38-113, and src-tauri/src/store/development_runs.rs:546-580.

Evidence

  • Base: origin/main 855850aa5162cc9100d1a5afdd9faee1a33e29d2 (fetched 2026-10-04).
  • Red commit: c9968f7d262929d8fddddf058cb3d800835a9a7e.
  • cargo test --bin projecta worker_delivery on the red commit: exit 101; both repeated_worker_delivery_done_is_idempotent and wrong_run_or_fence_cannot_complete_worker_delivery failed for the intended reasons.
  • cargo test --bin projecta worker_delivery at the final candidate: exit 0; 3 passed.
  • bash scripts/ci/gates.sh lane prepush at a066d870a0e39c6e61e61052716af71e2348cb05: exit 0; log /tmp/srv-w1-03f-a-prepush.log on the worker host. Rust suite: 1708 passed, 7 skipped.
  • Push hook reran the prepush lane at the same SHA: exit 0.
  • Remote verification: git ls-remote origin refs/heads/codex/w1-03f-a-delivery-state returned a066d870a0e39c6e61e61052716af71e2348cb05.
  • Diff size: 176 added lines, 0 removed lines across 2 files.

NICHT ABGEDECKT

  • Linux does not compile or run the #[cfg(windows)] tests or the Windows clippy arms; the Windows lane runs in the merge queue.
  • Slice b (src-tauri/src/bin/pa.rs) is intentionally not included, so pa worker done|blocked is not yet wired to this worker-layer contract.
  • No GUI or desktop runtime was launched.
  • The PC-only Z-1 delivery protocol was not run in this headless Linux slice.

Review status

Reviewer B (Claude Sonnet 5.5, Anthropic; author family codex), candidate a066d87, then f00f524.

Review disposition

  • Checked: authority (require_run_authority, development_runs.rs:1009) runs before the status check in transition_run (:965 vs :966), so a replay is only recognised for the live owner/fence; the string-matched error is the exact terminal-to-terminal message and the replay test would fail if it changed. No second pattern: existing store APIs and String errors are used. No high or medium finding.
  • Low (accepted, f00f524): replay with a different blocked reason, a whitespace-only difference, and done-after-blocked were untested. Added worker_delivery_replay_never_changes_a_different_terminal_result; test-only, passes on unchanged code (No-Test trailer).
  • Low (rejected, follow-up for slice b): matching on the error string is brittle against store message changes; a typed error belongs to the store seam, which this package may not edit (rule 6).
  • bash scripts/ci/gates.sh lane prepush at f00f524: exit 0.

Open points

  • W1-03f slice b must expose this contract through pa worker done|blocked without weakening the credential-bound run identity.

Test-First: src-tauri/src/workers.rs::repeated_worker_delivery_done_is_idempotent
Test-First: src-tauri/src/workers.rs::wrong_run_or_fence_cannot_complete_worker_delivery
Co-Authored-By: GPT-5 <47299143+Cuarroc@users.noreply.github.com>
Test-First: src-tauri/src/workers.rs::repeated_worker_delivery_done_is_idempotent
Test-First: src-tauri/src/workers.rs::wrong_run_or_fence_cannot_complete_worker_delivery
Co-Authored-By: GPT-5 <47299143+Cuarroc@users.noreply.github.com>
@Cuarroc
Cuarroc marked this pull request as ready for review October 4, 2026 14:46
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-04T14:50:54.392568Z a066d87 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a066d870a0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +52 to +54
let run = store
.get_development_run(run_id)
.await?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep replay authorization inside the transaction

When the initial transition finds an already-terminal run, its BEGIN IMMEDIATE transaction is released before this separate read. A concurrent task checkpoint and reclaim can change continuous_tasks.claim_owner or claim_fence in that gap, while the immutable run row still contains the old credentials and terminal status; the checks below then return Ok to a stale worker. Validate and return an idempotent terminal replay within the same store transaction, including the live task authority check.

Useful? React with 👍 / 👎.

Comment on lines +23 to +25
store
.complete_development_run(run_id, owner, fence, None)
.await,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Do not terminalize the run before native cleanup

When a native worker invokes the planned self-report command while its agent process is still running, this immediately changes the run from launched to completed (and the blocked branch similarly changes it to failed). The subsequent native exit path in store/native_completion.rs only accepts intent, launched, or reconciling at line 149 and requires reconciling at line 47, so capture finalization and session cleanup fail after either report, leaving launch/accounting state unresolved. Persist the worker's report separately or defer the terminal run transition until native cleanup has completed.

Useful? React with 👍 / 👎.

@mergify

mergify Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Queued — the merge queue status continues in this comment ↓.

No-Test: test-only coverage of existing behaviour; no production change.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@Cuarroc

Cuarroc commented Oct 4, 2026

Copy link
Copy Markdown
Owner Author

@Mergifyio queue

@mergify

mergify Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

This pull request spent 44 minutes 2 seconds in the queue, including 11 minutes 20 seconds running CI.

Required conditions to merge

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant