| author | |
| committer | |
| log | b4c74723d1a3072551e38d1bd3b5098e3872a51d |
| tree | 5620ef74e2aaa198aa746a1bf043af75858fc875 |
| parent | 1d975ad4310389b8e22bbce3d02a746854a11471 |
3 files changed, 39 insertions(+), 3 deletions(-)
.plans/16c-pr89-remediation-checklist.md+2-2| ... | @@ -378,8 +378,8 @@ Counts: active `51` (`valid=33`, `partially-valid=18`), closed-invalid `6` | ... | @@ -378,8 +378,8 @@ Counts: active `51` (`valid=33`, `partially-valid=18`), closed-invalid `6` |
| 378 | - Threads: PRRT_kwDORLtfbc5wiLhY | 378 | - Threads: PRRT_kwDORLtfbc5wiLhY |
| 379 | - Audit note: ProviderSessionDirectory upsert remains read-merge-write without concurrency control. | 379 | - Audit note: ProviderSessionDirectory upsert remains read-merge-write without concurrency control. |
| 380 | 380 | ||
| 381 | - [ ] `C051` Using `??` for `providerThreadId` and `adapterKey` makes it impossible to clear these fields by passing `null`, since `null ?? existing` evaluates to `existing`. Consider using explicit `undefined` checks (like `resumeCursor` does) if clearing should be supported. <details> <summary>🚀 Reply "<strong>fix it for me</strong>" or copy this <strong>AI Prompt</strong> for your agent:</summary> | 381 | - [x] `C051` Using `??` for `providerThreadId` and `adapterKey` makes it impossible to clear these fields by passing `null`, since `null ?? existing` evaluates to `existing`. Consider using explicit `undefined` checks (like `resumeCursor` does) if clearing should be supported. <details> <summary>🚀 Reply "<strong>fix it for me</strong>" or copy this <strong>AI Prompt</strong> for your agent:</summary> |
| 382 | - Status: `TODO` | 382 | - Status: `DONE` |
| 383 | - Verdict: `partially-valid` | 383 | - Verdict: `partially-valid` |
| 384 | - Severity: `Low` | 384 | - Severity: `Low` |
| 385 | - Area: `Other` | 385 | - Area: `Other` |
apps/server/src/provider/Layers/ProviderSessionDirectory.test.ts+33| ... | @@ -154,6 +154,39 @@ it.layer(makeDirectoryLayer(SqlitePersistenceMemory))("ProviderSessionDirectoryL | ... | @@ -154,6 +154,39 @@ it.layer(makeDirectoryLayer(SqlitePersistenceMemory))("ProviderSessionDirectoryL |
| 154 | } | 154 | } |
| 155 | })); | 155 | })); |
| 156 | 156 | ||
| 157 | it("clears providerThreadId when explicitly set to null", () => | ||
| 158 | Effect.gen(function* () { | ||
| 159 | const directory = yield* ProviderSessionDirectory; | ||
| 160 | const runtimeRepository = yield* ProviderSessionRuntimeRepository; | ||
| 161 | |||
| 162 | const sessionId = ProviderSessionId.makeUnsafe("sess-clear-provider-thread-id"); | ||
| 163 | const threadId = ThreadId.makeUnsafe("thread-clear-provider-thread-id"); | ||
| 164 | const providerThreadId = ProviderThreadId.makeUnsafe("provider-thread-to-clear"); | ||
| 165 | |||
| 166 | yield* directory.upsert({ | ||
| 167 | sessionId, | ||
| 168 | provider: "codex", | ||
| 169 | threadId, | ||
| 170 | adapterKey: "custom-adapter", | ||
| 171 | providerThreadId, | ||
| 172 | }); | ||
| 173 | |||
| 174 | yield* directory.upsert({ | ||
| 175 | sessionId, | ||
| 176 | provider: "codex", | ||
| 177 | providerThreadId: null, | ||
| 178 | }); | ||
| 179 | |||
| 180 | const runtime = yield* runtimeRepository.getBySessionId({ | ||
| 181 | providerSessionId: sessionId, | ||
| 182 | }); | ||
| 183 | assert.equal(Option.isSome(runtime), true); | ||
| 184 | if (Option.isSome(runtime)) { | ||
| 185 | assert.equal(runtime.value.providerThreadId, null); | ||
| 186 | assert.equal(runtime.value.adapterKey, "custom-adapter"); | ||
| 187 | } | ||
| 188 | })); | ||
| 189 | |||
| 157 | it("rehydrates persisted mappings across layer restart", () => | 190 | it("rehydrates persisted mappings across layer restart", () => |
| 158 | Effect.gen(function* () { | 191 | Effect.gen(function* () { |
| 159 | const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-provider-directory-")); | 192 | const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-provider-directory-")); |
apps/server/src/provider/Layers/ProviderSessionDirectory.ts+4-1| ... | @@ -108,7 +108,10 @@ const makeProviderSessionDirectory = Effect.gen(function* () { | ... | @@ -108,7 +108,10 @@ const makeProviderSessionDirectory = Effect.gen(function* () { |
| 108 | threadId: resolvedThreadId, | 108 | threadId: resolvedThreadId, |
| 109 | providerName: binding.provider, | 109 | providerName: binding.provider, |
| 110 | adapterKey: binding.adapterKey ?? existingRuntime?.adapterKey ?? binding.provider, | 110 | adapterKey: binding.adapterKey ?? existingRuntime?.adapterKey ?? binding.provider, |
| 111 | providerThreadId: binding.providerThreadId ?? existingRuntime?.providerThreadId ?? null, | 111 | providerThreadId: |
| 112 | binding.providerThreadId !== undefined | ||
| 113 | ? binding.providerThreadId | ||
| 114 | : (existingRuntime?.providerThreadId ?? null), | ||
| 112 | status: binding.status ?? existingRuntime?.status ?? "running", | 115 | status: binding.status ?? existingRuntime?.status ?? "running", |
| 113 | lastSeenAt: now, | 116 | lastSeenAt: now, |
| 114 | resumeCursor: | 117 | resumeCursor: |