From 0706cc26a5529e789267bb296cf04ce92ad98373 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Fri, 17 Jul 2026 21:33:14 +0200 Subject: [PATCH] relay(issue-62-authstorage): leg 5 status/log (slice 4 done) --- relays/issue-62-authstorage/log.md | 50 ++++++++++++ relays/issue-62-authstorage/status.md | 112 +++++++++++++++++++------- 2 files changed, 134 insertions(+), 28 deletions(-) diff --git a/relays/issue-62-authstorage/log.md b/relays/issue-62-authstorage/log.md index aa83fd6..14aa287 100644 --- a/relays/issue-62-authstorage/log.md +++ b/relays/issue-62-authstorage/log.md @@ -263,3 +263,53 @@ verify-green, per charter). **Blockers:** none. Sessiond-restart-pending note still ACTIVE (unchanged; this slice did not touch the daemon path, but slice 1 did). Handing off to leg 5 (slice 4). + +--- + +## Leg 5 — slice 4: piSessionService.ts migration (commit `4ccd4f8`) + +**What:** Migrated `src/server/sessions/piSessionService.ts` to the new +`ModelRuntime` API. +- `createDefaultRuntimeFactory` now takes a `ModelRuntime` and passes + `modelRuntime` to `createAgentSessionServices({ cwd, agentDir, modelRuntime })` + (dropped the `authStorage` + `modelRegistry` args). +- `PiAgentSession.modelRegistry: ModelRegistryInstance` → `modelRuntime: + ModelRuntime`. Removed the `ModelRegistryInstance` type alias and the + `AuthStorage`/`ModelRegistry` SDK imports; added `type ModelRuntime` + + `readStoredCredential` imports and `join` from node:path. `authService.js` + import reduced to just `AuthChange` (dropped `createModelRegistryForAgentDir`). +- `anthropicSubscriptionWarning(session, authPath?)`: reads via + `readStoredCredential("anthropic", authPath)`; param narrowed to + `Pick`. `warningsForSession` + passes `join(this.agentDir, "auth.json")`. +- Model reads rederived onto the runtime: `availableModels`/`setModel` → + `await modelRuntime.refresh()` + `getAvailableSnapshot()` + `getModel(...)`; + `syncCurrentModelAuthWarning` → `getModel(...)` + + `hasConfiguredAuth(providerId)`. +- `applyAuthChange` no longer refreshes a registry (shared runtime is refreshed + by AuthService before emit; all sessions share it), keeping the subscribe + callback synchronous. + +**Decision:** made `modelRuntime` a **required** `PiSessionServiceDependencies` +field rather than keeping an optional `modelRegistry?`-style fallback. The old +fallback built a registry synchronously in the constructor; `ModelRuntime` can +only be created by the async `ModelRuntime.create`, which cannot run in a +constructor. `sessiond.ts` already injects `modelRuntime: auth.runtime` (slice +1), so production wiring is unaffected. Consequence: the slice-5 test surface is +wider than the four files the assessment listed — every `new +PiSessionService(...)` in tests now needs `modelRuntime`, and `fakeRuntime`/the +`TestSession` type in `testSupport.ts` must expose `modelRuntime`. Documented in +status.md "Next task". + +**Result:** `npx tsc --noEmit` — `sessiond.ts` and `piSessionService.ts` at 0 +errors; all production code migrated (tsc output filtered to non-test/support +files is empty). Remaining errors are slice-5 test/support only: +authService.test (10), testSupport (4), warnings (5), promptQueue (17), +lifecycle (19), archiveCleanup (9), spawnSession (3), spawnSubsession (18), +sessionRoutes (1). `piSessionService.ts` lints clean. Committed `--no-verify` +(migration not yet verify-green, per charter). + +**Blockers:** none. **Sessiond-restart-pending note still ACTIVE** — this slice +added `piSessionService.ts` (a session-daemon path) to the pending-restart +surface; do not clear the note. Handing off to leg 6 (slice 5: tests + +testSupport). diff --git a/relays/issue-62-authstorage/status.md b/relays/issue-62-authstorage/status.md index 640ca34..1f6d385 100644 --- a/relays/issue-62-authstorage/status.md +++ b/relays/issue-62-authstorage/status.md @@ -1,6 +1,39 @@ # Relay status — issue-62-authstorage ## Current position +Slice 4 (`piSessionService.ts` migration) complete and committed (`4ccd4f8`). +`piSessionService.ts` now uses the new `ModelRuntime` API end to end: +- `createDefaultRuntimeFactory(modelRuntime, ...)` passes `modelRuntime` to + `createAgentSessionServices({ cwd, agentDir, modelRuntime })` (no more + `authStorage` + `modelRegistry`). +- `PiAgentSession.modelRegistry` → `PiAgentSession.modelRuntime: ModelRuntime`. +- `anthropicSubscriptionWarning(session, authPath?)` now reads via + `readStoredCredential("anthropic", authPath)` (sync); `warningsForSession` + passes `join(this.agentDir, "auth.json")`. Its `session` param narrowed to + `Pick` (no longer needs the + registry). +- Model reads rederived onto the runtime: `availableModels`/`setModel` use + `await modelRuntime.refresh()` + `getAvailableSnapshot()` + `getModel(...)`; + `syncCurrentModelAuthWarning` uses `getModel(...)` + + `hasConfiguredAuth(providerId)`. +- `applyAuthChange` no longer refreshes a registry (the shared runtime is + refreshed by AuthService before it emits, and all sessions share that + runtime), so the `auth.subscribe` callback stays synchronous. +- **`modelRuntime` is now a REQUIRED `PiSessionServiceDependencies` field** + (the old `modelRegistry?` fallback used a *sync* `ModelRegistry.create`; a + `ModelRuntime` can only be built by the async `ModelRuntime.create`, which + can't run inside a constructor). `sessiond.ts` already injects + `modelRuntime: auth.runtime` (slice 1), so it typechecks unchanged. +- Dropped the `AuthStorage` / `ModelRegistry` imports and the + `createModelRegistryForAgentDir` import (only `AuthChange` is still imported + from `authService.js`). + +`npx tsc --noEmit`: **`sessiond.ts` and `piSessionService.ts` are at 0 errors** +(production code is fully migrated; `grep -vE '\.test\.ts|testSupport\.ts'` on +tsc output is empty). All remaining errors are slice-5 test/support files. +`piSessionService.ts` lints clean. + +### Prior position (slice 3, leg 4, commit `1c3d6db`) Slice 3 (`oauthLoginFlowService.ts` migration) complete and committed (`1c3d6db`). `OAuthLoginFlowService` is reimplemented against the pi-ai `AuthInteraction` contract (`{ signal?, prompt(AuthPrompt), notify(AuthEvent) }`); the old @@ -76,34 +109,56 @@ Remaining errors otherwise live in slices 2/3/4 files and all test/support files (slice 5). ## Leg tracking -- **Last completed leg:** 4 (slice 3 — oauthLoginFlowService.ts migration). -- **Next leg to run:** 5. +- **Last completed leg:** 5 (slice 4 — piSessionService.ts migration). +- **Next leg to run:** 6. ## Next task -Run **charter slice 4 (`piSessionService.ts` migration)** as leg 5. Concretely -(see assessment §5.4 and §3.3): -- Pass `modelRuntime` to `createAgentSessionServices` (instead of the old - `modelRegistry`); update the `PiAgentSession` type accordingly. -- `sessiond.ts` already passes `modelRuntime: auth.runtime` into - `PiSessionService` (from slice 1) but `PiSessionServiceDependencies` still - declares `modelRegistry` — reconcile the dependency shape so the sessiond - wiring typechecks (this is the remaining `sessiond.ts` error). -- Switch `anthropicSubscriptionWarning` to `readStoredCredential(providerId, - authPath?)` (the sync credential read replacing the old AuthStorage-based - read). -- Target: after slice 4, `sessiond.ts` and `piSessionService.ts` reach 0 - errors; only the test/support files (slice 5) remain. -- **Note:** `piSessionService.ts` is a session-daemon path — keep the - sessiond-restart-pending note current (it is already active from slice 1). +Run **charter slice 5 (tests + testSupport)** as leg 6: migrate all test +doubles off `AuthStorage.inMemory(...)` / `ModelRegistry.create|inMemory(...)` +to the pi-ai `InMemoryCredentialStore` + `await ModelRuntime.create({ +credentials })`, and get `npm run verify` green. Follow the testing-guide skill +(async construction seams, no over-mocking of the SDK). -Then slice 5 migrates all test doubles to `InMemoryCredentialStore` + -`ModelRuntime.create` and gets `npm run verify` green (currently the failing -test/support files are `authService.test.ts` (10), `piSessionService.testSupport.ts` -(3), `.promptQueue.test.ts` (2), `.warnings.test.ts` (4)); slice 6 adds the -changeset + final verify + cleanup. +**Scope note (important):** slice 4 made `modelRuntime` a *required* +`PiSessionServiceDependencies` field (see Current position for why). That means +the slice-5 test surface is LARGER than the four files originally listed in the +assessment. Current `npx tsc --noEmit` failing files (all tests/support): +- `piSessionService.testSupport.ts` (4) — `fakeRuntime` builds + `modelRegistry: ModelRegistry.create(AuthStorage.inMemory())`; the + `TestSession` type still has `modelRegistry`. Give the fake a `modelRuntime` + (e.g. `await ModelRuntime.create({ credentials: new InMemoryCredentialStore() })` + — note this makes `fakeRuntime` async, which ripples into its callers) and + update `TestSession`. This is the central helper; fixing it first will clear + many downstream errors. +- `authService.test.ts` (10) — already partly slice-1/2/3 debt. +- `piSessionService.warnings.test.ts` (5) — `anthropicSubscriptionWarning` no + longer takes a registry; it now reads `readStoredCredential("anthropic", + authPath)`. Tests that build credentials via `authStorage.set(...)` must + instead write an `auth.json` (temp dir) and pass its path, OR the test seam + must be reconsidered. `SubscriptionSession` type ref to `modelRegistry` is + gone. Check whether `readStoredCredential` can be pointed at a temp authPath + cleanly; if not, consider whether the warning fn needs a small injectable + credential-read seam (raise via intervention if the API can't support the + test without contortion). +- `piSessionService.promptQueue.test.ts` (17), `.lifecycle.test.ts` (19), + `.archiveCleanup.test.ts` (9), `.spawnSession.test.ts` (3), + `.spawnSubsession.test.ts` (18), `sessionRoutes.test.ts` (1) — mostly the + new required `modelRuntime` dep on `new PiSessionService(...)` plus + `fakeRuntime`/`ModelRegistry.inMemory` usages. Many of these should clear + automatically once `testSupport.ts` provides a shared `modelRuntime` helper + and the `PiSessionService` test-construction path supplies it. -If slice 4 is already done when you arrive, apply the charter's task-selection -policy: pick the lowest-numbered incomplete slice (5 → 6). +Suggested approach: add a small shared test helper (e.g. +`await createTestModelRuntime()` wrapping `ModelRuntime.create({ credentials: +new InMemoryCredentialStore(...) })`) in `testSupport.ts`, thread it into +`fakeRuntime` and the `new PiSessionService(...)` call sites, then work file by +file until `npm run verify` (typecheck + lint + knip + test) is green. + +Then slice 6 adds the `.changeset/*.md` fragment, runs the full `npm run +verify`, and does final cleanup (ASSESSMENT stays). + +If slice 5 is already done when you arrive, apply the charter's task-selection +policy: pick the lowest-numbered incomplete slice (6). ### Build/tooling note (important for every leg) **Update (leg 2):** the human reports `/tmp` is now fully usable again, so the @@ -142,10 +197,11 @@ charter's Handover section. ## Blockers / intervention state None. Known constraints: - **Sessiond restart pending (ACTIVE):** slice 1 (leg 2, commit `e37148c`) - changed `sessiond.ts` + the session-daemon auth construction path. Per - AGENTS.md the human must **manually restart the sessiond service** for these - changes to take effect once the migration lands. Keep this note until the - human confirms the restart. + changed `sessiond.ts` + the session-daemon auth construction path; slice 4 + (leg 5, commit `4ccd4f8`) added `piSessionService.ts` (a session-daemon path) + to this surface. Per AGENTS.md the human must **manually restart the sessiond + service** for these changes to take effect once the migration lands. Keep + this note until the human confirms the restart. - `/tmp` disk-quota issue is resolved (human confirmed usable) — see the Build/tooling note above. - node_modules is installed (gitignored) at 0.80.10; a fresh `npm install` is