Skip to content

Keep goal policy refusals typed across owner RPC - #3816

Merged
trissim merged 1 commit into
mainfrom
work/pi-toad-errors--7425ce3d67d94062967e2f4208730b62--agent-comms
Nov 1, 2026
Merged

trissim merged 1 commit into
mainfrom
work/pi-toad-errors--7425ce3d67d94062967e2f4208730b62--agent-comms

Conversation

@trissim

@trissim trissim commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Summary

One commit, 5 files, +58/-27. Paired with OpenHCSDev/toad#1389: install the pair together. The current Toad head imports GoalRefused; removing GoalDisplay.action_failure is incompatible with the old Toad caller. Neither half is an independent deployment, and no alias or compatibility reader is retained.

Today's real goal-clear log lost a policy refusal twice:

ProblemGoalOrder.require_replacement -> ValueError
RuntimeServer.handle -> OwnerFailure.of -> NoRpcError -> RuntimeError
Toad request_owner -> ValueError -> GoalSession.write -> another ValueError

The repaired path is:

GoalRefused(Refused, RelationViolationError)
  GoalChangedError(GoalRefused, RejectedEnforcementError)
Goal/order guards -> existing RuntimeServer Refused handler
  -> existing OwnerRpcError -> Refused on the client
Toad preserves that value and catches Refused only at the control boundary

Domain and RPC capabilities share one type

GoalRefused is both a goal relation failure and a transportable refusal. Its constructor owns the existing invalid-parameters code; guards supply only their own reason. Local relation/ValueError consumers retain their actual domain contract through inheritance, not a compatibility alias. RuntimeServer's existing Refused dispatch now carries the code/message/data without a new wire case, schema, classifier or server exception switch.

The problem-order completion, replacement, edit and model-block refusals use it. Goal revision/state compare-and-set refusals also use it. GoalChangedError retains its enforcement behavior and gains the existing RPC refusal capability. Invalid persisted-state and record-validation failures are not blanket-reclassified as user refusals.

Delete the second string error system

Toad #1389 removes both request_owner's RuntimeError/Refused-to-ValueError wrapper and GoalSession.write's action_failure/rethrow wrapper while preserving owner readback in finally. This Core half deletes the now-unused GoalDisplay.action_failure contract and its three implementations (GoalUnavailable, NoGoal, Goal). The Toad model fixture's dead implementation is removed in its paired PR.

Whole Core src/tests/tools plus Toad src/tests search has zero remaining action_failure references after the paired deletion. Its former only live caller was GoalSession.write. No original logs, sessions, failed/UNKNOWN inputs or retained records are modified or replayed.

Evidence and limits

  • NRA source map: 1,876 modules over current owned Core src/tests/tools and Toad src/tests, with declarations, imports, decisions, raises and relevant consumers recorded. Dynamic/generated bindings remain outside the syntactic proof.
  • Targeted pre-deletion AST map of the action_failure family completed with exit 0.
  • Actual Python MRO confirms GoalRefused composes Refused and RelationViolationError. Pickle preserves its type/code/message through the existing CommandFailure mechanism.
  • Three actual policy/CAS guards round-trip through OwnerFailure.refused().wire(), OwnerLine.read() and reply() as Refused with their original code/message. GoalChangedError constructs with the same refusal capability. The paired actual toad.app cold import succeeds against the owned Core source.
  • An initial proof import used a nonexistent goal_revision module and failed before executing checks; corrected proof imports GoalRevision from its actual goals owner. Both results are retained. This is source/codec evidence, not a pytest or installed-UI claim.
  • git diff --check passes; remote head equals local HEAD.
  • No unit pytest/full suite/local NRA gate run. pi-commander owns the necessary combined execution, installation and saved-session UI acceptance. Toad's added real maintenance-barrier/ledger control is on the same PR.
  • Rebasing onto current main retains the separately installed next-tool-boundary channel-delivery repair. The refusal checkpoint is now 890cca9; its remote matches HEAD, and its exact delta remains 5 files +58/-27. The first final parse used a two-tree diff and included five newer main files; that scope was 29, not the paired PR's 24 changed Python files. A current three-dot targeted reproof records the actual patch scope; no parse omission is claimed from the broader run.

Checks for integration

  • tests/test_held_problem_orders.py::test_held_goal_replacement_is_a_preserved_refusal_without_writes: real held Todo, actual clear/set goal guard, unchanged goal/Todo, typed owner-wire round-trip.
  • tests/test_report_goal_evidence.py::test_report_is_not_implementation_order_completion now covers the order's three unconditional policy refusals through that same real wire codec.
  • Existing goal CAS/selection tests and paired Toad native goal/control checks; Toad's stale-edit expectation now names Refused, not the deleted generic wrapper.
  • Still required: real held-goal button refusal is visible without concealing programming/decoding errors; source binding and draft survive; owner state is read back. Toad's queue/project/maintenance journey remains on P434: bind stopped-owner releases to identity and keyed storage #1389 with the same integration owner.

Retained evidence: /home/ts/.cache/agent-scratch/pi-toad-errors/{refusal-before.log,refusal-before.rc,failure-display-before.log,failure-display-before.rc,refusal-owner-proof.log,refusal-owner-proof.rc,refusal-owner-proof-corrected.log,refusal-owner-proof-corrected.rc,core-publish.log}.

Assumed: a refused goal action must satisfy both the existing relation-failure and RPC-refusal contracts; the existing OwnerRpcError wire form is sufficient; both paired API heads are deployed together. No independent install or full channel/CLI/native queue-unification success is claimed.

Copilot AI balanced review requested due to automatic review settings October 31, 2026 21:03

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

Copy link
Copy Markdown
Contributor Author

Exact890cca9 source-qualified for paired integration, not installation acceptance. Commander qualification checkout branch integ-refusal-3816 is the identical commit; Toad combined7bab354b retains #1389 and #1378. Affected Core and Toad checks run serially against both source roots because both repositories have bare conftest imports; zero-execution collection failures are retained, not green proof.

@trissim
trissim merged commit b34f9df into main 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