Skip to content

Commit 729f2d8

Browse files
nutyworksclaude
andauthored
Show acknowledged debugger request failures and timeouts (#90)
* Show debugger request rejection and timeout feedback * Keep debugger feedback visible with occupied toast slots Render request feedback through separate vanilla toast slots above ordinary notifications. Preserve ordinary toast identities and paused lifetimes, as well as request correlation and real-time feedback expiry. Extend the native command-chain regression with five occupied timed toasts and actual draw observation for EN/KO timeout, rejection, expiry, and recovery. * Keep breakpoint feedback across unrelated controls --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent ec9f80c commit 729f2d8

26 files changed

Lines changed: 958 additions & 24 deletions

‎docs/verification/features/breakpoints.md‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,18 @@
44

55
Deleting from the condition modal waits for server acknowledgement before closing
66
back to its existing parent (including Source). Pending, rejected and unavailable
7-
requests remain visible in the same modal. The breakpoint list now only
8-
navigates: its row overflow, toggle, condition, delete and Undo actions are removed.
7+
requests remain visible in the same modal.
8+
9+
Inline toggle rejections additionally appear in a wrapped Codon toast without
10+
hovering the marker; the marker tooltip retains its detail. A missing edit reply
11+
after ten seconds reports that changes are unconfirmed, without changing the
12+
acknowledged definitions. Hidden markers also expire. Only the matching pending
13+
request may produce a rejection, so late/duplicate replies cannot release a retry.
14+
Notices disappear after six seconds. Save/Delete modal errors remain in the modal
15+
without a duplicate toast.
16+
17+
The breakpoint list now only navigates: its row overflow, toggle, condition,
18+
delete and Undo actions are removed.
919
Function entries open their original source line and matching stage; block entries
1020
open an exact matching recorded/static stage in the current pause's Flow. Matching
1121
uses location, stage identity and command fingerprint, never a guessed row index.
@@ -140,6 +150,7 @@ Screen; actual list navigation and focus remain a UI acceptance check.
140150
| --- | --- |
141151
| Definition/condition logic | `coreTest`: `BreakpointRegistryTest`, `BreakpointConditionEvaluatorTest`, `DebuggerEngineTest` |
142152
| Acknowledgement and pending UI state | `clientTest`: `ClientBreakpointStateTest` |
153+
| Non-hover inline rejection feedback (EN/KO), owner denial and unchanged definitions | `DebuggerRequestFeedbackGameTest` |
143154
| Persistence/preview codec | `test`: `WorldBreakpointPersistenceTest`, `BreakpointStagePreviewPayloadTest` |
144155
| Native editor input, modal details without screen replacement, draft discard/cancel, pending/rejected saves, server edits, wrapping and narrow layouts | `DebuggerBreakpointUiGameTest` |
145156
| Single-stage editor/Flow target, legacy toggle/clear and native first-occurrence stop | `SingleStageBreakpointGameTest` |
@@ -154,6 +165,15 @@ Screen; actual list navigation and focus remain a UI acceptance check.
154165
| Numeric line/stage/coordinate list order | `clientTest`: `BreakpointListOrderTest` |
155166

156167
Example: `./gradlew runClientGameTest -PclientGameTest=DebuggerBreakpointUiGameTest`.
168+
`DebuggerRequestFeedbackGameTest` sends inline toggle requests for a deliberately
169+
stale native command-block fingerprint and after owner permission is revoked. Its
170+
EN/KO screenshots show the rejection toast with the pointer away from all markers;
171+
the refused target is never added to acknowledged definitions. The same server
172+
rejection is also rendered above Source and the vanilla command-block editor.
173+
Requests use the inline edit API; these captures do not assert marker click routing.
174+
Modal notice
175+
suppression and hidden-marker expiry use the focused client state tests.
176+
157177
Inspect `*codon-breakpoint-*.png` in the shared screenshot directory. The UI test
158178
checks editing and transport; use the result test or manual trigger path to prove
159179
the execution actually pauses. Record manual world reload separately from the

‎docs/verification/features/stepping.md‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,20 @@ ID when the mailbox executes the request: a delayed control for an earlier stop
4242
must not advance a newer stop, including an execution-complete inspection stop.
4343
Explicit `/codon resume`, `stepinto`, `stepover` and `stepout` commands without an
4444
ID retain their manual/console behavior and target the stop present at execution.
45+
UI controls also send a request ID. A rejection for that exact pending request and
46+
pause appears in a wrapped Codon toast without opening chat. A missing reply two
47+
seconds after sending the control reports that the server outcome is unknown;
48+
waiting for pre-step Watch reads does not start that deadline. It does not mean rollback
49+
or fabricate an execution acknowledgement. Late/duplicate rejections cannot clear
50+
a newer pending request. Feedback uses separate toast slots and renders above ordinary
51+
notifications, so a full, frozen vanilla toast queue cannot block it. Ordinary toast
52+
identities and paused lifetimes remain unchanged. Feedback expires after six seconds.
53+
A new control, pause or authoritative advancement clears control feedback; those
54+
execution events do not acknowledge independent breakpoint edits, whose notices
55+
retain their original deadline. Disconnect clears every notice. These notices are separate from
56+
Watch-read status before a control is sent. Command-tree owner checks remain intact;
57+
denials before command execution and mailbox drops have no correlated reply and
58+
therefore use the same unknown-outcome timeout.
4559
The debugger mailbox admits at most 256 pending client requests and 64 client
4660
control commands per server, with per-connection limits of 32 and 8 respectively.
4761
Admitted queries and controls keep their receive order. Excess work is dropped
@@ -89,6 +103,7 @@ captures a selected row alongside a keyboard-focused, truncated action label.
89103
| Delayed controls and pause ID validation | `coreTest`: `DebuggerControlTest`; command transport: `DebuggerRequestTransportGameTest` |
90104
| Mailbox limits, concurrent admission, request/step order, recovery and shutdown completion | `test`: `DebuggerMailboxTest`, `DebuggerTaskQueueTest` |
91105
| Pending state, Watch read deadline/cancellation, current/history separation | `clientTest`: `ClientQuerySchedulerTest`, `ClientDebuggerStateTest`, `ClientHistoricalCallStackTest`, `DebuggerStatusTest` |
106+
| Visible rejection/timeout with all ordinary toast slots occupied, late replies and recovery | `DebuggerRequestFeedbackGameTest` |
92107
| Native command-chain execution/stage recording | `DebuggerExecutionFlowGameTest` |
93108
| Stop after step/resume in a parked native command context | `DebuggerStopRoutingGameTest` |
94109
| Native conditional function chronology | `DebuggerConditionalFunctionFlowGameTest` |
@@ -104,6 +119,29 @@ Record the before/after command and call depth, not merely that the screen opene
104119
execution-complete stop, then continues. Its native command runs once; the client
105120
receives the completed record with a selected Flow stage and inactive controls.
106121

122+
`DebuggerRequestFeedbackGameTest` triggers a real two-block chain and uses a cached
123+
earlier pause to provoke actual server rejections for all four UI controls. Its
124+
connection-local observer deliberately withholds rejection replies to reproduce
125+
post-send timeout, then delivers the same late reply. EN/KO screenshots show the
126+
wrapped toast and its automatic disappearance without hover.
127+
The fixture first fills all five vanilla slots with real timed SystemToasts. It
128+
observes actual toast draw submissions while the world is paused: timeout and
129+
rejection feedback must render, expire normally, and leave the five ordinary
130+
toast identities and frozen lifetimes intact. The unmodified queued-feedback path
131+
fails after its notice expires without a draw. This does not clear or advance
132+
ordinary notifications in production; fixture cleanup belongs to the test client.
133+
134+
Current-stop Resume recovers and the score of 11 proves each block ran once. The test also observes a
135+
real not-paused rejection after execution ends and resets connection-local state.
136+
Before recovery, a real breakpoint rejection triggers Resume in the same client
137+
frame, before the notice's first draw. The Korean capture after authoritative
138+
advancement must retain that rejection; controls cannot erase unrelated edit
139+
feedback. The state regression covers Step/Resume/Continue, the next pause, a
140+
new control request, six-second expiry and disconnect without extending the clock.
141+
For that deliberately stale client view, vanilla also pauses the integrated
142+
server; the test drains the mailbox on the server thread to receive its rejection.
143+
This is an integrated-server check, not a dedicated-server or real packet-loss test.
144+
107145
`DebuggerStopRoutingGameTest` uses a real outer vanilla Commands execution with
108146
command limit 1 and stops at the terminal after `execute positioned ~ ~ ~ run`.
109147
One mailbox runnable steps/resumes and then submits `stop` before the parked context

‎src/client/java/works/nuty/codon/client/CodonClientMod.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import works.nuty.codon.client.state.ClientPauseEffects;
2323
import works.nuty.codon.client.ui.CodonScreen;
2424
import works.nuty.codon.client.ui.DebuggerOverlay;
25+
import works.nuty.codon.client.ui.DebuggerFeedbackToast;
2526
import works.nuty.codon.client.ui.ScreenLayers;
2627

2728
/**
@@ -75,6 +76,7 @@ public void onInitializeClient() {
7576
input = inputManager;
7677

7778
ClientNetworking.register(state, camera, effects);
79+
DebuggerFeedbackToast.register(state);
7880
sourceState = ClientSourceBrowseNetworking.register();
7981
ClientTickEvents.START_CLIENT_TICK.register(camera::tick);
8082
ClientTickEvents.END_CLIENT_TICK.register(inputManager);

‎src/client/java/works/nuty/codon/client/network/ClientNetworking.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import works.nuty.codon.network.BreakpointDefinitionsSyncPayload;
2020
import works.nuty.codon.network.BreakpointEditPayload;
2121
import works.nuty.codon.network.BreakpointEditResultPayload;
22+
import works.nuty.codon.network.ControlRejectedPayload;
2223
import works.nuty.codon.network.BreakpointStagePreviewRequestPayload;
2324
import works.nuty.codon.network.BreakpointStagePreviewSyncPayload;
2425
import works.nuty.codon.network.ExecutionFlowSyncPayload;
@@ -156,6 +157,9 @@ public static void register(ClientDebuggerState state, DebuggerFreecam freecam,
156157
ClientPlayNetworking.registerGlobalReceiver(BreakpointEditResultPayload.TYPE, (payload, context) ->
157158
context.client().execute(() -> state.breakpoints().finish(payload.requestId(),
158159
ClientBreakpointState.Result.valueOf(payload.status().name()))));
160+
ClientPlayNetworking.registerGlobalReceiver(ControlRejectedPayload.TYPE, (payload, context) ->
161+
context.client().execute(() -> state.rejectControl(payload.pauseId(), payload.requestId(),
162+
"command.codon.error." + payload.reason().name().toLowerCase(java.util.Locale.ROOT))));
159163
ClientPlayNetworking.registerGlobalReceiver(BreakpointStagePreviewSyncPayload.TYPE, (payload, context) ->
160164
context.client().execute(() -> state.stagePreviews().accept(payload.requestId(), payload.location(),
161165
ClientStagePreviewState.Status.valueOf(payload.status().name()), payload.savedCommand(),
@@ -269,16 +273,16 @@ private static boolean requestStagePreview(ClientDebuggerState state,
269273
/** Send only credited reads; a pending step waits for every Watch from its pause. */
270274
public static void sendWatchQueries(Minecraft client, ClientDebuggerState state) {
271275
if (client.player == null) return;
272-
queryScheduler.pump(state, sender(client));
276+
queryScheduler.pump(state, sender(client, state));
273277
}
274278

275279
public static void requestControl(Minecraft client, ClientDebuggerState state, long pauseId,
276280
String command, boolean readBeforeStep) {
277281
if (client.player == null) return;
278-
queryScheduler.requestControl(state, pauseId, command, readBeforeStep, sender(client));
282+
queryScheduler.requestControl(state, pauseId, command, readBeforeStep, sender(client, state));
279283
}
280284

281-
private static ClientQueryScheduler.Sender sender(Minecraft client) {
285+
private static ClientQueryScheduler.Sender sender(Minecraft client, ClientDebuggerState state) {
282286
return new ClientQueryScheduler.Sender() {
283287
@Override public void editor(works.nuty.codon.client.state.ClientWatchEditorState.Query query) {
284288
ClientPlayNetworking.send(new WatchEditorQueryPayload(query.pauseId(), query.requestId(),
@@ -293,7 +297,8 @@ private static ClientQueryScheduler.Sender sender(Minecraft client) {
293297
query.offset(), query.path()));
294298
}
295299
@Override public void control(long pauseId, String command) {
296-
if (client.player != null) client.player.connection.sendCommand("codon " + command + " " + pauseId);
300+
if (client.player != null) client.player.connection.sendCommand("codon " + command + " " + pauseId
301+
+ " " + state.controlRequestId());
297302
}
298303
};
299304
}

‎src/client/java/works/nuty/codon/client/state/ClientBreakpointState.java‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,16 @@
1010
import java.util.List;
1111
import java.util.Map;
1212
import java.util.function.LongSupplier;
13+
import java.util.function.Consumer;
1314

1415
/** Server-confirmed breakpoint definitions, paged snapshots, and per-target edit status. */
1516
public final class ClientBreakpointState {
1617
public enum Action { TOGGLE, SAVE, DELETE }
17-
public enum Result { APPLIED, STALE_SOURCE, INVALID_TARGET, NO_PERMISSION, FAILED }
18+
public enum Result { APPLIED, STALE_SOURCE, INVALID_TARGET, NO_PERMISSION, FAILED, TIMED_OUT }
1819
public record Edit(long requestId, Action action, BreakpointDefinition definition) { }
1920
private static final long TIMEOUT_NANOS = 10_000_000_000L;
2021
private final LongSupplier clock;
22+
private final Consumer<Result> inlineFailure;
2123
private final Map<BreakpointTarget, BreakpointDefinition> definitions = new HashMap<>();
2224
private final Map<BreakpointTarget, Pending> pending = new HashMap<>();
2325
private final Map<BreakpointTarget, Result> errors = new HashMap<>();
@@ -27,7 +29,11 @@ public record Edit(long requestId, Action action, BreakpointDefinition definitio
2729
private long nextRequestId;
2830

2931
public ClientBreakpointState() { this(System::nanoTime); }
30-
public ClientBreakpointState(LongSupplier clock) { this.clock = clock; }
32+
public ClientBreakpointState(LongSupplier clock) { this(clock, result -> { }); }
33+
public ClientBreakpointState(LongSupplier clock, Consumer<Result> inlineFailure) {
34+
this.clock = clock;
35+
this.inlineFailure = inlineFailure;
36+
}
3137

3238
public synchronized boolean acceptPage(long transferId, int offset, boolean last,
3339
List<BreakpointDefinition> page) {
@@ -54,19 +60,23 @@ public synchronized boolean acceptPage(long transferId, int offset, boolean last
5460
if (pending.containsKey(target)) return null;
5561
long id = ++nextRequestId;
5662
if (id <= 0) id = nextRequestId = 1;
57-
pending.put(target, new Pending(id, clock.getAsLong()));
63+
pending.put(target, new Pending(id, clock.getAsLong(), action));
5864
errors.remove(target);
5965
return new Edit(id, action, definition);
6066
}
6167

6268
public synchronized void finish(long requestId, Result result) {
69+
expirePending();
6370
BreakpointTarget target = null;
6471
for (var entry : pending.entrySet()) {
6572
if (entry.getValue().requestId() == requestId) { target = entry.getKey(); break; }
6673
}
6774
if (target == null) return;
68-
pending.remove(target);
69-
if (result != Result.APPLIED) errors.put(target, result);
75+
Pending entry = pending.remove(target);
76+
if (result != Result.APPLIED) {
77+
errors.put(target, result);
78+
if (entry.action() == Action.TOGGLE) inlineFailure.accept(result);
79+
}
7080
}
7181

7282
public synchronized @Nullable BreakpointDefinition get(BreakpointTarget target) {
@@ -96,17 +106,23 @@ private void expire(BreakpointTarget target) {
96106
Pending entry = pending.get(target);
97107
if (entry != null && clock.getAsLong() - entry.startedAt() >= TIMEOUT_NANOS) {
98108
pending.remove(target);
99-
errors.put(target, Result.FAILED);
109+
errors.put(target, Result.TIMED_OUT);
110+
if (entry.action() == Action.TOGGLE) inlineFailure.accept(Result.TIMED_OUT);
100111
}
101112
}
102113

114+
/** Poll even when the marker is hidden; no automatic retry or optimistic edit. */
115+
public synchronized void expirePending() {
116+
for (var target : List.copyOf(pending.keySet())) expire(target);
117+
}
118+
103119
public synchronized void reset() {
104120
definitions.clear();
105121
pending.clear();
106122
errors.clear();
107123
incoming.clear();
108-
incomingTransferId = completedTransferId = nextRequestId = 0;
124+
incomingTransferId = completedTransferId = 0;
109125
}
110126

111-
private record Pending(long requestId, long startedAt) { }
127+
private record Pending(long requestId, long startedAt, Action action) { }
112128
}

‎src/client/java/works/nuty/codon/client/state/ClientDebuggerState.java‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ public final class ClientDebuggerState {
3939
private boolean controlAwaitingReads;
4040
private boolean watchReadsFailed;
4141
private long controlRequestedAt;
42+
private long controlRequestId;
43+
private final ClientRequestFeedback feedback;
4244
private final LongSupplier clock;
4345
private final ClientWatchState watches;
4446
private final ClientWatchEditorState watchEditor;
@@ -67,10 +69,13 @@ public ClientDebuggerState(DebuggerPreferences preferences) {
6769
public ClientDebuggerState(LongSupplier clock, DebuggerPreferences preferences) {
6870
this.clock = Objects.requireNonNull(clock);
6971
this.preferences = Objects.requireNonNull(preferences);
72+
this.feedback = new ClientRequestFeedback(clock);
7073
this.watches = new ClientWatchState(clock);
7174
this.watchEditor = new ClientWatchEditorState(clock);
7275
this.nbt = new ClientNbtState(clock);
73-
this.breakpoints = new ClientBreakpointState(clock);
76+
this.breakpoints = new ClientBreakpointState(clock, result -> feedback.show(ClientRequestFeedback.Kind.BREAKPOINT,
77+
"codon.ui.breakpoint_rejected",
78+
"codon.breakpoint.error." + result.name().toLowerCase(java.util.Locale.ROOT)));
7479
this.stagePreviews = new ClientStagePreviewState();
7580
this.nbt.setEnabled(preferences.nbtExpanded());
7681
this.nbt.setEnabledListener(preferences::setNbtExpanded);
@@ -85,6 +90,7 @@ public GizmoMode next() {
8590
}
8691

8792
public void applyPause(PauseSnapshot snapshot) {
93+
feedback.clearControl();
8894
boolean repeatedPause = paused && this.snapshot != null && this.snapshot.pauseId() == snapshot.pauseId();
8995
boolean interruptedReads = repeatedPause && (controlAwaitingReads || watchReadsFailed);
9096
unobservedSelection = null;
@@ -144,6 +150,7 @@ public void applyResume() {
144150
}
145151

146152
private void clearPause(boolean stepping) {
153+
feedback.clearControl();
147154
unobservedSelection = null;
148155
if (stepping) watchEditor.invalidate();
149156
else watchEditor.cancel();
@@ -748,7 +755,9 @@ public DebuggerPreferences preferences() {
748755
/** Only a fresh server packet completes a control request; UI never fabricates a pause. */
749756
public boolean beginControlRequest() {
750757
if (!paused || snapshot == null || controlPending()) return false;
758+
feedback.clearControl();
751759
controlPending = true;
760+
controlRequestId = controlRequestId == Long.MAX_VALUE ? 1 : controlRequestId + 1;
752761
controlRequestedAt = clock.getAsLong();
753762
return true;
754763
}
@@ -773,17 +782,30 @@ public void controlSent() {
773782
}
774783

775784
public boolean controlPending() {
776-
if (controlPending && !controlAwaitingReads && clock.getAsLong() - controlRequestedAt >= 2_000_000_000L)
785+
if (controlPending && !controlAwaitingReads && clock.getAsLong() - controlRequestedAt >= 2_000_000_000L) {
777786
controlPending = false;
787+
feedback.show(ClientRequestFeedback.Kind.CONTROL, "codon.ui.control_timed_out", "codon.ui.control_timeout_detail");
788+
}
778789
return controlPending;
779790
}
780791

792+
public long controlRequestId() { return controlRequestId; }
793+
public ClientRequestFeedback feedback() { return feedback; }
794+
795+
/** Ignore expired/replaced requests, even if another attempt targets the same pause. */
796+
public void rejectControl(long pauseId, long requestId, String messageKey) {
797+
if (!controlPending() || snapshot == null || snapshot.pauseId() != pauseId || controlRequestId != requestId) return;
798+
controlPending = false;
799+
feedback.show(ClientRequestFeedback.Kind.CONTROL, "codon.ui.control_rejected", messageKey);
800+
}
801+
781802
/** Delay only the waiting label; controlPending() still disables actions immediately. */
782803
public boolean displayControlWaiting() {
783804
return controlPending() && clock.getAsLong() - controlRequestedAt >= PendingDisplay.GRACE_NANOS;
784805
}
785806

786807
public void reset() {
808+
feedback.clear();
787809
applyResume();
788810
recentFlowSnapshot = null;
789811
displayedCallStack = List.of();

0 commit comments

Comments
 (0)