Repository navigation
Lock claims decided by confirmation; kickoff undo and cleanup release locks; reducer replays late arrivals - #131
Draft
magnificentlycursed wants to merge 14 commits into
Draft
magnificentlycursed wants to merge 14 commits into
magnificentlycursed wants to merge 14 commits into
Conversation
…off rollback increment The umbrella states the write-path problem, the measured and estimated evidence, five increments named by feature in build order, and the cross-increment decisions (lock-clearing wire format reusing the generation protocol gate; git binary resolution on macOS; scaffold-first builds; a work-in-progress limit). The first increment's document specifies claim outcomes decided by a fallible, network-bounded confirmation, kickoff undo for run and plan, and kickoff cleanup that releases the agent's lock and session. Both were reviewed in two rounds by one Solution Architect reviewer.
…back increment Adds the types and stubs the increment builds on, and the failing tests that define it (design: .design/lock-claim-outcome-and-kickoff-rollback.md). - LockClaimResult::Unconfirmed: a published claim whose confirmation did not complete; session work refuses it with a rerun message, and the other callers report it. - The Published outcome type and confirm_v3_locks_bounded (stub). - ClaimFailpoints on SharedWriter (test builds only) to inject slow publication, failed or hung confirmation fetches, failures after the push, and a slow reduce. - KickoffRollback, UndoStep, UndoOps and branch_deletable (stubs), and an optional base_commit in kickoff metadata. Red until the implementation lands: five claim tests against real v3 hubs (slow publication, failed confirmation and rerun, failure after push, bounded confirmation fetch, already-held confirmation) and eight rollback unit tests. Guards that pass: the slow-reduce claim, the metadata round trip, and the existing claim and release tests. Session-level and kickoff integration criteria come with the implementation. AC-4 in the design is amended: its bound is checked against the hung fetch, not 4 s, because publication and the pre-confirmation read are outside the deadline.
…fs; cleanup releases locks A lock claim is no longer reported as failed because its publication took more than 30 s. The outcome comes from a confirmation against a fresh fetch: the hub-lock wait and the fetch share the 30 s bound, and the local reduce of the fetched snapshot runs to completion. A failed or timed-out confirmation, or any failure after the push lands, is Unconfirmed, which is not ownership; session work refuses it with a rerun message, and the rerun confirms without publishing a second claim. An already-held lock is confirmed against the remote, not the local view. - commit_v3 and the emit path report whether the events were published. - The confirmation fetch runs through utils::command_output_with_timeout, promoted from kickoff and extended to run in its own process group and terminate it with SIGTERM, then SIGKILL after 500 ms, on Unix. - The v3 fetch is split into a fetch command and adopt_fetched_v3_refs. Kickoff run and plan undo what they created when they fail before the agent starts: container or tmux session, the issue lock (released and verified under the worktree agent's identity), the session, the worktree daemon, the pipeline state, and the worktree and branch when this invocation created them. Kickoff metadata records the branch's base commit. kickoff cleanup brings the worktree daemon up to release the agent's locks and end its session, then removes the worktree and deletes the branch when it has no commits beyond its recorded base (--keep-branch keeps it). Steps that do not complete are reported with their remedy. Fixes #110 and the timeout half of #118. Design: .design/lock-claim-outcome-and-kickoff-rollback.md (amended during the build; the amendments and test coverage are recorded there).
…remote checkpoint Lock confirmation adopted the fetched checkpoint, and verifying a remote checkpoint rebuilds state from every event with a per-event ssh-keygen signature check. On the shared hub that made the confirmation of an already-held lock take 56 s and a new claim 302 s on macOS. The lock table comes from reducing the agent refs; the checkpoint only shortens the replay. Confirmation now adopts other agents' fetched ref tips (adopt_fetched_agent_tips) and reduces them, and leaves checkpoint adoption to the ordinary fetch. A test checks that the confirmation alone finds a claim the checkout never fetched.
…stop that finished late is not a failure A manual check of kickoff undo after an activation failure across a sleep showed the lock release and session end failing on the worktree's stale readiness, and the daemon stop reported as failed although the process had exited. Undo now ensures the worktree daemon grants mutations before it releases the lock or ends the session, as cleanup already did, and treats a daemon stop error as done when the daemon is gone within a further 5 s.
…nt succeeds The process that spawns a daemon dropped its child handle without waiting on it. When that process is still running as the daemon exits, as kickoff is when its undo stops the worktree daemon, the daemon lingers as a zombie that kill -0 still reports alive, and daemon stop fails with "did not exit after forced shutdown". A background thread now waits on the child, so it is reaped when it exits; a short-lived parent exits first and the daemon is adopted as before.
…kickoff rollback Results on this Mac with installed builds of the branch: claims complete without stranding (85 s new, 34 s already held after the confirmation change); kickoff undo after an activation failure leaves nothing behind after the readiness and reaper fixes; cleanup of a stopped agent releases its lock, stops its daemon and removes its worktree and unchanged branch. Out-of-scope observations (the daemon exiting after writes over 90 s) are noted for the later increments.
Correctness: - Lock confirmation fails when any other agent's fetched ref fails to adopt, and on v3 a confirmed view with no lock is Unconfirmed, not ownership. - The confirmation fetch never prompts: no stdin, GIT_TERMINAL_PROMPT=0, and ssh BatchMode appended to the user's ssh command (GIT_SSH is left alone). - command_output_with_timeout drains output on reader threads while waiting and waits at most a second for it once the command has exited. - locks steal and lock auto-steal act on the claim result instead of reporting success; the auto-steal test now asserts the untaken steal that v3 returns until tracker #818 is fixed. - Undo releases the lock through the worktree's command service and classifies the outcome from a bounded confirmation (stranded, unverified or done) whatever the release reported. - The undo report follows the original error in the returned error, so swarm and sentinel keep it. - kickoff cleanup reads the agent's locks from the host's view and wakes the worktree daemon only when there is one to release, stops daemons through the shared undo step, and exits non-zero when it leaves work behind. - On a reused worktree, undo leaves a pre-existing agent, daemon and lock as found (operator decision; AC-9 amended). - Hub-lock errors no longer quote a wait duration. Tests: a grandchild in the fetch's process group and a SIGTERM-cleaned lock file, a real failing fetch, a hydration failure after push, contention found during confirmation (fails until the reducer change that follows), the real undo release (stranded and unverified), pre-existing reuse state, the host key file surviving undo, the pipeline restore through the undo wiring, and daemon reaping. Deadlines in timing tests are 5 s against 6 s delays. Docs: design amendments, coverage and limits restated; CHANGELOG and the --keep-branch guide lines.
… late A reduce applied unseen events on top of the checkpoint. When another agent's event ordered before an event the checkpoint already covered (it was written without it, for example after a transient fetch failure), it was applied out of order: a lock claim lost to a later claim already in the checkpoint, and display ids were allocated in a different order. Clients reducing from different checkpoints disagreed about who held a lock, and a checkpoint written from that state then failed verification against the total-order reconstruction for every reader. When the earliest unseen event orders before the latest covered event, reduce now replays every event after the authority baseline in total order (rebuild_from_authority, sharing the histories already validated). The cost is one full replay, only on a late arrival. A property test checks that a reduce from any per-agent checkpoint cut, with the remaining events arriving late, equals a full replay (three agents, contended locks, releases by non-holders, timestamp ties, inversions and skew; 256 cases), with a deterministic regression for the case found by the lock contention test. Both fail with the rule disabled. The phase-2 per-agent selection test now expects the replay on a late arrival.
verify_content spawned ssh-keygen -Y verify per event and polled it with a 50 ms sleep, so every signature check cost at least 50 ms. A reduce that replays the hub verifies every signed event, and on the shared hub that made a lock confirmation take minutes. The reducer's late-arrival replay will run whenever writers overlap, so this cost would recur. The signed content now goes to ssh-keygen on stdin from the file already written, and the wait uses utils::command_output_with_timeout (30 s bound, process-group termination, drained output). That helper now polls every 1 ms at first, backing off to 20 ms, so a short command returns promptly. Tests cover a prompt return, output larger than a pipe buffer, and the deadline, which verify_content turns into an error, never a success.
kickoff cleanup released nothing when the worktree's daemon did not become ready, then removed the worktree anyway, so its remedy pointed at a directory it had just deleted and the agent identity that could release the lock was gone (#110, field run on this branch's build, 2026-10-09T20:43Z). Kickoff undo had the same order. When a lock release ends in anything but done (failed, stranded, unverified, or readiness not granted), undo and cleanup still stop the worktree daemon, so an orphan cannot hold the hub write lock, but keep the worktree and its branch and say so: run crosslink locks release there, then kickoff cleanup again, or git worktree remove --force to give up. Cleanup's release and removal sit behind the UndoOps seam, so the not-ready path is tested without starting a daemon; CleanupResult gains worktree_kept.
…anges; reorder the umbrella The increment's design records the late-arrival rule as what REQ-6(b) rests on, the signature-wait change, and the kept-worktree amendments to REQ-6 and REQ-8, with the field run on #110 as evidence against the earlier build and the tests that verify the fix. The umbrella builds write cost second and the knowledge cache third (operator decision, 2026-10-09).
- Cleanup decides whether an agent may still hold a lock from the host's view plus the agent's own recent claims, read from its ref in the shared repository (the host's view may not have seen them), and keeps the worktree when that cannot be read. - Undo and cleanup no longer end the worktree session: it is local state discarded with the worktree, and a failed session end printed a remedy for a path that was then removed. - Signature checks use a strict variant of the timeout helper: output that did not arrive in full is an error, not a bad signature. Both pipes share one post-exit deadline. - The reducer logs a late-arrival replay at debug level. Tests: the reduce-equivalence property now generates issue creation (display id allocation) and two successive checkpoint cuts, and prints the plan and cuts on failure (with the proptest regression seed); cleanup keeps the worktree when the held locks cannot be read; the already-held rerun is back in AC-4a; the agent's own claims are read from its ref; a leftover pipe holder does not extend the helper's call. Docs: stale and overstated coverage corrected; older binaries, revert scope, the reducer's costs and display-id renumbering (#824) recorded; the reduce invariant added to the umbrella; CHANGELOG notes cleanup's exit status, the kept worktree, the reducer and the signature wait.
…eanup line The crosslink-guide skill gained a --keep-branch note; regenerate its Codex plugin copy and the generated-assets manifest (scripts/sync-codex-plugin.py).
This branch has not been deployed
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
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.
Summary
First increment of the write-path umbrella (
.design/write-path-cost.md): lock-claim outcome and kickoff rollback (.design/lock-claim-outcome-and-kickoff-rollback.md). Fixes #110 and the timeout half of #118.Unconfirmed.Unconfirmedis not ownership:session workandlocks claimexit non-zero with "claim published but not confirmed", and a rerun confirms without publishing a second claim. An already-held lock is confirmed against the remote, not the local view.runandplanundo what they created when they fail before the agent starts: the container or tmux session, the lock (released and verified under the worktree agent's identity), the daemon, the pipeline state, and the worktree and branch they created. On a reused worktree, what already existed is left as found.kickoff cleanupreleases the agent's locks before removing its worktree, and deletes branches with no commits beyond the recorded base (--keep-branchkeeps them). If a release cannot complete, the worktree and branch are kept (the daemon is still stopped), because the agent identity in the worktree is what can still release the lock. Cleanup now exits non-zero when it leaves work behind.locks stealand auto-steal act on the claim result.Review
The design was reviewed in two Solution Architect rounds. The code was reviewed in two rounds by Software Engineer, Quality Engineer and Solution Architect reviewers (vsdd phase 3, read-only, one pass each with a written stop rule). Round 2: "ready to merge after fixes", no blockers, and all findings fixed in
f83becac. Decisions, amendments made during the build, manual checks and a field observation from another repository's session are recorded in the increment design.Known limits and follow-ups
IssueCreatedarrives late. That is canonical semantics. Hydration of a swap is untested (tracker #824).blocked_corrupt, and a per-agent selector: tracker #822. Stickyblocked_corrupt(blocked_corrupt from a hub write lock holder outlives the holder: the verdict persists after the process and the lock file are gone until the daemon restarts #130): tracker #823.reconcile::migration::tests::production_ready_observer_completes_fallback_pointer_windowfails on macOS ondeveloptoo (a 5 s timing wait).Test plan
cargo fmt --check, and clippy with-D warnings -W clippy::unwrap_used -W clippy::expect_used.7ec14546;f83becac;🤖 Generated with Claude Code