Skip to content

Record that Procedure holds carry the fault Run holds no longer do - #616

Open
xmap wants to merge 1 commit into
mainfrom
fix/procedure-hold-contention-repro
Open

Record that Procedure holds carry the fault Run holds no longer do#616
xmap wants to merge 1 commit into
mainfrom
fix/procedure-hold-contention-repro

Conversation

@xmap

@xmap xmap commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Why

Cause-scoped claims fixed order-dependent hold contention for Run. Procedure was left with the same three conditions and no note saying so, which is how a known fault becomes a rediscovered one.

  1. ProcedureStatus.HELD is one bit. ProcedureHeld carries a required reason that explains a hold without owning one, so a releaser cannot read it to learn whether the hold is its own.
  2. hold_procedure admits RUNNING only, so a second concern arriving at an already-held Procedure cannot record its intent.
  3. resume_procedure has no notion of who placed the hold; its only cross-concern guard is the hand-patched parent_run_held flag, one bespoke case rather than a general rule.

Two concerns can hold a Procedure, which is what makes the shape a fault: an operator via hold_procedure, and the Conductor via conduct_or_hold, which pauses to Held on a recoverable step failure so the conduct stays resumable.

Severity is lower than for Run, and the tests say why

Rather than leaving the structural similarity to imply otherwise.

append_activities admits RUNNING only, so a conduct cannot quietly carry on past a hold it did not see and never reaches its own pause attempt. Dropping the Conductor's hold silently needs a narrow race, not the wide human-response window that made the Run fault dangerous.

What bites with no race at all is condition 3: an operator resume clears the Conductor's pause outright, because nothing records whose hold it is. Condition 2 holds but fails loudly, with ProcedureCannotHoldError rather than a silent drop.

These are characterization tests

They pass against today's behaviour and document the fault rather than failing on it, so the suite stays green and a future fix shows up as these tests changing.

Why it was untracked

The file did not typecheck: two Procedure constructions passed a raw str where ProcedureName is required. Pyright in the main checkout catches that; a worktree without the file never saw it. Both are wrapped now. The ProcedureRegistered payload keeps its raw str, which is correct for an event payload.

Also removes two rename-leftover directories (set_agent_target_plan, amend_allocation_ceiling) that held nothing but __pycache__ and failed test_no_empty_feature_dirs once anything in the main checkout was committed. Both renamed successors exist.

Verification

  • tests/unit/operation 1,287 passed; tests/architecture 30,313 passed / 621 skipped
  • pyright 0 errors, ruff clean

🤖 Generated with Claude Code

Cause-scoped claims fixed order-dependent hold contention for Run. Procedure
was left with the same three conditions and no note saying so, which is how a
known fault becomes a rediscovered one.

  1. ProcedureStatus.HELD is one bit. ProcedureHeld carries a required reason
     that explains a hold without owning one, so a releaser cannot read it to
     learn whether the hold is its own.
  2. hold_procedure admits RUNNING only, so a second concern arriving at an
     already-held Procedure cannot record its intent.
  3. resume_procedure has no notion of who placed the hold; its only
     cross-concern guard is the hand-patched parent_run_held flag, one bespoke
     case rather than a general rule.

Two concerns can hold a Procedure, which is what makes the shape a fault: an
operator via hold_procedure, and the Conductor via conduct_or_hold, which
pauses to Held on a recoverable step failure so the conduct stays resumable.

SEVERITY IS LOWER THAN FOR RUN, and the tests say why rather than leaving the
structural similarity to imply otherwise. append_activities admits RUNNING
only, so a conduct cannot quietly carry on past a hold it did not see and
never reaches its own pause attempt; dropping the Conductor's hold silently
needs a narrow race, not the wide human-response window that made the Run
fault dangerous. What bites with no race at all is condition 3: an operator
resume clears the Conductor's pause outright, because nothing records whose
hold it is. Condition 2 holds but fails LOUDLY, with ProcedureCannotHoldError
rather than a silent drop.

These are characterization tests. They pass against today's behaviour and
document the fault rather than failing on it, so the suite stays green and a
future fix shows up as these tests changing.

The file had sat untracked because it did not typecheck: two Procedure
constructions passed a raw str where ProcedureName is required, which pyright
in the main checkout catches and a worktree without the file never saw.
Wrapped both; the ProcedureRegistered payload keeps its raw str, which is
correct for an event payload.

Verified: tests/unit/operation 1,287 passed; pyright 0 errors; ruff clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@xmap
xmap enabled auto-merge (squash) August 6, 2026 17:20
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