From b4c74723d1a3072551e38d1bd3b5098e3872a51d Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 26 Feb 2026 10:12:57 -0800 Subject: [PATCH] fix C051 --- .plans/16c-pr89-remediation-checklist.md | 4 +-- .../Layers/ProviderSessionDirectory.test.ts | 33 +++++++++++++++++++ .../Layers/ProviderSessionDirectory.ts | 5 ++- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/.plans/16c-pr89-remediation-checklist.md b/.plans/16c-pr89-remediation-checklist.md index 70203677e2ba9a45839a80490f51ca5271e8edf6..c77b59ddaab1f2e66325ebf4e7795a0981ac85f0 100644 --- a/.plans/16c-pr89-remediation-checklist.md +++ b/.plans/16c-pr89-remediation-checklist.md @@ -378,8 +378,8 @@ Counts: active `51` (`valid=33`, `partially-valid=18`), closed-invalid `6` - Threads: PRRT_kwDORLtfbc5wiLhY - Audit note: ProviderSessionDirectory upsert remains read-merge-write without concurrency control. -- [ ] `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.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent: - - Status: `TODO` +- [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.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent: + - Status: `DONE` - Verdict: `partially-valid` - Severity: `Low` - Area: `Other` diff --git a/apps/server/src/provider/Layers/ProviderSessionDirectory.test.ts b/apps/server/src/provider/Layers/ProviderSessionDirectory.test.ts index 90fa94032b1fc22789fb3b3055e8c0029eb9c83e..a892c6f0fc7c82af984dbb1bb23c2187657f9c99 100644 --- a/apps/server/src/provider/Layers/ProviderSessionDirectory.test.ts +++ b/apps/server/src/provider/Layers/ProviderSessionDirectory.test.ts @@ -154,6 +154,39 @@ it.layer(makeDirectoryLayer(SqlitePersistenceMemory))("ProviderSessionDirectoryL } })); + it("clears providerThreadId when explicitly set to null", () => + Effect.gen(function* () { + const directory = yield* ProviderSessionDirectory; + const runtimeRepository = yield* ProviderSessionRuntimeRepository; + + const sessionId = ProviderSessionId.makeUnsafe("sess-clear-provider-thread-id"); + const threadId = ThreadId.makeUnsafe("thread-clear-provider-thread-id"); + const providerThreadId = ProviderThreadId.makeUnsafe("provider-thread-to-clear"); + + yield* directory.upsert({ + sessionId, + provider: "codex", + threadId, + adapterKey: "custom-adapter", + providerThreadId, + }); + + yield* directory.upsert({ + sessionId, + provider: "codex", + providerThreadId: null, + }); + + const runtime = yield* runtimeRepository.getBySessionId({ + providerSessionId: sessionId, + }); + assert.equal(Option.isSome(runtime), true); + if (Option.isSome(runtime)) { + assert.equal(runtime.value.providerThreadId, null); + assert.equal(runtime.value.adapterKey, "custom-adapter"); + } + })); + it("rehydrates persisted mappings across layer restart", () => Effect.gen(function* () { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-provider-directory-")); diff --git a/apps/server/src/provider/Layers/ProviderSessionDirectory.ts b/apps/server/src/provider/Layers/ProviderSessionDirectory.ts index 700f8b50914af68602a6046b5e463918e1d39dc3..1b096e91e0edc686b7243d6a9cea28d7c2fbd3cf 100644 --- a/apps/server/src/provider/Layers/ProviderSessionDirectory.ts +++ b/apps/server/src/provider/Layers/ProviderSessionDirectory.ts @@ -108,7 +108,10 @@ const makeProviderSessionDirectory = Effect.gen(function* () { threadId: resolvedThreadId, providerName: binding.provider, adapterKey: binding.adapterKey ?? existingRuntime?.adapterKey ?? binding.provider, - providerThreadId: binding.providerThreadId ?? existingRuntime?.providerThreadId ?? null, + providerThreadId: + binding.providerThreadId !== undefined + ? binding.providerThreadId + : (existingRuntime?.providerThreadId ?? null), status: binding.status ?? existingRuntime?.status ?? "running", lastSeenAt: now, resumeCursor: -- 2.54.0