docs: plan Pi-enabled model selector
This commit is contained in:
User
@@ -0,0 +1,918 @@
|
||||
# Pi-Enabled Model Selector Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Restore a usable model selector that exposes exactly GLM 5.2, DeepSeek V4 Flash, and local Qwen3.6 from Pi's effective `enabledModels` configuration.
|
||||
|
||||
**Architecture:** Add a focused Pi-settings reader that resolves global/project precedence and returns exact composite model IDs. The ephemeral Pi lister will run with a scrubbed, provider-neutral environment, intersect Pi's available catalog with those IDs, and keep the existing API shape; session spawning will explicitly treat `local-qwen` as local.
|
||||
|
||||
**Tech Stack:** Node.js 22, TypeScript, Fastify, Pi RPC, Vitest, React 18, Testing Library/MSW, Docker Compose.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- The selector exposes exactly `zai/glm-5.2`, `deepseek/deepseek-v4-flash`, and `local-qwen/qwen3.6-35b-a3b`.
|
||||
- `zai/glm-5v-turbo` remains hidden.
|
||||
- Pi `enabledModels` is the single source of truth; do not add an application allowlist.
|
||||
- Only exact `provider/model` entries are accepted; do not expand wildcard, fuzzy, model-only, or thinking-suffixed patterns.
|
||||
- Missing or malformed model scope fails closed to an empty model list and emits only sanitized warnings.
|
||||
- Do not expose credential values or `auth.json` contents in code, tests, logs, or command output.
|
||||
- Preserve hosted-provider credential isolation and reject unknown providers.
|
||||
- UI strings remain English.
|
||||
- Backend gates are `npx vitest run` and `npx tsc --noEmit -p .`.
|
||||
- Frontend gates are `npx vitest run` and `npx tsc -b`.
|
||||
- Completion requires rebuilding and recreating the impacted Docker container, not merely building source assets.
|
||||
|
||||
---
|
||||
|
||||
## File map
|
||||
|
||||
- Create `backend/src/pi/enabled-models.ts`: load and validate effective Pi `enabledModels`.
|
||||
- Create `backend/test/enabled-models.test.ts`: precedence, validation, and fail-closed unit tests.
|
||||
- Modify `backend/src/pi/list-models.ts`: provider-neutral Pi spawn, enabled-model intersection, warning callback.
|
||||
- Modify `backend/test/list-models.test.ts`: regression, ordering, filtering, caching, and scrubbed-environment tests.
|
||||
- Modify `backend/src/routes/settings.ts`: validate the composite `provider/model` pair.
|
||||
- Modify `backend/src/routes/meta.ts`: sanitized warning when the lister fails.
|
||||
- Modify `backend/src/app.ts`: wire model-scope warnings into the Fastify logger.
|
||||
- Modify `backend/test/routes-settings.test.ts`: duplicate-ID/different-provider validation regression.
|
||||
- Modify `backend/src/pi/provider-credentials.ts`: classify `local-qwen` as local.
|
||||
- Modify `backend/test/provider-credentials.test.ts`: direct custom-local credential-policy test.
|
||||
- Modify `backend/test/pi-process-manager.test.ts`: session-spawn coverage for `local-qwen`.
|
||||
- Modify `frontend/src/shell/AppShell.new-session.test.tsx`: render and select the three-model response.
|
||||
- Modify `/home/chirone/thothii-data/pi-config/agent/settings.json`: remove live GLM-5V from `enabledModels`.
|
||||
- Modify `PROJECT_STATE.md`: record the regression fix, verification counts, deployment time, and image/container digest.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Effective Pi `enabledModels` reader
|
||||
|
||||
**Files:**
|
||||
- Create: `backend/src/pi/enabled-models.ts`
|
||||
- Test: `backend/test/enabled-models.test.ts`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `harnessDir`, optional `agentDir`, and an injectable `(path: string) => string` reader.
|
||||
- Produces: `loadPiEnabledModels(opts): PiEnabledModelsResult`, where the result is `{ ids: string[]; warnings: string[]; source?: string }`.
|
||||
|
||||
- [ ] **Step 1: Write the failing settings-reader tests**
|
||||
|
||||
```ts
|
||||
import { expect, test } from "vitest";
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { loadPiEnabledModels } from "../src/pi/enabled-models.js";
|
||||
|
||||
function fixture(globalValue: unknown, projectValue?: unknown) {
|
||||
const root = mkdtempSync(join(tmpdir(), "tht-enabled-models-"));
|
||||
const agentDir = join(root, "agent");
|
||||
const harnessDir = join(root, "harness");
|
||||
mkdirSync(agentDir, { recursive: true });
|
||||
mkdirSync(join(harnessDir, ".pi"), { recursive: true });
|
||||
writeFileSync(join(agentDir, "settings.json"), JSON.stringify(globalValue));
|
||||
if (projectValue !== undefined) {
|
||||
writeFileSync(join(harnessDir, ".pi", "settings.json"), JSON.stringify(projectValue));
|
||||
}
|
||||
return { root, agentDir, harnessDir };
|
||||
}
|
||||
|
||||
test("loads exact global enabledModels in configured order", () => {
|
||||
const f = fixture({ enabledModels: [
|
||||
"zai/glm-5.2",
|
||||
"deepseek/deepseek-v4-flash",
|
||||
"local-qwen/qwen3.6-35b-a3b",
|
||||
] });
|
||||
try {
|
||||
expect(loadPiEnabledModels(f)).toMatchObject({
|
||||
ids: [
|
||||
"zai/glm-5.2",
|
||||
"deepseek/deepseek-v4-flash",
|
||||
"local-qwen/qwen3.6-35b-a3b",
|
||||
],
|
||||
warnings: [],
|
||||
});
|
||||
} finally { rmSync(f.root, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
test("project enabledModels overrides global enabledModels", () => {
|
||||
const f = fixture(
|
||||
{ enabledModels: ["zai/glm-5.2", "zai/glm-5v-turbo"] },
|
||||
{ enabledModels: ["local-qwen/qwen3.6-35b-a3b"] },
|
||||
);
|
||||
try {
|
||||
expect(loadPiEnabledModels(f).ids).toEqual(["local-qwen/qwen3.6-35b-a3b"]);
|
||||
} finally { rmSync(f.root, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
test("project settings without enabledModels fall back to global settings", () => {
|
||||
const f = fixture({ enabledModels: ["zai/glm-5.2"] }, { theme: "thothii-mono" });
|
||||
try {
|
||||
expect(loadPiEnabledModels(f).ids).toEqual(["zai/glm-5.2"]);
|
||||
} finally { rmSync(f.root, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
test("invalid entries are ignored, deduplicated, and reported without their content", () => {
|
||||
const f = fixture({ enabledModels: [
|
||||
"zai/glm-5.2", "zai/glm-5.2", "glm-5.2", "zai/*", "zai/glm-5.2:high", 7,
|
||||
] });
|
||||
try {
|
||||
const result = loadPiEnabledModels(f);
|
||||
expect(result.ids).toEqual(["zai/glm-5.2"]);
|
||||
expect(result.warnings).toHaveLength(4);
|
||||
expect(result.warnings.join(" ")).not.toContain("glm-5.2:high");
|
||||
} finally { rmSync(f.root, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
test.each([
|
||||
["missing field", {}],
|
||||
["empty field", { enabledModels: [] }],
|
||||
["wrong type", { enabledModels: "zai/glm-5.2" }],
|
||||
])("%s fails closed", (_name, settings) => {
|
||||
const f = fixture(settings);
|
||||
try {
|
||||
const result = loadPiEnabledModels(f);
|
||||
expect(result.ids).toEqual([]);
|
||||
expect(result.warnings.length).toBeGreaterThan(0);
|
||||
} finally { rmSync(f.root, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
test("malformed project settings fail closed instead of exposing global models", () => {
|
||||
const f = fixture({ enabledModels: ["zai/glm-5.2"] });
|
||||
writeFileSync(join(f.harnessDir, ".pi", "settings.json"), "{");
|
||||
try {
|
||||
const result = loadPiEnabledModels(f);
|
||||
expect(result.ids).toEqual([]);
|
||||
expect(result.warnings).toHaveLength(1);
|
||||
} finally { rmSync(f.root, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
test("missing global settings fail closed", () => {
|
||||
const f = fixture({ enabledModels: ["zai/glm-5.2"] });
|
||||
rmSync(join(f.agentDir, "settings.json"));
|
||||
try {
|
||||
const result = loadPiEnabledModels(f);
|
||||
expect(result.ids).toEqual([]);
|
||||
expect(result.warnings).toHaveLength(1);
|
||||
} finally { rmSync(f.root, { recursive: true, force: true }); }
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the focused test and verify the missing module failure**
|
||||
|
||||
Run: `cd backend && npx vitest run test/enabled-models.test.ts`
|
||||
|
||||
Expected: FAIL because `../src/pi/enabled-models.js` does not exist.
|
||||
|
||||
- [ ] **Step 3: Implement the effective settings reader**
|
||||
|
||||
```ts
|
||||
import { readFileSync } from "node:fs";
|
||||
import { homedir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
|
||||
export interface PiEnabledModelsResult {
|
||||
ids: string[];
|
||||
warnings: string[];
|
||||
source?: string;
|
||||
}
|
||||
|
||||
interface LoadOptions {
|
||||
harnessDir: string;
|
||||
agentDir?: string;
|
||||
read?: (path: string) => string;
|
||||
}
|
||||
|
||||
type Settings = Record<string, unknown>;
|
||||
const INVALID = Symbol("invalid-settings");
|
||||
|
||||
function readSettings(
|
||||
path: string,
|
||||
optional: boolean,
|
||||
read: (path: string) => string,
|
||||
warnings: string[],
|
||||
): Settings | undefined | typeof INVALID {
|
||||
try {
|
||||
const value = JSON.parse(read(path));
|
||||
if (!value || typeof value !== "object" || Array.isArray(value)) throw new Error();
|
||||
return value as Settings;
|
||||
} catch (error) {
|
||||
if (optional && (error as NodeJS.ErrnoException)?.code === "ENOENT") return undefined;
|
||||
warnings.push(`Pi settings are unreadable or invalid: ${path}`);
|
||||
return INVALID;
|
||||
}
|
||||
}
|
||||
|
||||
function isExactCompositeId(value: unknown): value is string {
|
||||
if (typeof value !== "string" || /[\s*?:]/.test(value)) return false;
|
||||
const parts = value.split("/");
|
||||
return parts.length === 2 && parts.every((part) => part.length > 0);
|
||||
}
|
||||
|
||||
export function loadPiEnabledModels(opts: LoadOptions): PiEnabledModelsResult {
|
||||
const warnings: string[] = [];
|
||||
const read = opts.read ?? ((path: string) => readFileSync(path, "utf8"));
|
||||
const agentDir = opts.agentDir ?? join(homedir(), ".pi", "agent");
|
||||
const globalPath = join(agentDir, "settings.json");
|
||||
const projectPath = join(opts.harnessDir, ".pi", "settings.json");
|
||||
const globalSettings = readSettings(globalPath, false, read, warnings);
|
||||
const projectSettings = readSettings(projectPath, true, read, warnings);
|
||||
if (globalSettings === INVALID || projectSettings === INVALID) return { ids: [], warnings };
|
||||
|
||||
const projectDefinesScope = projectSettings
|
||||
? Object.prototype.hasOwnProperty.call(projectSettings, "enabledModels")
|
||||
: false;
|
||||
const settings = projectDefinesScope ? projectSettings : globalSettings;
|
||||
const source = projectDefinesScope ? projectPath : globalPath;
|
||||
const raw = settings?.enabledModels;
|
||||
if (!Array.isArray(raw) || raw.length === 0) {
|
||||
warnings.push(`Pi enabledModels is missing or empty: ${source}`);
|
||||
return { ids: [], warnings, source };
|
||||
}
|
||||
|
||||
const ids: string[] = [];
|
||||
const seen = new Set<string>();
|
||||
raw.forEach((value, index) => {
|
||||
if (!isExactCompositeId(value)) {
|
||||
warnings.push(`Ignoring non-exact enabledModels entry at index ${index}: ${source}`);
|
||||
return;
|
||||
}
|
||||
if (!seen.has(value)) {
|
||||
seen.add(value);
|
||||
ids.push(value);
|
||||
}
|
||||
});
|
||||
if (ids.length === 0) warnings.push(`Pi enabledModels contains no exact model IDs: ${source}`);
|
||||
return { ids, warnings, source };
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the reader tests and type-check**
|
||||
|
||||
Run: `cd backend && npx vitest run test/enabled-models.test.ts && npx tsc --noEmit -p .`
|
||||
|
||||
Expected: all focused tests PASS and TypeScript exits 0.
|
||||
|
||||
- [ ] **Step 5: Commit the reader**
|
||||
|
||||
```bash
|
||||
git add backend/src/pi/enabled-models.ts backend/test/enabled-models.test.ts
|
||||
git commit -m "feat(backend): read Pi enabled model scope"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Provider-neutral Pi listing and exact scope filtering
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/src/pi/list-models.ts`
|
||||
- Modify: `backend/test/list-models.test.ts`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `loadPiEnabledModels({ harnessDir })` from Task 1 and Pi RPC `get_available_models`.
|
||||
- Produces: `createPiModelLister(cfg, opts): () => Promise<PiModel[]>`; new options are `loadEnabledModels?: () => PiEnabledModelsResult` and `warn?: (message: string) => void`.
|
||||
|
||||
- [ ] **Step 1: Replace provider-specific lister expectations with failing regression tests**
|
||||
|
||||
Add this helper near the top of `backend/test/list-models.test.ts`. In the first mapping
|
||||
test pass both `zai/glm-5.2` and `anthropic/claude-opus-4-8`; in the cache test pass
|
||||
`zai/glm-5.2`; in environment-only tests whose fake catalog is empty pass
|
||||
`test/unavailable` so Pi still spawns. This keeps every existing lister test independent
|
||||
of the developer's home directory:
|
||||
|
||||
```ts
|
||||
function enabled(...ids: string[]) {
|
||||
return () => ({ ids, warnings: [], source: "/test/settings.json" });
|
||||
}
|
||||
```
|
||||
|
||||
Remove the obsolete lister-only tests that expect enumeration to inject the selected
|
||||
provider key or reject compound providers. Session credential isolation remains
|
||||
covered by `provider-credentials.test.ts` and `pi-process-manager.test.ts`.
|
||||
|
||||
Add these regression tests:
|
||||
|
||||
```ts
|
||||
test("model listing does not require PI_PROVIDER or read the generic credential", async () => {
|
||||
const script = scriptWith([
|
||||
{ provider: "zai", id: "glm-5.2", name: "GLM-5.2", reasoning: true },
|
||||
]);
|
||||
const calls: any[][] = [];
|
||||
try {
|
||||
const lister = createPiModelLister(loadConfig({
|
||||
PI_BIN: "/usr/local/bin/pi",
|
||||
THT_MODEL_API_KEY_FILE: "/missing-and-must-not-be-read",
|
||||
}), {
|
||||
loadEnabledModels: enabled("zai/glm-5.2"),
|
||||
spawnFn: (...args: any[]) => {
|
||||
calls.push(args);
|
||||
return spawn("node", [FAKE, script]) as any;
|
||||
},
|
||||
});
|
||||
await expect(lister()).resolves.toHaveLength(1);
|
||||
expect(calls[0][2].env).not.toHaveProperty("ZAI_API_KEY");
|
||||
expect(calls[0][2].env).not.toHaveProperty("THT_MODEL_API_KEY_FILE");
|
||||
} finally { rmSync(path.dirname(script), { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
test("returns only enabled available models in enabledModels order", async () => {
|
||||
const script = scriptWith([
|
||||
{ provider: "zai", id: "glm-5v-turbo", name: "GLM-5V-Turbo", reasoning: true },
|
||||
{ provider: "local-qwen", id: "qwen3.6-35b-a3b", name: "Qwen3.6 Local", reasoning: false },
|
||||
{ provider: "zai", id: "glm-5.2", name: "GLM-5.2", reasoning: true },
|
||||
{ provider: "deepseek", id: "deepseek-v4-flash", name: "DeepSeek V4 Flash", reasoning: true },
|
||||
]);
|
||||
try {
|
||||
const lister = createPiModelLister(loadConfig({}), {
|
||||
loadEnabledModels: enabled(
|
||||
"zai/glm-5.2",
|
||||
"deepseek/deepseek-v4-flash",
|
||||
"local-qwen/qwen3.6-35b-a3b",
|
||||
),
|
||||
spawnFn: () => spawn("node", [FAKE, script]) as any,
|
||||
});
|
||||
expect((await lister()).map((m) => `${m.provider}/${m.id}`)).toEqual([
|
||||
"zai/glm-5.2",
|
||||
"deepseek/deepseek-v4-flash",
|
||||
"local-qwen/qwen3.6-35b-a3b",
|
||||
]);
|
||||
} finally { rmSync(path.dirname(script), { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
test("empty enabled model scope fails closed without spawning Pi", async () => {
|
||||
let spawns = 0;
|
||||
const warnings: string[] = [];
|
||||
const lister = createPiModelLister(loadConfig({}), {
|
||||
loadEnabledModels: () => ({ ids: [], warnings: ["scope invalid"] }),
|
||||
warn: (message) => warnings.push(message),
|
||||
spawnFn: () => { spawns += 1; throw new Error("must not spawn"); },
|
||||
});
|
||||
await expect(lister()).resolves.toEqual([]);
|
||||
expect(spawns).toBe(0);
|
||||
expect(warnings).toEqual(["scope invalid"]);
|
||||
});
|
||||
|
||||
test("warns and returns empty when enabled identifiers are unavailable", async () => {
|
||||
const script = scriptWith([
|
||||
{ provider: "zai", id: "glm-5v-turbo", name: "GLM-5V-Turbo", reasoning: true },
|
||||
]);
|
||||
const warnings: string[] = [];
|
||||
try {
|
||||
const lister = createPiModelLister(loadConfig({}), {
|
||||
loadEnabledModels: enabled("zai/glm-5.2"),
|
||||
warn: (message) => warnings.push(message),
|
||||
spawnFn: () => spawn("node", [FAKE, script]) as any,
|
||||
});
|
||||
await expect(lister()).resolves.toEqual([]);
|
||||
expect(warnings).toEqual(["No Pi-enabled models are currently available"]);
|
||||
} finally { rmSync(path.dirname(script), { recursive: true, force: true }); }
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the lister tests and verify the old implementation fails**
|
||||
|
||||
Run: `cd backend && npx vitest run test/list-models.test.ts`
|
||||
|
||||
Expected: FAIL because the lister still requires the configured provider credential,
|
||||
does not accept `loadEnabledModels`, and does not filter/order the response.
|
||||
|
||||
- [ ] **Step 3: Implement provider-neutral enumeration and filtering**
|
||||
|
||||
In `backend/src/pi/list-models.ts`, remove the `secretValue` import, import
|
||||
`loadPiEnabledModels` and its result type, extend `Opts`, and use the following core
|
||||
flow:
|
||||
|
||||
```ts
|
||||
import {
|
||||
loadPiEnabledModels,
|
||||
type PiEnabledModelsResult,
|
||||
} from "./enabled-models.js";
|
||||
|
||||
interface Opts {
|
||||
spawnFn?: (
|
||||
command: string,
|
||||
args: string[],
|
||||
options: { cwd: string; env: NodeJS.ProcessEnv },
|
||||
) => ChildProcessWithoutNullStreams;
|
||||
ttlMs?: number;
|
||||
nowMs?: () => number;
|
||||
loadEnabledModels?: () => PiEnabledModelsResult;
|
||||
warn?: (message: string) => void;
|
||||
}
|
||||
|
||||
// Inside listModels(), before spawning Pi:
|
||||
const enabled = (opts.loadEnabledModels
|
||||
?? (() => loadPiEnabledModels({ harnessDir: cfg.harnessDir })))();
|
||||
for (const warning of enabled.warnings) opts.warn?.(warning);
|
||||
if (enabled.ids.length === 0) {
|
||||
cache = { at: now(), models: [] };
|
||||
return [];
|
||||
}
|
||||
|
||||
const env = buildPiChildEnv({});
|
||||
delete env.THT_DATA_ROOT;
|
||||
if (cfg.dataRoot !== undefined) env.THT_DATA_ROOT = cfg.dataRoot;
|
||||
|
||||
// After mapping the RPC response to `available: PiModel[]`:
|
||||
const byCompositeId = new Map(
|
||||
available.map((model) => [`${model.provider}/${model.id}`, model]),
|
||||
);
|
||||
const models = enabled.ids.flatMap((id) => {
|
||||
const model = byCompositeId.get(id);
|
||||
return model ? [model] : [];
|
||||
});
|
||||
if (models.length === 0) opts.warn?.("No Pi-enabled models are currently available");
|
||||
cache = { at: now(), models };
|
||||
return models;
|
||||
```
|
||||
|
||||
Keep the existing `finally { child.kill(); }`, eight-second timeout, response mapping,
|
||||
TTL behavior, cwd, and `THT_DATA_ROOT` handling unchanged.
|
||||
|
||||
- [ ] **Step 4: Run focused and neighboring backend tests**
|
||||
|
||||
Run: `cd backend && npx vitest run test/enabled-models.test.ts test/list-models.test.ts test/provider-credentials.test.ts`
|
||||
|
||||
Expected: all tests PASS.
|
||||
|
||||
- [ ] **Step 5: Commit the lister regression fix**
|
||||
|
||||
```bash
|
||||
git add backend/src/pi/list-models.ts backend/test/list-models.test.ts
|
||||
git commit -m "fix(backend): scope Pi model listing to enabled models"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Composite settings validation and sanitized observability
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/src/routes/settings.ts`
|
||||
- Modify: `backend/src/routes/meta.ts`
|
||||
- Modify: `backend/src/app.ts`
|
||||
- Modify: `backend/test/routes-settings.test.ts`
|
||||
- Modify: `backend/test/routes-sql-meta.test.ts`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: filtered `ListModelsFn` results containing `{ provider, id }`.
|
||||
- Produces: `PUT /settings` validation by composite ID and warning-only Fastify logs for model-list failures.
|
||||
|
||||
- [ ] **Step 1: Write the failing composite-validation test**
|
||||
|
||||
```ts
|
||||
test("PUT /settings validates provider and model as one composite identifier", async () => {
|
||||
const { app, dir } = appWithTmpSettings({}, {
|
||||
listModels: async () => [
|
||||
{ provider: "provider-a", id: "shared-id", name: "A", reasoning: false },
|
||||
],
|
||||
});
|
||||
try {
|
||||
const wrongProvider = await app.inject({
|
||||
method: "PUT", url: "/settings",
|
||||
payload: {
|
||||
workspace: "psd", provider: "provider-b", model: "shared-id", thinking: "low",
|
||||
},
|
||||
});
|
||||
expect(wrongProvider.statusCode).toBe(400);
|
||||
|
||||
const exactPair = await app.inject({
|
||||
method: "PUT", url: "/settings",
|
||||
payload: {
|
||||
workspace: "psd", provider: "provider-a", model: "shared-id", thinking: "low",
|
||||
},
|
||||
});
|
||||
expect(exactPair.statusCode).toBe(200);
|
||||
} finally { rmSync(dir, { recursive: true, force: true }); }
|
||||
});
|
||||
```
|
||||
|
||||
In `routes-sql-meta.test.ts`, spy on the application logger and assert the graceful
|
||||
fallback logs only an error type, not the thrown message:
|
||||
|
||||
```ts
|
||||
test("GET /models logs a sanitized warning when listing fails", async () => {
|
||||
const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), {
|
||||
thtRunner: {} as any,
|
||||
listModels: async () => { throw new Error("credential-value-must-not-appear"); },
|
||||
});
|
||||
const warn = vi.spyOn(app.log, "warn");
|
||||
const res = await app.inject({ method: "GET", url: "/models" });
|
||||
expect(res.json()).toEqual({ models: [] });
|
||||
expect(JSON.stringify(warn.mock.calls)).not.toContain("credential-value-must-not-appear");
|
||||
expect(warn).toHaveBeenCalled();
|
||||
});
|
||||
```
|
||||
|
||||
Add `vi` to that test file's Vitest import.
|
||||
|
||||
- [ ] **Step 2: Run route tests and verify failures**
|
||||
|
||||
Run: `cd backend && npx vitest run test/routes-settings.test.ts test/routes-sql-meta.test.ts`
|
||||
|
||||
Expected: FAIL because validation currently compares only `id` and the failure path
|
||||
does not log.
|
||||
|
||||
- [ ] **Step 3: Implement composite validation and warnings**
|
||||
|
||||
In `settings.ts`, replace the available model type and predicate:
|
||||
|
||||
```ts
|
||||
let available: { provider: string; id: string }[] = [];
|
||||
// existing try/catch remains
|
||||
if (available.length > 0 && !available.some(
|
||||
(candidate) => candidate.provider === b.provider && candidate.id === b.model,
|
||||
)) {
|
||||
return reply.code(400).send({
|
||||
error: `Unknown model: ${b.provider ?? "unknown"}/${b.model}`,
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
In `meta.ts`, accept the request and log only a sanitized type:
|
||||
|
||||
```ts
|
||||
app.get("/models", async (req) => {
|
||||
const fn = deps.listModels ?? (async () => []);
|
||||
try {
|
||||
return { models: await fn() };
|
||||
} catch (error) {
|
||||
app.log.warn({
|
||||
component: "pi-model-list",
|
||||
errorType: error instanceof Error ? error.name : typeof error,
|
||||
}, "Pi model listing failed");
|
||||
return { models: [] as PiModel[] };
|
||||
}
|
||||
});
|
||||
```
|
||||
|
||||
In `app.ts`, enable warning-level explicit logs without request logging and route
|
||||
configuration warnings through the same logger:
|
||||
|
||||
```ts
|
||||
const app = Fastify({ logger: { level: "warn" }, disableRequestLogging: true });
|
||||
|
||||
const listModels = deps?.listModels ?? createPiModelLister(config, {
|
||||
warn: (detail) => app.log.warn(
|
||||
{ component: "pi-model-list", detail },
|
||||
"Pi enabled-model configuration warning",
|
||||
),
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run route tests and the backend type checker**
|
||||
|
||||
Run: `cd backend && npx vitest run test/routes-settings.test.ts test/routes-sql-meta.test.ts && npx tsc --noEmit -p .`
|
||||
|
||||
Expected: all route tests PASS and TypeScript exits 0.
|
||||
|
||||
- [ ] **Step 5: Commit API validation and logging**
|
||||
|
||||
```bash
|
||||
git add backend/src/routes/settings.ts backend/src/routes/meta.ts backend/src/app.ts backend/test/routes-settings.test.ts backend/test/routes-sql-meta.test.ts
|
||||
git commit -m "fix(backend): validate configured model provider pair"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: `local-qwen` session credential policy
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/src/pi/provider-credentials.ts`
|
||||
- Modify: `backend/test/provider-credentials.test.ts`
|
||||
- Modify: `backend/test/pi-process-manager.test.ts`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: selected provider `local-qwen` from application settings.
|
||||
- Produces: a scrubbed Pi child environment with no generic hosted-provider credential requirement.
|
||||
|
||||
- [ ] **Step 1: Write failing direct and session-spawn tests**
|
||||
|
||||
Add to `provider-credentials.test.ts`:
|
||||
|
||||
```ts
|
||||
test("local-qwen is an explicit local provider and needs no generic key", () => {
|
||||
const env = buildPiChildEnv({
|
||||
ambient: {
|
||||
PI_PROVIDER_API_KEY: "must-not-leak",
|
||||
OPENAI_API_KEY: "must-not-leak",
|
||||
THT_MODEL_API_KEY_FILE: "/must/not/leak",
|
||||
},
|
||||
provider: "local-qwen",
|
||||
});
|
||||
expect(env).not.toHaveProperty("PI_PROVIDER_API_KEY");
|
||||
expect(env).not.toHaveProperty("OPENAI_API_KEY");
|
||||
expect(env).not.toHaveProperty("THT_MODEL_API_KEY_FILE");
|
||||
});
|
||||
```
|
||||
|
||||
Change the existing local session test in `pi-process-manager.test.ts` to run for both
|
||||
providers:
|
||||
|
||||
```ts
|
||||
test.each(["ollama", "local-qwen"])(
|
||||
"local provider %s spawns without a model key and scrubs ambient credentials",
|
||||
async (provider) => {
|
||||
vi.stubEnv("PI_PROVIDER_API_KEY", "ambient-secret");
|
||||
vi.stubEnv("THT_MODEL_API_KEY_FILE", "/ambient/secret-path");
|
||||
vi.stubEnv("OPENAI_API_KEY", "unselected-provider-secret");
|
||||
const calls: any[][] = [];
|
||||
const child = recordingChild();
|
||||
child.stderr.resume = () => {};
|
||||
const mgr = new PiProcessManager(loadConfig({ PI_BIN: "/usr/local/bin/pi" }), {
|
||||
spawnFn: (...args: any[]) => { calls.push(args); return child as any; },
|
||||
});
|
||||
try {
|
||||
await mgr.spawnFor(`local-session-${provider}`, { provider });
|
||||
expect(calls[0][2].env).not.toHaveProperty("PI_PROVIDER_API_KEY");
|
||||
expect(calls[0][2].env).not.toHaveProperty("THT_MODEL_API_KEY_FILE");
|
||||
expect(calls[0][2].env).not.toHaveProperty("OPENAI_API_KEY");
|
||||
} finally {
|
||||
mgr.teardown(`local-session-${provider}`);
|
||||
vi.unstubAllEnvs();
|
||||
}
|
||||
},
|
||||
);
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the focused tests and verify `local-qwen` is rejected**
|
||||
|
||||
Run: `cd backend && npx vitest run test/provider-credentials.test.ts test/pi-process-manager.test.ts`
|
||||
|
||||
Expected: the `local-qwen` cases FAIL with `model provider credential is unavailable`.
|
||||
|
||||
- [ ] **Step 3: Add only `local-qwen` to the explicit local-provider set**
|
||||
|
||||
```ts
|
||||
const LOCAL_PROVIDERS = new Set([
|
||||
"ollama", "lmstudio", "local", "aritmolab", "local-qwen", "faux",
|
||||
]);
|
||||
```
|
||||
|
||||
Do not relax the unknown-provider or compound-provider branches.
|
||||
|
||||
- [ ] **Step 4: Run credential and process-manager tests**
|
||||
|
||||
Run: `cd backend && npx vitest run test/provider-credentials.test.ts test/pi-process-manager.test.ts`
|
||||
|
||||
Expected: all tests PASS, including the existing unknown/compound-provider failures.
|
||||
|
||||
- [ ] **Step 5: Commit the provider policy change**
|
||||
|
||||
```bash
|
||||
git add backend/src/pi/provider-credentials.ts backend/test/provider-credentials.test.ts backend/test/pi-process-manager.test.ts
|
||||
git commit -m "fix(backend): allow configured local Qwen provider"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 5: Frontend selector regression test
|
||||
|
||||
**Files:**
|
||||
- Modify: `frontend/src/shell/AppShell.new-session.test.tsx`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: unchanged `GET /models` response with three `PiModel` objects.
|
||||
- Produces: test proof that the selector displays all three and persists Qwen's provider/model pair.
|
||||
|
||||
- [ ] **Step 1: Add the three-model selector test**
|
||||
|
||||
Add `within` to the Testing Library import, then add:
|
||||
|
||||
```tsx
|
||||
test("model selector shows the three Pi-enabled models and persists the selected provider", async () => {
|
||||
let saved: unknown;
|
||||
server.use(
|
||||
http.get("http://localhost:8787/settings", () => HttpResponse.json({
|
||||
workspace: "default", provider: "zai", model: "glm-5.2", thinking: "low",
|
||||
})),
|
||||
http.get("http://localhost:8787/models", () => HttpResponse.json({ models: [
|
||||
{ provider: "zai", id: "glm-5.2", name: "GLM-5.2", reasoning: true },
|
||||
{ provider: "deepseek", id: "deepseek-v4-flash", name: "DeepSeek V4 Flash", reasoning: true },
|
||||
{ provider: "local-qwen", id: "qwen3.6-35b-a3b", name: "Qwen3.6 35B A3B Local", reasoning: false },
|
||||
] })),
|
||||
http.put("http://localhost:8787/settings", async ({ request }) => {
|
||||
saved = await request.json();
|
||||
return HttpResponse.json(saved);
|
||||
}),
|
||||
);
|
||||
renderShell();
|
||||
|
||||
const select = await screen.findByRole("combobox", { name: "Model" });
|
||||
expect(within(select).getAllByRole("option").map((option) => option.textContent)).toEqual([
|
||||
"GLM-5.2", "DeepSeek V4 Flash", "Qwen3.6 35B A3B Local",
|
||||
]);
|
||||
await userEvent.selectOptions(select, "qwen3.6-35b-a3b");
|
||||
await waitFor(() => expect(saved).toMatchObject({
|
||||
provider: "local-qwen", model: "qwen3.6-35b-a3b",
|
||||
}));
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the focused frontend test**
|
||||
|
||||
Run: `cd frontend && npx vitest run src/shell/AppShell.new-session.test.tsx`
|
||||
|
||||
Expected: PASS with the existing component.
|
||||
|
||||
- [ ] **Step 3: Run frontend type checking**
|
||||
|
||||
Run: `cd frontend && npx tsc -b`
|
||||
|
||||
Expected: TypeScript exits 0.
|
||||
|
||||
- [ ] **Step 4: Commit the regression coverage**
|
||||
|
||||
```bash
|
||||
git add frontend/src/shell/AppShell.new-session.test.tsx
|
||||
git commit -m "test(frontend): cover configured model selector"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 6: Full verification, live Pi scope, Docker recreation, and state record
|
||||
|
||||
**Files:**
|
||||
- Modify: `/home/chirone/thothii-data/pi-config/agent/settings.json`
|
||||
- Modify: `PROJECT_STATE.md`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: the completed backend/frontend commits and mounted Pi profile.
|
||||
- Produces: deployed healthy `core` container whose live `/models` response contains exactly the three approved models.
|
||||
|
||||
- [ ] **Step 1: Run the complete source gates from a clean task branch**
|
||||
|
||||
```bash
|
||||
cd backend && npx vitest run && npx tsc --noEmit -p .
|
||||
cd ../frontend && npx vitest run && npx tsc -b
|
||||
cd .. && git diff --check && git status --short
|
||||
```
|
||||
|
||||
Expected: both complete test suites PASS, both type checks exit 0, `git diff --check`
|
||||
prints nothing, and status contains only intentional task files plus the pre-existing
|
||||
untracked `.vite/` directory in the main workspace.
|
||||
|
||||
- [ ] **Step 2: Update the mounted Pi scope using the approved exact list**
|
||||
|
||||
Apply this JSON change to
|
||||
`/home/chirone/thothii-data/pi-config/agent/settings.json` while preserving all other
|
||||
keys and formatting:
|
||||
|
||||
```diff
|
||||
"enabledModels": [
|
||||
"deepseek/deepseek-v4-flash",
|
||||
"zai/glm-5.2",
|
||||
- "local-qwen/qwen3.6-35b-a3b",
|
||||
- "zai/glm-5v-turbo"
|
||||
+ "local-qwen/qwen3.6-35b-a3b"
|
||||
]
|
||||
```
|
||||
|
||||
Validate without displaying credentials:
|
||||
|
||||
```bash
|
||||
docker compose exec -T core node -e '
|
||||
const s=require("/home/thoth/.pi/agent/settings.json");
|
||||
console.log(JSON.stringify({enabledModels:s.enabledModels}));
|
||||
'
|
||||
```
|
||||
|
||||
Expected: the printed array contains exactly DeepSeek Flash, GLM 5.2, and local Qwen;
|
||||
GLM-5V is absent.
|
||||
|
||||
- [ ] **Step 3: Build and recreate only the impacted `core` service**
|
||||
|
||||
```bash
|
||||
docker compose build core
|
||||
docker compose up -d --no-deps --force-recreate --wait --wait-timeout 60 core
|
||||
docker compose ps core
|
||||
```
|
||||
|
||||
Expected: build exits 0 and `core` reaches `running (healthy)`.
|
||||
|
||||
- [ ] **Step 4: Verify the live API returns the exact composite IDs**
|
||||
|
||||
```bash
|
||||
docker compose exec -T core node --input-type=module - <<'NODE'
|
||||
const response = await fetch("http://127.0.0.1:8787/models");
|
||||
const body = await response.json();
|
||||
const ids = body.models.map((m) => `${m.provider}/${m.id}`);
|
||||
const expected = [
|
||||
"deepseek/deepseek-v4-flash",
|
||||
"zai/glm-5.2",
|
||||
"local-qwen/qwen3.6-35b-a3b",
|
||||
];
|
||||
if (response.status !== 200 || JSON.stringify(ids) !== JSON.stringify(expected)) {
|
||||
throw new Error(`unexpected model list: ${JSON.stringify(ids)}`);
|
||||
}
|
||||
console.log(JSON.stringify({ status: response.status, ids }));
|
||||
NODE
|
||||
```
|
||||
|
||||
Expected: status 200 and exactly the three IDs in Pi `enabledModels` order.
|
||||
|
||||
- [ ] **Step 5: Smoke-test settings validation and restore the original setting**
|
||||
|
||||
First exercise the same process-manager path used by a session, stopping before any
|
||||
prompt is sent:
|
||||
|
||||
```bash
|
||||
docker compose exec -T core node --input-type=module - <<'NODE'
|
||||
import { loadConfig } from "/app/backend/dist/config.js";
|
||||
import { PiProcessManager } from "/app/backend/dist/pi/pi-process-manager.js";
|
||||
const manager = new PiProcessManager(loadConfig(process.env));
|
||||
for (const [provider, model] of [
|
||||
["deepseek", "deepseek-v4-flash"],
|
||||
["local-qwen", "qwen3.6-35b-a3b"],
|
||||
]) {
|
||||
const id = `model-smoke-${provider}`;
|
||||
const runtime = manager.createFor(id, { provider });
|
||||
try {
|
||||
await manager.configure(runtime, { provider, model, thinking: "low" });
|
||||
console.log(JSON.stringify({ provider, model, configured: true }));
|
||||
} finally {
|
||||
manager.teardown(id);
|
||||
}
|
||||
}
|
||||
NODE
|
||||
```
|
||||
|
||||
Expected: both models print `configured: true`; no session prompt or DWH operation is
|
||||
started.
|
||||
|
||||
Then verify API validation while restoring the user's setting:
|
||||
|
||||
```bash
|
||||
docker compose exec -T core node --input-type=module - <<'NODE'
|
||||
const base = "http://127.0.0.1:8787";
|
||||
const original = await (await fetch(`${base}/settings`)).json();
|
||||
async function put(provider, model) {
|
||||
return fetch(`${base}/settings`, {
|
||||
method: "PUT",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify({ ...original, provider, model }),
|
||||
});
|
||||
}
|
||||
try {
|
||||
for (const [provider, model] of [
|
||||
["deepseek", "deepseek-v4-flash"],
|
||||
["local-qwen", "qwen3.6-35b-a3b"],
|
||||
]) {
|
||||
const response = await put(provider, model);
|
||||
if (response.status !== 200) throw new Error(`${provider}/${model}: ${response.status}`);
|
||||
}
|
||||
const hidden = await put("zai", "glm-5v-turbo");
|
||||
if (hidden.status !== 400) throw new Error(`hidden model accepted: ${hidden.status}`);
|
||||
console.log(JSON.stringify({ deepseek: 200, localQwen: 200, hiddenGlm5v: 400 }));
|
||||
} finally {
|
||||
const restored = await fetch(`${base}/settings`, {
|
||||
method: "PUT",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify(original),
|
||||
});
|
||||
if (restored.status !== 200) throw new Error(`settings restore failed: ${restored.status}`);
|
||||
}
|
||||
NODE
|
||||
```
|
||||
|
||||
Expected: DeepSeek and Qwen return 200, GLM-5V returns 400, and the original settings
|
||||
are restored even if a smoke assertion fails.
|
||||
|
||||
- [ ] **Step 6: Inspect health, sanitized logs, and runtime identity**
|
||||
|
||||
```bash
|
||||
docker compose ps core
|
||||
docker compose logs --since=10m core
|
||||
docker image inspect thothii-core:local --format '{{.Id}} {{.Created}}'
|
||||
docker inspect thothii-core-1 --format '{{.Image}} {{.State.Health.Status}} {{.State.StartedAt}}'
|
||||
```
|
||||
|
||||
Expected: health is `healthy`; logs contain no credential value, uncaught model-list
|
||||
error, or unexpected Pi crash; the running container image ID equals the rebuilt image.
|
||||
|
||||
- [ ] **Step 7: Update and commit `PROJECT_STATE.md`**
|
||||
|
||||
Record the verified backend/frontend test totals, type-check success, the three live
|
||||
model IDs, removal of GLM-5V, deployment timestamp, rebuilt image ID, running container
|
||||
image ID, and health result. Preserve the file's current snapshot format.
|
||||
|
||||
```bash
|
||||
git add PROJECT_STATE.md
|
||||
git commit -m "docs: record model selector deployment"
|
||||
git status --short
|
||||
```
|
||||
|
||||
Expected: the commit succeeds and the task branch is clean apart from the known
|
||||
untracked `.vite/` directory if it is visible in that workspace.
|
||||
|
||||
---
|
||||
|
||||
## Final review checklist
|
||||
|
||||
- [ ] Every approved model appears once and in Pi configuration order.
|
||||
- [ ] GLM-5V and other authenticated Pi models are absent from `/models`.
|
||||
- [ ] A missing `PI_PROVIDER` does not prevent model enumeration.
|
||||
- [ ] Enumeration does not inherit or inject generic/provider credentials.
|
||||
- [ ] DeepSeek and Qwen settings updates succeed; the hidden model is rejected.
|
||||
- [ ] `local-qwen` spawns without a hosted-provider key.
|
||||
- [ ] Unknown and compound providers still fail closed.
|
||||
- [ ] No credential values appear in logs or test output.
|
||||
- [ ] Backend and frontend complete gates pass.
|
||||
- [ ] The rebuilt `core` container is healthy and runs the new image.
|
||||
Reference in New Issue
Block a user