Skip to content

Commit b695224

Browse files
committed
fix(daemon): preserve the write error's structured details in the close-time save failure
Review feedback on #1392 (thymikee): toOrdinaryCloseSaveScriptFailure rebuilt the AppError from only the original message, dropping its machine-readable details.reason ("script_target_exists"), details.path, and cause. A caller dispatching on those fields (or reading the CLI's --json error.details) lost them even though the underlying write failure carried them. Preserve the original error's details/cause, overriding only the close-specific hint and retriable:false. Extends the #1391 regression test to assert the routed close response still carries reason/path.
1 parent 37b9938 commit b695224

2 files changed

Lines changed: 25 additions & 8 deletions

File tree

src/daemon/handlers/__tests__/session-device-claims.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,11 @@ test('#1391: a close-time script save failure still clears the advisory claim an
299299
assert.equal(error.code, 'COMMAND_FAILED');
300300
assert.match(error.message, /session was closed, but its script was not saved/);
301301
assert.equal(error.details?.retriable, false);
302+
// The original write failure's machine-readable details survive the
303+
// close-specific hint/retriable override, so a caller dispatching on them
304+
// (e.g. `reason === 'script_target_exists'`) still can.
305+
assert.equal(error.details?.reason, 'script_target_exists');
306+
assert.equal(error.details?.path, targetPath);
302307
assert.equal(store.get('close-save-script-failure'), undefined);
303308
assert.deepEqual(inspectDeviceClaims({ serial: android.id }), []);
304309
assert.equal(fs.readFileSync(targetPath, 'utf8'), 'pre-existing\n');

src/daemon/handlers/session-close.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -289,21 +289,33 @@ function finalizeOrdinaryCloseScript(params: {
289289
* here (a non-clobber refusal or another fs AppError — see
290290
* `handleSessionScriptWriteFailure`'s non-repair, non-active-publication
291291
* branch); anything else is already swallowed into a silent `{written:false}`
292-
* there. The message is corrected for THIS call site: the shared
293-
* `publishHealedScriptAtomically` wording ("retry close --save-script")
292+
* there. Only the message/hint/retriable are corrected for THIS call site (the
293+
* shared `publishHealedScriptAtomically` wording, "retry close --save-script",
294294
* describes the repair-commit retry contract, which does not apply here — by
295295
* the time the agent sees this, `close` has already released the device and
296-
* deleted the session, so there is nothing left to retry in place.
296+
* deleted the session, so there is nothing left to retry in place); the
297+
* original error's machine-readable `details` (e.g. `reason:
298+
* "script_target_exists"`, `path`) are preserved so a caller dispatching on
299+
* them still can.
297300
*/
298301
function toOrdinaryCloseSaveScriptFailure(error: unknown): AppError {
299-
const detail = error instanceof AppError ? error.message : normalizeError(error).message;
302+
const overrides = {
303+
hint: 'Remove the existing target (or pass --force/--overwrite), then re-record with open --save-script.',
304+
retriable: false,
305+
};
306+
if (error instanceof AppError) {
307+
return new AppError(
308+
'COMMAND_FAILED',
309+
`The session was closed, but its script was not saved: ${error.message}`,
310+
{ ...error.details, ...overrides },
311+
error.cause,
312+
);
313+
}
314+
const detail = normalizeError(error).message;
300315
return new AppError(
301316
'COMMAND_FAILED',
302317
`The session was closed, but its script was not saved: ${detail}`,
303-
{
304-
hint: 'Remove the existing target (or pass --force/--overwrite), then re-record with open --save-script.',
305-
retriable: false,
306-
},
318+
overrides,
307319
);
308320
}
309321

0 commit comments

Comments
 (0)