Repository navigation
Share goal refusal reporting across button and slash controls - #1396
Merged
Merged
Conversation
There was a problem hiding this comment.
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
enabled auto-merge
November 1, 2026 17:37
trissim
disabled auto-merge
November 1, 2026 17:46
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
2 commits; 2 files,+30/-13. One GoalSession refusal handler now covers both button activation and slash/control writes. One copied error envelope is avoided; the old inline handler is moved into the existing session's context manager. The stray debug print in the touched control test is deleted.
Paired Core OpenHCSDev/agent-comms#3839 at
ca1340edcae27fa8ed2010d5903900e58f3c3434makes expected attempt/generation refusals use the existing GoalRefused/Refused wire contract rather than RuntimeError.Before / after
Before: GoalSession.activate caught Refused for button interactions, while
/goal retrycalled GoalSession.change directly and missed that handler. The identical user operation had different refusal behavior depending on its entrance.After: GoalSession.reporting_refusals owns reporting and original-view validation. Both activate and change use it. A nested button->change refusal is reported once by the inner context and does not reach the outer one. Retired-source failures still re-raise rather than showing a notice in a replacement view.
GoalSession.write retains its finally refresh, and does not swallow the refusal. GoalEdit.save therefore retains its own editor error display and draft, instead of closing on a failed save. Generic RuntimeError, malformed replies and storage uncertainty are not caught or relabeled.
No slash-only patch, broad catch, fallback or second state store was added.
Source evidence
NRA source projection before editing:22 selected Core/installed-Toad modules,329 classes,zero parse omissions; declarations, imports,writes,decisions,calls and handlers retained at
/home/ts/.cache/agent-scratch/pi-shell-colleague/retry-ui/before-source-map.json. Owned goal_interaction.py was read and matched the installed structure. Runtime mutation,foreign subclasses/registration and ambiguous dynamic bindings remain OPEN.Compile check passed for both changed Toad files and six paired Core files.
source-proof.jsonretains source hashes and Core's actual MI declaration/MRO check. This proves neither UI behavior nor a physical crash.Verification outstanding
Existing
test_refused_goal_control_preserves_owner_state_and_composeris extended over button clear and/goal retryusing the existing scripted external owner boundary, with exactly one error notification, unchanged owner/projection and composer. No product method is patched. Existing stale-session and editor-malformed-reply tests remain unchanged.Passed at the exact reviewed pair: Core14passed/2.07s/rc0 and Toad12passed/240.22s/rc0, controller0. One serial controller held existing TestLease, installed71e Python reused existing test dependencies read-only. Source origin receipts prove archived Coreca1340ed/Toad3f9ffd, not another editable checkout. Logs/selections:
/home/ts/.cache/agent-scratch/pi-shell-colleague/retry-ui/paired-run-01/. Both real Textual button and/goal retrycatalog cases, once-only notification, source/draft, editor and stale-binding controls passed. No native/provider/frontend or production install launch. Actual copied saved-native-session/UI journey remains outstanding; preserve59d334 and all original/UNKNOWN attempts.Annotation correction after hosted review
The first head incorrectly annotated reporting_refusals with ACP Agent. Real callers supply the declared AgentConnection from agent_binding.py:37. This commit changes only that annotation/import, using the existing owner normally and removing the extra TYPE_CHECKING import. Auto-merge was disabled (rc0) before rework; commander and Errors recorded the old head under_review. My own review-tool attempt was explicitly refused and not bypassed.
NRA ModuleSyntaxIndex proof: all26method bodies are AST-identical to reviewed/tested3f9ffd6e; compile and actual runtime import succeeded. Targeted before/after mypy, with the same existing dependencies and config, reports5errors before and3after (both process rc1): the two newly introduced parameter-domain errors are gone. Remaining three baseline errors are get_goal_history/update_goal missing from AgentConnection's declaration and _modal's old None-only inferred type. This is not a claim of zero type errors. No pytest rerun for an annotation-only correction; earlier14/12 results remain evidence for the preserved runtime bodies. Logs/source hashes: retry-ui/annotation-proof.json and annotation-proof-v2.log. First proof attempt passed a tuple to mypy.api.run, which requires list; it failed before mypy execution and remains retained in annotation-proof.log.
Assumed
Not merged,installed or live-accepted. Existing error owner confirmed no competing goal_interaction/slash edits in1389; no foreign worktree was edited. Native/provider behavior remains unqualified here.