Skip to content

Fix generation-owned Retry refusal diagnostics - #3837

Merged
trissim merged 1 commit into
mainfrom
fix/goal-retry-state-refusals
Nov 1, 2026
Merged

trissim merged 1 commit into
mainfrom
fix/goal-retry-state-refusals

Conversation

@trissim

@trissim trissim commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Summary

1 commit, 1 production file, +21/-1. Three generation states stop inheriting one false unresolved-attempt diagnosis; all five existing concrete lifecycle members now own Retry behavior. 33 existing concrete methods, all class headers/fields and every SQL/input/decision fence are unchanged. No state migration, provider launch or input replay.

Commander52628 assigned this independent diagnostic repair after Carry52625 observed the generic message for a reserved accepted attempt. Original attempt 59d334 is not executed, altered or retried.

Before / after

Before:

GenerationState.authorize_retry -> "attempt unresolved; inspect before retrying"
  ReadyGeneration -> existing ready recovery
  BlockedGeneration -> existing exact blocked-attempt retry
  ReservedGeneration, CompletedGeneration, CancelledGeneration -> inherited false diagnosis

After:

GenerationState.authorize_retry -> abstract lifecycle contract
  ReadyGeneration, BlockedGeneration -> unchanged authorization implementations
  ReservedGeneration -> names reserved attempt; Retry cannot replace it
  CompletedGeneration -> completed work cannot be restarted by Retry
  CancelledGeneration -> cancelled work cannot be restarted by Retry

A reserved generation alone does not prove its attempt is running. Reservation and claim share ReservedGeneration; their distinction belongs to GoalAttemptPhase. The new refusal says only what the generation proves. No extra phase read, boolean, discriminator switch, ledger or policy roster is introduced.

The one deleted line is the misleading inherited refusal. The family already supplies the identity and distinct contracts: IMPL-4 (half-finished family), fact: lifecycle-specific Retry result; at: old goal_generation.py:90-93; derive: subclass; by: existing GenerationState members. Paper1 Theorem4.8 forbids merging different required answers merely because all three currently refuse. This patch makes the existing abstraction do the work rather than classifying its members again.

Preserved owners and consumers

  • GoalAttemptStore.authorize_current_retry:854-857 reads the current generation and dispatches once to its lifecycle.
  • GenerationState's Ready and Blocked methods retain their exact arguments and calls. Ready's current generation/grant rotation and Blocked's goal/generation/attempt/phase/decision fences remain in GoalAttemptStore, untouched.
  • Every new refusal immediately raises the existing ValueError type. No store method is called. Reserved's named identity is derived from the existing Generation.attempt_identity.
  • TransitionGoalAction.change:210-222 calls require_transition before changing/publishing the goal; RetryGoalAction.require_transition:728-731 remains unchanged.
  • Real entrypoints: CommsGoalTool.apply (tools.py:527-542, status=retry), and Toad /goal retry (RetryControl, slash_command.py:181) through RetryGoalAction/RetryGoalRuntimeRequest/GoalScheduler.retry_goal. No frontend or tool-specific message copy is added.

Existing work / overlap

Fresh main/base: 6e3a2a3a1e4eb3dbcc2911775ea81549cec30abb. Reused my finished isolated checkout, preserving all untracked shell/review evidence.

Open #3767 exact e6d5448edda7f436a85f3ac0394f651986ed5948, branch work/unit-stopgrant--P859-field-types: the complete goal_generation.py delta is only the TYPE_CHECKING GoalExecutionSource import. That hunk is untouched here. Old unit-stopgrant is absent from the current active roster; salvage52644 confirms it owns #3820, not #3767, and has no committed/uncommitted retry-method overlap. No foreign branch or worktree edited.

Source proof, not runtime acceptance

Existing NRA parser / ModuleSyntaxIndex / class-family projection, serial with cache disabled: 37 modules, 464 original class declarations, 460 canonical projections, 13,203 AST events, zero parse omissions; rc0. Four function-local test classes remain unprojected OPEN: Responses (goal_generation_installed_journey.py:55), PausedReader (test_coordination_store.py:1156), Client (test_stack_goal_cancel_resume.py:43), SpecializedPausedGoal (test_turn_goal_account.py:20). Dynamic/foreign/rebinding/runtime registration and behavior equivalence remain OPEN. A first projection used the removed ParsedModule.syntax_index convenience and failed; its traceback/rc are retained, and the successful projection uses ModuleSyntaxIndex.build.

Source-delta certificate reuses NRA parsing and compares the AST: only the base contract changes; exactly three ValueError-only methods are added; 33 old concrete methods, all class headers and fields are identical; the only new import is abc.abstractmethod. All five concrete state declarations supply authorize_retry. Source compile passes; git diff --check has no output. This is source proof, not executed retry behavior.

Commands: runtime Python -I -B on the two one-shot NRA projection/source-delta scripts under timeout -s INT; final rc0 for both. Scripts removed after successful completion. Evidence owner/path: pi-shell-colleague, /home/ts/.cache/agent-scratch/pi-shell-colleague/retry-diagnostics/ (before-map.json, source-proof.json, logs/rc and exact overlap metadata). Retain through integration acceptance. No tests, local NRA gate, environment install, native/model/frontend launch, real retry or original/UNKNOWN replay was run.

Integration proof owed to pi-commander

Existing affected tests to combine as necessary:

  • tests/test_goal_resume.py::test_a_blocked_goal_is_retried_by_its_unit_or_commander_and_authorize_retry_guards_each (tool/runtime positives plus reserved refusal).
  • tests/test_goal_attempts.py::test_failure_blocks_resume_until_explicit_separate_retry_decision and ::test_verified_completion_is_terminal_without_a_new_ready_grant (unchanged ledger fences).
  • tests/test_goal_attempt_lifecycle.py::test_current_saved_failure_and_history_survive_retry_and_retirement (history, retry and cancellation).

Those existing tests do not independently assert every new refusal's meaning. The integration owner still owes the Reserved (both unclaimed and claimed), Completed and Cancelled diagnostic/no-write controls on fresh isolated records, plus Ready/Blocked positives. The affected installed tool/UI Retry entrypoint remains unverified here. Never use original59d334 as a control or infer a running attempt from reservation alone.

Assumed

  • No external dynamic GenerationState subclass is admitted by this repair; the complete source family has five concrete members, all now implement the contract. Dynamic extensions are explicitly OPEN.
  • Diagnostic-only source correction qualifies for the recoverable hotfix authorized by52628; no lifecycle transition or stored format changes.
  • Commander owns serial integration and installation. Merged is not installed or seen working.

goal_generation.py:93: every refusal used to be an unresolved failure
even the goal that had already completed
the error message was the only thing still doing unfinished work

Copilot AI balanced review requested due to automatic review settings November 1, 2026 14:53

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 merged commit 71eae09 into main Nov 1, 2026
2 of 4 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