Skip to content

Make declared failures own their reasons - #2674

Closed
trissim wants to merge 4 commits into
next/hand-precedencefrom
next/failure-reason
Closed

trissim wants to merge 4 commits into
next/hand-precedencefrom
next/failure-reason

Conversation

@trissim

@trissim trissim commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

A terminal already owns a TurnFailure, but Done flattened it through asdict and diagnostics decoded a second enum from the resulting dictionary. That enum was copied into class code attributes, a separate preflight flag, JSON diagnostics and failed-goal rows. Delete the round trip, the enum and all production mappings. Source grep: 27 matching lines before, zero after at f5b83f5.

Former readers now ask these owners:

  • TurnFailure instances carry their own identity; all code constants mapping classes to enum members are gone. Labels derive from declared_name.
  • Backend and watchdog record NativePreflightExit/NativePreflightTimeout instances, both InputIdUnavailable subclasses, instead of maintaining a second preflight_failure flag. NativeLaunchInvalid and generic BackendFailed cover their existing unclassified outcomes. New preflight declarations remain above identity/authority failures in the declaration order inherited from Derive family precedence from declaration order #2662.
  • Done carries the selected failure object. Its failure_kind is that object's class; reason_code is only a derived label. FailedGoalTurn supplies that class to failed-goal observation and uses its declared_name for the attempt's resolution. EndedWithoutResult names BackendFailed directly. No asdict/enum decoding remains.
  • FailedTurnEvidence stores failure: type[TurnFailure], replacing reason. Existing FamilyClassStorage and FieldCodec own SQL representation and decoding. AttemptFailedExecution reads evidence.failure.declared_name for display.
  • Diagnostic publication takes the declared class and measurements directly, writing failure instead of reason. TurnProgress supplies the terminal's class. The existing generic selected-result publisher supplies BackendFailed instead of an empty event dictionary. The unused NativeTerminalFailureRecord and its unused enum reader are deleted.

Stored data has a one-time Carry, tools/cutover/carry_declared_failures.py. The installer's existing CarryChain discovers it. Its legacy labels live only in this tool, mapped to actual failure classes; no runtime decoder or alias is retained. It visits the current root and retained roots from CarryCatalogSource, backs up the declared goal-attempt/diagnostic files, prepares SQL changes in memory, rebuilds FailedTurnEvidence through its existing TypedTable.rebuild, changes reason to failure, updates old terminal labels in failed AttemptRecord resolutions, and rewrites terminal diagnostic JSON's reason key. Writes require the installer's held root; already-carried values produce no further rewrite, and unknown old labels are refused. The tool has NOT run against retained data. Delete it once all retained roots have crossed.

OS decisions also move to existing Platform. is_owned owns effective-UID policy; is_private derives from it. make_private_descriptor changes the already-open descriptor, never a reopened pathname. Route reads, shared guards and exclusive publication use these methods, retaining their original locks, FD-relative operations and inode comparisons. LinuxPlatform owns proc-fd socket addresses; the other platform implementation owns the existing temporary-directory-link lifetime. PrivateSocketRole holds and closes its original FD. No UID/mode/fchmod decisions remain in active_route.py, and private_path.py no longer decides sys.platform. The branch adds no new platform family or helper module.

This PR is stacked on next/hand-precedence, head b21a182, as requested. #2662 is still OPEN at the last check; rebase this branch onto main and change its PR base after #2662 lands. OS source changes include merged #2643. No local tests, scans or censuses ran; the integrator owns the combined run. Existing tests were adapted to their changed inputs, without executing them. Integration should exercise preflight feedback, agent-event terminal propagation, failed-goal observation/schema and diagnostic publication, then the actual carry on old-label SQL/JSON, its second-run no-change behavior, and pinned route/socket operations.

Assumed: class names are the canonical reason labels, so old public reason spellings intentionally change rather than surviving as aliases; generic missing outcomes remain BackendFailed. The installer holds the current root and all retained snapshots are inactive before this Carry writes them. The existing Carry backup and restore window protects the files this step declares. Windows ACL policy remains the existing Platform policy; pinned route mechanics remain POSIX. No runtime or migration success is claimed from the source grep.

@trissim
trissim marked this pull request as ready for review October 23, 2026 08:14
Copilot AI balanced review requested due to automatic review settings October 23, 2026 08:14

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 reached their quota limit.

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 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Landed on main through batch #2684 (merge f966a53): head f5b83f5 is an ancestor of origin/main. GitHub can't mark it merged because its base is a stacked branch, and it can't be retargeted because main already contains every commit.

@trissim trissim closed this Oct 23, 2026
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