Skip to content

Commit 1c01943

Browse files
committed
fix: ensure stable recording ID reuse in non-persistent runtime and update tests
1 parent 781b5b0 commit 1c01943

5 files changed

Lines changed: 128 additions & 8 deletions

File tree

.kato/test-tmp/in-chat-record-commands/new.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ desc: ''
55
created: 1771754400000
66
updated: 1771754400000
77
sessionId: session-1
8-
recordingIds: [2da347e7-cdd0-4fa9-ad18-aaa823dd9aa6]
8+
recordingIds: [42a0bf5e-142b-41e2-8a1b-91cace88400b]
99
conversationEventKinds: [message.user]
1010
---

apps/daemon/src/orchestrator/daemon_runtime.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1368,7 +1368,21 @@ async function applyControlCommandsForEvent(
13681368
provider,
13691369
sessionId,
13701370
);
1371-
if (active && active.outputPath === resolvedDestination) {
1371+
const activeOnResolvedDestination = active &&
1372+
active.outputPath === resolvedDestination;
1373+
const existingRecordingId = sessionEventState.destinationRecordingIds.get(
1374+
resolvedDestination,
1375+
);
1376+
const recordingIdToUse = existingRecordingId ??
1377+
(activeOnResolvedDestination ? active.recordingId : undefined) ??
1378+
crypto.randomUUID();
1379+
if (!existingRecordingId) {
1380+
sessionEventState.destinationRecordingIds.set(
1381+
resolvedDestination,
1382+
recordingIdToUse,
1383+
);
1384+
}
1385+
if (activeOnResolvedDestination) {
13721386
commandNoop = true;
13731387
} else {
13741388
const seedEvents = buildCommandSeedEvents(
@@ -1382,6 +1396,7 @@ async function applyControlCommandsForEvent(
13821396
targetPath: resolvedDestination,
13831397
seedEvents,
13841398
title: recordingTitle,
1399+
recordingId: recordingIdToUse,
13851400
});
13861401
}
13871402
sessionEventState.primaryRecordingDestination = resolvedDestination;

dev-docs/notes/conv.2026.2026-02-27-inchat-command-redesign.md

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -742,4 +742,103 @@ So the clean status split is:
742742

743743
I’d mark the redesign task complete, with one brief note in it:
744744
- relative explicit path support is intentionally deferred to the per-workspace daemon task
745-
- `primaryRecordingDestination` remains the session pointer; `defaultOutputPath` will be a config default, not a rename of that field
745+
- `primaryRecordingDestination` remains the session pointer; `defaultOutputPath` will be a config default, not a rename of that field
746+
747+
# User_2026-02-27_2154_41
748+
749+
coderabbit feedback:
750+
751+
In `@apps/daemon/src/orchestrator/daemon_runtime.ts`:
752+
- Around line 1321-1347: The ::record branch activates a recording without
753+
reusing an existing recordingId, which can rotate IDs for the same destination
754+
and break stability; look up sessionEventState.destinationRecordingIds for the
755+
resolved destination (the same key used by ::init/::capture), and when calling
756+
recordingPipeline.activateRecording pass the existing recordingId (or preserve
757+
it in the activateRecording payload) instead of always creating a new one;
758+
update the activation call in the block using resolvedDestination,
759+
recordingPipeline.getActiveRecording, and recordingPipeline.activateRecording to
760+
include/reuse the recordingId from
761+
sessionEventState.destinationRecordingIds[resolvedDestination] when present.
762+
763+
**Planning code inspection and fix**
764+
765+
# gpt-5.3-codex_2026-02-27_2154_48
766+
767+
I’m checking that `::record` path in the non-persistent runtime branch against the current code, then I’ll patch it and add a regression test if the feedback is valid.
768+
769+
**Exploring record branch code**
770+
771+
**Planning stable recording ID reuse**
772+
**Designing stable recording ID handling**
773+
**Augmenting existing test for ID reuse**
774+
**Implementing robust recordingId handling and test adjustments**
775+
776+
# gpt-5.3-codex_2026-02-27_2155_31
777+
778+
I confirmed the feedback is valid: the non-persistent `::record` branch was not passing a stable `recordingId`. I’m patching that branch now and tightening the existing non-persistent `::init + ::record` test so it proves the activated ID matches the init-prepared file.
779+
780+
**Adding activated recording ID tracking**
781+
782+
**Running targeted tests**
783+
784+
# gpt-5.3-codex_2026-02-27_2155_53
785+
786+
I patched the runtime branch and added a regression assertion in the existing non-persistent `::init + ::record` test to prove the activated `recordingId` matches the init-prepared file. I’m running the targeted tests now.
787+
788+
**Running full lint and tests**
789+
790+
# gpt-5.3-codex_2026-02-27_2156_06
791+
792+
The targeted regression passes. I’m running the full quality gates now because this touches core runtime behavior.
793+
794+
# gpt-5.3-codex_2026-02-27_2156_09
795+
796+
`lint` is done; `check` and the full test suite are still running. Once they complete I’ll give you the exact change summary and whether this feedback item is fully cleared.
797+
798+
**Fixing test file cleanup to ensure fresh state**
799+
800+
# gpt-5.3-codex_2026-02-27_2156_22
801+
802+
The full suite exposed a test hygiene issue, not a logic regression: that non-persistent test reuses a fixed `.kato/test-tmp/.../new.md` path, so stale files can make the new frontmatter assertion nondeterministic. I’m making that test clean its temp directory before it runs.
803+
804+
**Adding async directory cleanup**
805+
806+
**Rerunning full tests and checks**
807+
808+
# gpt-5.3-codex_2026-02-27_2156_37
809+
810+
I fixed the flaky temp-file reuse in the regression test. `check` and the full test suite are running again; once they finish, this feedback item should be closed cleanly.
811+
812+
**Planning detailed git diff inspection**
813+
814+
**Extending code view for ID trace**
815+
816+
**Summarizing fix and validation**
817+
818+
# gpt-5.3-codex_2026-02-27_2157_05
819+
820+
The feedback was valid, and it’s fixed.
821+
822+
What changed:
823+
- The non-persistent `::record` branch now reuses the stable per-destination `recordingId` from `sessionEventState.destinationRecordingIds`, falls back to the active recording’s ID on same-target no-op, and only generates a new ID if neither exists.
824+
- That reused ID is now passed into `recordingPipeline.activateRecording(...)`.
825+
826+
Code:
827+
- Reuse/seed stable ID: [daemon_runtime.ts:1371](/home/djradon/hub/spectacular-voyage/kato/apps/daemon/src/orchestrator/daemon_runtime.ts:1371)
828+
- Pass reused ID into activation: [daemon_runtime.ts:1393](/home/djradon/hub/spectacular-voyage/kato/apps/daemon/src/orchestrator/daemon_runtime.ts:1393)
829+
830+
Regression coverage:
831+
- Tightened the existing non-persistent `::init + ::record` test so it now verifies the activated `recordingId` is non-empty and matches the ID written into the init-prepared file’s frontmatter.
832+
- Also made that test clear its temp directory first so it can’t be polluted by stale files from prior runs.
833+
834+
Test updates:
835+
- Temp-dir cleanup: [daemon-runtime_test.ts:2957](/home/djradon/hub/spectacular-voyage/kato/tests/daemon-runtime_test.ts:2957)
836+
- Capture activation `recordingId`: [daemon-runtime_test.ts:3071](/home/djradon/hub/spectacular-voyage/kato/tests/daemon-runtime_test.ts:3071)
837+
- Assert file contains the same ID: [daemon-runtime_test.ts:3143](/home/djradon/hub/spectacular-voyage/kato/tests/daemon-runtime_test.ts:3143)
838+
839+
Validation:
840+
- `deno task lint` passed
841+
- `deno task check` passed
842+
- `deno task test` passed (`279 passed, 0 failed`)
843+
844+
So this CodeRabbit item is cleared.

dev-docs/notes/dev.todo.md

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
id: mhthe39ktidk76iy77kcxbn
33
title: Todo
44
desc: ''
5-
updated: 1772257357673
5+
updated: 1772258142220
66
created: 1771812869620
77
---
88

@@ -37,11 +37,8 @@ created: 1771812869620
3737

3838
- [ ] Add `kato config validate` command for preflight runtime config checks.
3939
- [ ] Improve startup error UX for config/schema failures with actionable remediation hints.
40-
- [ ] Add workspace command surface: `kato workspace register|list|unregister|discover`.
4140
- [ ] Add workspace pre-persist verification and runtime re-verification status
4241
(`valid|invalid|unverified`) with explicit error reasons.
43-
- [ ] Add workspace-aware relative path resolution (session workspace identity)
44-
so in-chat relative targets do not depend on daemon cwd.
4542

4643
## Config And Feature-Flag Evolution
4744

tests/daemon-runtime_test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2954,6 +2954,8 @@ Deno.test("runDaemonRuntimeLoop applies in-chat ::record commands from newly ing
29542954
);
29552955
const oldPath = join(inChatCommandDir, "old.md");
29562956
const newPath = join(inChatCommandDir, "new.md");
2957+
await removeDirIfPresent(inChatCommandDir);
2958+
await Deno.mkdir(inChatCommandDir, { recursive: true });
29572959

29582960
let pollCount = 0;
29592961
const ingestionRunner: ProviderIngestionRunner = {
@@ -3067,15 +3069,18 @@ Deno.test("runDaemonRuntimeLoop applies in-chat ::record commands from newly ing
30673069
};
30683070

30693071
const activatedTargets: string[] = [];
3072+
const activatedRecordingIds: string[] = [];
30703073
const appendedMessageIds: string[] = [];
30713074
let activeRecording = false;
30723075
const recordingPipeline: RecordingPipelineLike = {
30733076
activateRecording(input) {
30743077
activeRecording = true;
30753078
activatedTargets.push(input.targetPath);
3079+
const recordingId = input.recordingId ?? "rec-1";
3080+
activatedRecordingIds.push(recordingId);
30763081
const nowIso = "2026-02-22T10:00:01.000Z";
30773082
return Promise.resolve({
3078-
recordingId: "rec-1",
3083+
recordingId,
30793084
provider: input.provider,
30803085
sessionId: input.sessionId,
30813086
outputPath: input.targetPath,
@@ -3136,9 +3141,13 @@ Deno.test("runDaemonRuntimeLoop applies in-chat ::record commands from newly ing
31363141
});
31373142

31383143
assertEquals(activatedTargets, [newPath]);
3144+
assertEquals(activatedRecordingIds.length, 1);
3145+
assert(activatedRecordingIds[0].length > 0);
31393146
assertEquals(appendedMessageIds, ["m2", "m3"]);
31403147
const newDestinationStat = await Deno.stat(newPath);
31413148
assert(newDestinationStat.isFile);
3149+
const newDestinationContent = await Deno.readTextFile(newPath);
3150+
assert(newDestinationContent.includes(activatedRecordingIds[0]));
31423151
});
31433152

31443153
Deno.test("runDaemonRuntimeLoop applies in-chat ::capture then activates recording on same path", async () => {

0 commit comments

Comments
 (0)