Skip to content

Commit 89f3617

Browse files
RtlZeroMemoryclaude
andcommitted
fix: address PR #417 review feedback
- core: printAbove/setInlineRows gain lifecycle-idle and in-commit/ in-render re-entrancy guards (mirroring update()) - node: setInlineRows derives the runtime baseline from the START-resolved config (width policy settles during start), stripping Node-transport keys (fpsCap/maxEventBytes/frameTransport) so engine_set_config strict validation cannot reject the resend; commitScrollback validates rows client-side on both paths - tests: guaranteed backend teardown via try/finally in all bodies - example: inlineRows updates commit only after setInlineRows resolves Re-verified in tmux: 4 printAbove checkpoints, live +/- resize, no stderr. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 2668a07 commit 89f3617

6 files changed

Lines changed: 106 additions & 57 deletions

File tree

examples/inline-status/src/index.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,22 @@ app.keys({
7777
"ctrl+c": () => void app.stop(),
7878
/* Grow/shrink the live region at runtime; layout follows the resize. */
7979
"shift+=": () => {
80-
inlineRows = Math.min(16, inlineRows + 1);
81-
void app.setInlineRows(inlineRows).catch(() => {});
80+
const nextRows = Math.min(16, inlineRows + 1);
81+
void app.setInlineRows(nextRows).then(
82+
() => {
83+
inlineRows = nextRows;
84+
},
85+
() => {},
86+
);
8287
},
8388
"-": () => {
84-
inlineRows = Math.max(5, inlineRows - 1);
85-
void app.setInlineRows(inlineRows).catch(() => {});
89+
const nextRows = Math.max(5, inlineRows - 1);
90+
void app.setInlineRows(nextRows).then(
91+
() => {
92+
inlineRows = nextRows;
93+
},
94+
() => {},
95+
);
8696
},
8797
});
8898

packages/core/src/app/createApp.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,14 @@ export function createApp<S>(opts: CreateAppStateOptions<S> | CreateAppRoutesOnl
10181018

10191019
async printAbove(view: VNode, printOpts?: Readonly<{ rows?: number }>): Promise<void> {
10201020
guards.assertOperational("printAbove");
1021+
guards.assertLifecycleIdle("printAbove");
1022+
if (inCommit) guards.throwCode("ZRUI_REENTRANT_CALL", "printAbove: called during commit");
1023+
if (inRender) {
1024+
guards.throwCode(
1025+
"ZRUI_UPDATE_DURING_RENDER",
1026+
guards.updateDuringRenderDetail("printAbove"),
1027+
);
1028+
}
10211029
sm.assertOneOf(["Running"], "printAbove: app must be running");
10221030
const commitFn = backend.commitScrollback?.bind(backend);
10231031
if (commitFn === undefined) {
@@ -1038,6 +1046,14 @@ export function createApp<S>(opts: CreateAppStateOptions<S> | CreateAppRoutesOnl
10381046

10391047
async setInlineRows(rows: number): Promise<void> {
10401048
guards.assertOperational("setInlineRows");
1049+
guards.assertLifecycleIdle("setInlineRows");
1050+
if (inCommit) guards.throwCode("ZRUI_REENTRANT_CALL", "setInlineRows: called during commit");
1051+
if (inRender) {
1052+
guards.throwCode(
1053+
"ZRUI_UPDATE_DURING_RENDER",
1054+
guards.updateDuringRenderDetail("setInlineRows"),
1055+
);
1056+
}
10411057
sm.assertOneOf(["Running"], "setInlineRows: app must be running");
10421058
const setFn = backend.setInlineRows?.bind(backend);
10431059
if (setFn === undefined) {

packages/node/src/__tests__/scrollback_commit.test.ts

Lines changed: 53 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,13 @@ for (const executionMode of ["worker", "inline"] as const) {
3737
nativeShimModule: SHIM,
3838
});
3939
await backend.start();
40-
assert.ok(backend.commitScrollback !== undefined);
41-
await backend.commitScrollback(makeClearDrawlist(), 2);
42-
await backend.stop();
43-
backend.dispose();
40+
try {
41+
assert.ok(backend.commitScrollback !== undefined);
42+
await backend.commitScrollback(makeClearDrawlist(), 2);
43+
} finally {
44+
await backend.stop();
45+
backend.dispose();
46+
}
4447
});
4548

4649
test(`scrollback: ${executionMode} path surfaces engine rejection`, async () => {
@@ -49,15 +52,18 @@ for (const executionMode of ["worker", "inline"] as const) {
4952
nativeShimModule: SHIM,
5053
});
5154
await backend.start();
52-
await assert.rejects(
53-
backend.commitScrollback?.(makeClearDrawlist(), 99) ?? Promise.reject(new Error("missing")),
54-
(err: unknown) =>
55-
err instanceof ZrUiError &&
56-
err.code === "ZRUI_BACKEND_ERROR" &&
57-
err.message.includes("engine_commit_scrollback failed"),
58-
);
59-
await backend.stop();
60-
backend.dispose();
55+
try {
56+
await assert.rejects(
57+
backend.commitScrollback?.(makeClearDrawlist(), 99) ?? Promise.reject(new Error("missing")),
58+
(err: unknown) =>
59+
err instanceof ZrUiError &&
60+
err.code === "ZRUI_BACKEND_ERROR" &&
61+
err.message.includes("engine_commit_scrollback failed"),
62+
);
63+
} finally {
64+
await backend.stop();
65+
backend.dispose();
66+
}
6167
});
6268

6369
test(`scrollback: ${executionMode} path accepts runtime inline rows`, async () => {
@@ -66,13 +72,16 @@ for (const executionMode of ["worker", "inline"] as const) {
6672
nativeShimModule: SHIM,
6773
});
6874
await backend.start();
69-
assert.ok(backend.setInlineRows !== undefined);
70-
/* The strict shim only accepts inlineRows=5 with plat.screenMode=1, so a
71-
resolving call proves the runtime payload reached the engine intact. */
72-
await backend.setInlineRows(5);
73-
await backend.commitScrollback?.(makeClearDrawlist(), 1);
74-
await backend.stop();
75-
backend.dispose();
75+
try {
76+
assert.ok(backend.setInlineRows !== undefined);
77+
/* The strict shim only accepts inlineRows=5 with plat.screenMode=1, so a
78+
resolving call proves the runtime payload reached the engine intact. */
79+
await backend.setInlineRows(5);
80+
await backend.commitScrollback?.(makeClearDrawlist(), 1);
81+
} finally {
82+
await backend.stop();
83+
backend.dispose();
84+
}
7685
});
7786

7887
test(`scrollback: ${executionMode} path validates inline rows client-side`, async () => {
@@ -81,12 +90,15 @@ for (const executionMode of ["worker", "inline"] as const) {
8190
nativeShimModule: SHIM,
8291
});
8392
await backend.start();
84-
await assert.rejects(
85-
backend.setInlineRows?.(0) ?? Promise.reject(new Error("missing")),
86-
(err: unknown) => err instanceof ZrUiError && err.code === "ZRUI_INVALID_PROPS",
87-
);
88-
await backend.stop();
89-
backend.dispose();
93+
try {
94+
await assert.rejects(
95+
backend.setInlineRows?.(0) ?? Promise.reject(new Error("missing")),
96+
(err: unknown) => err instanceof ZrUiError && err.code === "ZRUI_INVALID_PROPS",
97+
);
98+
} finally {
99+
await backend.stop();
100+
backend.dispose();
101+
}
90102
});
91103
}
92104

@@ -96,16 +108,19 @@ test("scrollback: alt-mode backend rejects commit and rows APIs", async () => {
96108
nativeShimModule: SHIM,
97109
});
98110
await backend.start();
99-
await assert.rejects(
100-
backend.commitScrollback?.(makeClearDrawlist(), 1) ?? Promise.reject(new Error("missing")),
101-
(err: unknown) =>
102-
err instanceof ZrUiError && err.message.includes('requires screen.mode "inline"'),
103-
);
104-
await assert.rejects(
105-
backend.setInlineRows?.(5) ?? Promise.reject(new Error("missing")),
106-
(err: unknown) =>
107-
err instanceof ZrUiError && err.message.includes('requires screen.mode "inline"'),
108-
);
109-
await backend.stop();
110-
backend.dispose();
111+
try {
112+
await assert.rejects(
113+
backend.commitScrollback?.(makeClearDrawlist(), 1) ?? Promise.reject(new Error("missing")),
114+
(err: unknown) =>
115+
err instanceof ZrUiError && err.message.includes('requires screen.mode "inline"'),
116+
);
117+
await assert.rejects(
118+
backend.setInlineRows?.(5) ?? Promise.reject(new Error("missing")),
119+
(err: unknown) =>
120+
err instanceof ZrUiError && err.message.includes('requires screen.mode "inline"'),
121+
);
122+
} finally {
123+
await backend.stop();
124+
backend.dispose();
125+
}
111126
});

packages/node/src/backend/backendSharedConfig.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,10 @@ const RUNTIME_STRIPPED_KEYS: readonly string[] = [
127127
"requested_drawlist_version",
128128
"requestedEventBatchVersion",
129129
"requested_event_batch_version",
130+
/* Node-transport keys (worker init only; unknown to engine_set_config). */
131+
"fpsCap",
132+
"maxEventBytes",
133+
"frameTransport",
130134
];
131135

132136
/**
@@ -160,12 +164,12 @@ export function isInlineScreenNativeConfig(
160164
return mode === NATIVE_SCREEN_MODE_INLINE;
161165
}
162166

163-
/** Validate a runtime inline viewport height request. */
164-
export function validateInlineRowsOrThrow(rows: number): void {
167+
/** Validate a rows argument for inline screen-mode APIs. */
168+
export function validateInlineRowsOrThrow(rows: number, label = "rows"): void {
165169
if (!Number.isInteger(rows) || rows < 1 || rows > INLINE_ROWS_MAX) {
166170
throw new ZrUiError(
167171
"ZRUI_INVALID_PROPS",
168-
`setInlineRows: rows must be an integer in [1, ${String(INLINE_ROWS_MAX)}]`,
172+
`${label} must be an integer in [1, ${String(INLINE_ROWS_MAX)}]`,
169173
);
170174
}
171175
}

packages/node/src/backend/nodeBackend.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,10 +204,6 @@ export function createNodeBackendInternal(opts: NodeBackendInternalOpts = {}): N
204204
};
205205
let initConfigResolved: EngineCreateConfig | null = null;
206206
const isInlineScreen = isInlineScreenNativeConfig(nativeConfig);
207-
const runtimeConfigBase = deriveRuntimeConfigBase({
208-
...nativeConfig,
209-
targetFps: nativeTargetFps,
210-
});
211207

212208
let worker: Worker | null = null;
213209
let disposed = false;
@@ -888,6 +884,7 @@ export function createNodeBackendInternal(opts: NodeBackendInternalOpts = {}): N
888884
if (!isInlineScreen) {
889885
throw new ZrUiError("ZRUI_BACKEND_ERROR", 'commitScrollback requires screen.mode "inline"');
890886
}
887+
validateInlineRowsOrThrow(rows, "commitScrollback: rows");
891888
const buf = new ArrayBuffer(drawlist.byteLength);
892889
copyInto(buf, drawlist);
893890
const d = deferred<void>();
@@ -904,8 +901,13 @@ export function createNodeBackendInternal(opts: NodeBackendInternalOpts = {}): N
904901
if (!isInlineScreen) {
905902
throw new ZrUiError("ZRUI_BACKEND_ERROR", 'setInlineRows requires screen.mode "inline"');
906903
}
907-
validateInlineRowsOrThrow(rows);
908-
send({ type: "setConfig", config: { ...runtimeConfigBase, inlineRows: rows } });
904+
validateInlineRowsOrThrow(rows, "setInlineRows: rows");
905+
if (initConfigResolved === null) {
906+
throw new Error("NodeBackend: not started");
907+
}
908+
/* Resend the resolved create surface (width policy settles at start). */
909+
const runtimeBase = deriveRuntimeConfigBase(initConfigResolved);
910+
send({ type: "setConfig", config: { ...runtimeBase, inlineRows: rows } });
909911
},
910912

911913
async getCaps(): Promise<TerminalCaps> {

packages/node/src/backend/nodeBackendInline.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -239,10 +239,6 @@ export function createNodeBackendInlineInternal(opts: NodeBackendInternalOpts =
239239
);
240240
const nativeTargetFps = resolveTargetFps(fpsCap, nativeConfig);
241241
const isInlineScreen = isInlineScreenNativeConfig(nativeConfig);
242-
const runtimeConfigBase = deriveRuntimeConfigBase({
243-
...nativeConfig,
244-
targetFps: nativeTargetFps,
245-
});
246242

247243
const initConfigBase = {
248244
...nativeConfig,
@@ -793,6 +789,7 @@ export function createNodeBackendInlineInternal(opts: NodeBackendInternalOpts =
793789
if (!isInlineScreen) {
794790
throw new ZrUiError("ZRUI_BACKEND_ERROR", 'commitScrollback requires screen.mode "inline"');
795791
}
792+
validateInlineRowsOrThrow(rows, "commitScrollback: rows");
796793
const commit = native.engineCommitScrollback;
797794
if (commit === undefined) {
798795
throw new ZrUiError(
@@ -818,8 +815,13 @@ export function createNodeBackendInlineInternal(opts: NodeBackendInternalOpts =
818815
if (!isInlineScreen) {
819816
throw new ZrUiError("ZRUI_BACKEND_ERROR", 'setInlineRows requires screen.mode "inline"');
820817
}
821-
validateInlineRowsOrThrow(rows);
822-
const rc = native.engineSetConfig(engineId, { ...runtimeConfigBase, inlineRows: rows });
818+
validateInlineRowsOrThrow(rows, "setInlineRows: rows");
819+
if (initConfigResolved === null) {
820+
throw new Error("NodeBackend(inline): not started");
821+
}
822+
/* Resend the resolved create surface (width policy settles at start). */
823+
const runtimeBase = deriveRuntimeConfigBase(initConfigResolved);
824+
const rc = native.engineSetConfig(engineId, { ...runtimeBase, inlineRows: rows });
823825
if (rc < 0) {
824826
throw new ZrUiError("ZRUI_BACKEND_ERROR", `engine_set_config failed: code=${String(rc)}`);
825827
}

0 commit comments

Comments
 (0)