Skip to content

Preserve expected Retry refusals across owner RPC - #3839

Merged
trissim merged 1 commit into
mainfrom
fix/goal-retry-refusal-transport
Nov 1, 2026
Merged

trissim merged 1 commit into
mainfrom
fix/goal-retry-refusal-transport

Conversation

@trissim

@trissim trissim commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

1 commit; 6 files, +68/-13. Four expected attempt-refusal classes now share one typed RPC refusal capability; storage uncertainty remains a separate error. Five generation Resume/Retry refusals and the pending-origin Retry refusal now use GoalRefused instead of plain ValueError.

Paired Toad: OpenHCSDev/toad#1396 at 3f9ffd6e3b82234801d2921020c4c92db43526f1. This closes an open transport/UI end of #3837; #3837's installed wording was not Retry UI acceptance.

The defect

On installed Core71e, ReservedGeneration.authorize_retry raised ValueError. RuntimeServer.handle encoded it with OwnerFailure.of; NoRpcError.raised reconstructed RuntimeError. Toad's GoalSession handled only Refused. The expected refusal therefore bypassed the UI refusal handler. /goal retry also called GoalSession.change directly, outside activate's handler.

This is a source-proven path, not a reproduced physical crash. Original attempt59d334 and all failed/UNKNOWN inputs remain untouched.

Before / after owners

Before:

  • GoalAttemptError(RuntimeError) owned ledger diagnostics, including storage uncertainty, unresolved/stale attempts, reservation conflicts and historical missing authority.
  • Expected generation refusals were untyped ValueErrors.
  • Owner RPC could preserve Refused, but those producers did not supply one.

After:

  • GoalAttemptRefused(GoalRefused, GoalAttemptError) composes the existing refusal wire contract with the existing attempt diagnostic/catch contract.
  • UnresolvedAttemptError, StaleAttemptError, ReservationConflictError and MissingGoalGenerationError inherit it.
  • StorageUncertainError still inherits only GoalAttemptError. An uncertain write is not mislabeled an invalid request.
  • Generation refusals and the pending-origin retry guard use GoalRefused. Messages, transitions, ledger writes, retry authorizations and scheduler busy fences are unchanged.

No RuntimeError catch, textual classifier, fallback or alternate wire format was added. MissingGoalGenerationError's redundant explicit ValueError parent is removed because GoalRefused already derives that membership.

Source evidence

NRA parse_python_module_roots + ModuleSyntaxIndex + class-family index: 22/22 selected Core/installed-Toad modules,329 classes,zero parse omissions. Full declaration/import/write/decision/handler/call projection retained at /home/ts/.cache/agent-scratch/pi-shell-colleague/retry-ui/before-source-map.json. Runtime mutation, foreign subclasses, runtime registration and ambiguous dynamic bindings remain OPEN; this is not a behavior proof.

Interpreter compile and actual class-MRO check passed with installed71e Python, using owned source. GoalAttemptRefused derives both contracts; all four expected members inherit it; StorageUncertainError does not derive GoalRefused. source-proof.json retains paths and hashes. No product store or process was started.

Behavioral verification

Exact pushed pair checked serially under existing TestLease using installed71e Python and read-only existing test dependencies. Module-origin receipts prove archived Coreca1340ed and Toad3f9ffd sources, not the dirty live checkout. Core14passed/2.07s/rc0; Toad12passed/240.22s/rc0; controller0. Logs and selections: /home/ts/.cache/agent-scratch/pi-shell-colleague/retry-ui/paired-run-01/.

Passed authored cases:

  • tests/test_owner_lines.py: expected attempt refusals retain code/message through the actual owner wire decoder; storage uncertainty remains RuntimeError.
  • tests/test_goal_attempts.py::test_retry_refusal_preserves_the_generation_and_owner_rpc_contract: Reserved(unclaimed/claimed),Completed,Cancelled retain both live and reopened ledger snapshots.

The14Core cases also include blocked retry/completion/historical-authority controls. The12Toad cases include button and /goal retry, single refusal notification, original state/draft, stale-view reraising and editor refusal/malformed-reply retention. No product method was patched. These are targeted ledger/wire/Textual checks, not a real saved-native-session acceptance. That actual copied saved-session/native/UI journey remains required; do not replay59d334 or any uncertain input. The controller released TestLease; no competing pytest/native process was launched.

Assumed

  • No installation or live acceptance is claimed.
  • No blanket conversion of storage failures to user refusals is permitted.
  • PR3767's exact head was already inspected for the previous repair; current open PR overlap was checked again. Its import-only generation edit is not modified or rebased here. Commander/Error colleagues confirmed no active editor on these sites.

Copilot AI balanced review requested due to automatic review settings November 1, 2026 17:16

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@trissim
trissim added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit 3489762 Nov 1, 2026
9 of 10 checks passed
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.

2 participants