From d91466e66d347faa9325a115163f5eabaec20606 Mon Sep 17 00:00:00 2001 From: Shangxin Date: Fri, 16 Oct 2026 17:43:39 +0000 Subject: [PATCH] feat(config): isolate agent config directory, support hot reloading and plaintext key display - Config Isolation: isolate Pi agent configuration directory under /agent via resolveAgentDirPath(), set PI_CODING_AGENT_DIR in daemon launch, and explicitly pass agentDir down to Pi session factory and resident agent instances. - Hot Reloading: mark provider, model, and apiKey settings as applies: 'now', allow dynamic provider get/set in ConfigService, and introduce restartAgent() in watch-session to hot reload live agent instances upon configuration updates without requiring a daemon restart. - Plaintext Config Display: render apiKey in plaintext in settings.slint and config-service for user visibility and immediate editability. --- src/app-model.ts | 29 +++---- src/config-paths.ts | 26 ++++++ src/config-service.ts | 148 ++++++++++------------------------- src/panel-view.ts | 44 ++++++++--- src/pi-session-factory.ts | 36 ++++----- src/resident-agent.ts | 42 +++++----- src/squid-app.ts | 14 ++-- src/watch-desktop.ts | 2 + src/watch-session.ts | 61 +++++++++++---- tests/config-service.test.ts | 56 +++++-------- tests/config.test.ts | 1 + tests/watch-session.test.ts | 34 ++++++-- ui/settings.slint | 26 +----- 13 files changed, 259 insertions(+), 260 deletions(-) diff --git a/src/app-model.ts b/src/app-model.ts index ac4868e..e1fc3c0 100644 --- a/src/app-model.ts +++ b/src/app-model.ts @@ -108,28 +108,23 @@ export function readModelEntry(agentDir: string, provider: string): ModelEntryVa } /** - * Whether a credential for the provider can be *read*, never exposing its - * value: the key's content stays inside Pi's auth file and this module. Callers - * that need the value to restate it (an endpoint rewrite that also rewrote - * auth.json) are the reason `writeModelEndpoint` exists instead — an endpoint - * edit must not need the key at all. - * - * Readable, not stored, and the name says so on purpose. Pi's - * `readStoredCredential` catches every error and answers the same `undefined` - * for a missing file, a file this process may not open, and a truncated or - * hand-broken one — so a `false` here cannot tell "none was ever set" from "one - * is set and unreachable". That is still the right answer for every caller: the - * status check above reads through the same call, and Pi's own store keeps its - * last snapshot when a reload throws, so a credential this cannot read is one no - * session can use either. What it is not is a claim about what the *user* has - * done, and the settings form once made it exactly that ("Not set yet") in the - * one field where acting on the mistake overwrites something nobody can get - * back. Do not restore a name, or a comment, that says this reports existence. + * Whether a credential for the provider can be *read*. */ export function hasReadableCredential(agentDir: string, provider: string): boolean { return readStoredCredential(provider, join(agentDir, "auth.json")) !== undefined; } +/** + * Reads the stored credential key in plaintext for display in the settings UI. + */ +export function readStoredCredentialKey(agentDir: string, provider: string): string | undefined { + const credential = readStoredCredential(provider, join(agentDir, "auth.json")); + if (credential && credential.type === "api_key" && typeof credential.key === "string") { + return credential.key; + } + return undefined; +} + export interface FirstRunAnswers { readonly baseUrl: string; readonly modelId: string; diff --git a/src/config-paths.ts b/src/config-paths.ts index 4cffe96..a30c982 100644 --- a/src/config-paths.ts +++ b/src/config-paths.ts @@ -46,6 +46,32 @@ export function resolveConfigPath( return { path: defaultConfigPath(env, platform, home) }; } +/** + * Dedicated, isolated agent directory for Squid's PI Agent configuration. + * + * Defaults to `/agent` to guarantee complete isolation + * from any pre-existing local PI Agent installation (`~/.pi/agent`). + * Can be overridden by SQUID_AGENT_DIR or PI_CODING_AGENT_DIR. + */ +export function resolveAgentDirPath( + options: ResolveConfigPathOptions = {}, +): string { + const env = options.env ?? process.env; + if (env.SQUID_AGENT_DIR && env.SQUID_AGENT_DIR.length > 0) { + return env.SQUID_AGENT_DIR; + } + if (env.PI_CODING_AGENT_DIR && env.PI_CODING_AGENT_DIR.length > 0) { + return env.PI_CODING_AGENT_DIR; + } + const resolution = resolveConfigPath(options); + const configPath = "path" in resolution + ? resolution.path + : defaultConfigPath(env, options.platform ?? process.platform, options.home ?? homedir()); + const platform = options.platform ?? process.platform; + const pathMod = platform === "win32" ? win32 : posix; + return pathMod.join(pathMod.dirname(configPath), "agent"); +} + function isAbsoluteFor(platform: NodeJS.Platform, value: string): boolean { return platform === "win32" ? win32.isAbsolute(value) : posix.isAbsolute(value); } diff --git a/src/config-service.ts b/src/config-service.ts index 43d79a8..6609ba3 100644 --- a/src/config-service.ts +++ b/src/config-service.ts @@ -9,6 +9,7 @@ import { DEFAULT_BASE_URL, hasReadableCredential, readModelEntry, + readStoredCredentialKey, writeModelConfiguration, writeModelEndpoint, } from "./app-model.js"; @@ -23,39 +24,12 @@ import { * Pi's own `models.json`/`auth.json` — never a Squid-side parallel copy, per * the contract in `app-model.ts`. * - * The secret rule is structural, not a convention to remember: `apiKey`'s - * *value* never leaves this module's callers' sight — the dump reports it as - * the empty string, and a set with an empty value means "keep the stored - * credential". Nothing here ever reads the key back to restate it, which is - * why an endpoint edit goes through `writeModelEndpoint` (models.json only): - * the credential half is simply not touched. + * Model configurations (provider, baseUrl, modelId, apiKey) support hot reload: + * any change applies immediately ("now") and triggers in-session agent reload + * without restarting the application. * - * What the dump does say is whether a credential can be *read*, as - * `secretPresent` — deliberately weaker than whether one is stored, because an - * unreadable store answers no. - * That is the one fact the rule above accidentally hid: two very different - * states — a key is stored, or none ever was — both render as an empty field, - * so the only way to distinguish them was to save nothing and read the refusal. - * A yes/no is not the secret, and it is derived from `hasReadableCredential`, - * which is the only question this module asks about a stored key. Its absence - * is therefore never evidence about what the user has done, and no surface - * downstream may narrate it that way. - * - * Applies is per key, not global: file-backed settings are read once at - * startup by the resolvers, so any write to them can only land after a - * restart. A model setting saved while no agent is live can take effect - * immediately — `watch-session` starts the agent from the same files on - * recovery — but once an agent is running, Pi's session already holds the old - * endpoint, and honesty wins: that set also says "restart". - * - * Display metadata rides along with every entry, `group` and `description` - * beside `label`. It comes from `config-schema.ts` for the file-backed keys and - * from `MODEL_CONFIG_KEYS` for the three that live in Pi's store, which is the - * same split as everything else here — but the panel must not be able to tell, - * because a form that grouped one store's rows and not the other's would be - * confessing an implementation detail the user has no use for. So both halves - * carry the same three display fields, and neither is optional: an entry cannot - * reach the wire without having been placed under a capability and explained. + * API keys and credentials are displayed directly in plaintext in the UI for + * developer transparency and rapid iteration during MVP. */ /** The kinds a dump can name — the schema's three plus the secret case. */ @@ -75,25 +49,7 @@ export interface ConfigDumpEntry { /** An env var is governing this run, so the file value is not the live one. */ readonly overridden: boolean; readonly values?: readonly string[]; - /** - * A credential for this row can be read — that it is reachable, never any part - * of it. Only a `secret` kind carries this, and only when there is one to - * report, so an absent flag reads as "nothing readable" without a second - * spelling for it. Readable, not stored: `hasReadableCredential` answers no - * for a store it cannot open or parse just as it does for one with no entry, - * so this absence must never be rendered as "not set yet" — that tells a user - * they never entered a key that may well be sitting there, in the one field - * where acting on the belief overwrites something unrecoverable. - * - * It exists because the secret rule left one honest question unanswerable. A - * secret's value is always the empty string here, so a stored key and no key - * at all render identically, and the only way to tell them apart was to save - * an empty field and read back either "kept" or a refusal — - * a probe, in the one field where a mistaken save is least recoverable. - * Reporting presence is not reporting the secret: nothing in this module - * reads a stored key back, and this flag is derived from - * `hasReadableCredential`, which answers only yes or no. - */ + /** A credential for this row can be read. */ readonly secretPresent?: boolean; } @@ -149,11 +105,8 @@ export const MODEL_CONFIG_KEYS: readonly ModelConfigKey[] = [ id: "apiKey", label: "API key", group: "Model", - // Describes what the key is *for*, not what it is: this row's value is the - // one thing the dump never carries, so the sentence beside an always-empty - // field is all the user has to go on. description: "The secret that lets Squid use that model on your behalf.", - kind: "secret", + kind: "text", }, ]; @@ -167,20 +120,20 @@ export interface ConfigServiceOptions { /** The run's environment, for overridden flags — read, never written. */ readonly env: Record; readonly agentDir: string; - readonly provider: string; + provider: string; /** What an unset entry defaults to when a write needs a full trio. */ readonly defaultModelId: string; readonly defaultBaseUrl: string; /** Whether an agent currently holds the model configuration in memory. */ readonly agentLive: () => boolean; + readonly getProvider?: () => string; + readonly setProvider?: (provider: string) => void; } /** * Everything the settings form starts from. Re-reads the config file on every * call — there is no cache to go stale when a set lands mid-run — and reads - * the model entry from Pi's files. A value an env var is overriding is still - * the file value here, because the form edits the file; the `overridden` flag - * is the disclosure that the live run disagrees. + * the model entry from Pi's files. */ export function configDumpEntries(options: ConfigServiceOptions): ConfigDump { const file = readConfigFile(options.configPath ?? ""); @@ -188,14 +141,12 @@ export function configDumpEntries(options: ConfigServiceOptions): ConfigDump { return { ok: false, error: file.configError }; } const values = file.kind === "read" ? file.values : {}; - const entry = readModelEntry(options.agentDir, options.provider); + const activeProvider = options.getProvider ? options.getProvider() : options.provider; + const entry = readModelEntry(options.agentDir, activeProvider); const entries: ConfigDumpEntry[] = []; const overriddenEnv = (name: string): boolean => options.env[name] !== undefined; - const applies = (): "now" | "restart" => (options.agentLive() ? "restart" : "now"); - // Asked once for the whole dump rather than per row: it reads Pi's auth store, - // and only the one secret row has any use for the answer. - const readableCredential = hasReadableCredential(options.agentDir, options.provider); + const readableCredential = hasReadableCredential(options.agentDir, activeProvider); for (const key of MODEL_CONFIG_KEYS) { entries.push({ @@ -204,22 +155,14 @@ export function configDumpEntries(options: ConfigServiceOptions): ConfigDump { group: key.group, description: key.description, kind: key.kind, - // The secret's value is structurally absent from every dump: empty is - // "nothing to show", which is also exactly what "keep what is stored" - // means on the way back in. - value: key.kind === "secret" - ? "" + value: key.id === "apiKey" + ? readStoredCredentialKey(options.agentDir, activeProvider) ?? "" : key.id === "modelBaseUrl" ? entry?.baseUrl ?? "" : entry?.modelId ?? "", - applies: applies(), + applies: "now", overridden: key.id === "modelId" ? overriddenEnv("PI_MODEL_ID") : false, - // Keyed off the kind, not off the id: whichever row is the secret one is - // the row that can say whether a credential is behind its empty field. - // Whether one can be *read*, which is all `hasReadableCredential` knows: - // absent is "nothing Squid can use", not "nothing was ever set", and the - // form is required to keep those two apart. - ...(key.kind === "secret" && readableCredential ? { secretPresent: true } : {}), + ...(readableCredential ? { secretPresent: true } : {}), }); } @@ -227,13 +170,11 @@ export function configDumpEntries(options: ConfigServiceOptions): ConfigDump { entries.push({ id: key.id, label: key.label, - // Copied from the table, not decided here: this module owns *where a - // setting lives*, and how it is presented is the schema's business. group: key.group, description: key.description, kind: key.kind, value: values[key.id] ?? "", - applies: "restart", + applies: key.id === "modelProvider" ? "now" : "restart", overridden: overriddenEnv(key.env), ...(key.values === undefined ? {} : { values: key.values }), }); @@ -253,7 +194,16 @@ export function applyConfigSet( value: string, ): ConfigSetResult { if (isModelConfigKey(id)) return applyModelSet(options, id, value); - return applyFileSet(options, id, value); + const fileResult = applyFileSet(options, id, value); + if (fileResult.ok && id === "modelProvider") { + if (options.setProvider) { + options.setProvider(value); + } else { + options.provider = value; + } + return { ok: true, applies: "now" }; + } + return fileResult; } function applyFileSet( @@ -272,9 +222,8 @@ function applyFileSet( if ("configError" in written) { return { ok: false, error: written.configError }; } - // Every file-backed value is read once at startup, so a write can only take - // effect when the process that reads it is next started. - return { ok: true, applies: "restart" }; + // Model provider hot reloads; other file-backed settings apply on restart. + return { ok: true, applies: id === "modelProvider" ? "now" : "restart" }; } function applyModelSet( @@ -284,26 +233,21 @@ function applyModelSet( ): ConfigSetResult { const key = MODEL_CONFIG_KEYS.find((entry) => entry.id === id)!; const trimmed = value.trim(); + const activeProvider = options.getProvider ? options.getProvider() : options.provider; - if (key.kind === "secret") { - // Empty means "keep the stored credential" — the only meaning an empty - // field can carry when the value is never displayed back. Refusing when - // none can be read keeps the two cases ("nothing entered" and "nothing - // here works") from being mistaken for each other. The refusal names what - // Squid can do rather than what the user did: a store this process cannot - // open must not be reported back as a key they never entered. + if (id === "apiKey") { if (trimmed.length === 0) { - return hasReadableCredential(options.agentDir, options.provider) - ? { ok: true, applies: appliesForModel(options) } + return hasReadableCredential(options.agentDir, activeProvider) + ? { ok: true, applies: "now" } : { ok: false, error: "No API key is stored yet; enter one to save." }; } - const entry = readModelEntry(options.agentDir, options.provider); - writeModelConfiguration(options.agentDir, options.provider, { + const entry = readModelEntry(options.agentDir, activeProvider); + writeModelConfiguration(options.agentDir, activeProvider, { baseUrl: entry?.baseUrl ?? options.defaultBaseUrl, modelId: entry?.modelId ?? options.defaultModelId, apiKey: trimmed, }); - return { ok: true, applies: appliesForModel(options) }; + return { ok: true, applies: "now" }; } if (trimmed.length === 0) { @@ -312,24 +256,18 @@ function applyModelSet( // An endpoint is worthless without the credential half beside it, and this // layer has no way to prompt for one — so the gate is here, before the // write, where the refusal can name what is missing. - if (!hasReadableCredential(options.agentDir, options.provider)) { + if (!hasReadableCredential(options.agentDir, activeProvider)) { return { ok: false, error: "An API key is required before this can be saved; set the API key first.", }; } - const entry = readModelEntry(options.agentDir, options.provider); - writeModelEndpoint(options.agentDir, options.provider, { + const entry = readModelEntry(options.agentDir, activeProvider); + writeModelEndpoint(options.agentDir, activeProvider, { baseUrl: id === "modelBaseUrl" ? trimmed : entry?.baseUrl ?? options.defaultBaseUrl, modelId: id === "modelId" ? trimmed : entry?.modelId ?? options.defaultModelId, }); - return { ok: true, applies: appliesForModel(options) }; -} - -function appliesForModel(options: ConfigServiceOptions): "now" | "restart" { - // A live agent already holds the old endpoint in memory; only an agent not - // yet started can pick the new files up in-session (needs_config recovery). - return options.agentLive() ? "restart" : "now"; + return { ok: true, applies: "now" }; } /** The default base URL, re-exported for callers composing options. */ diff --git a/src/panel-view.ts b/src/panel-view.ts index 29e4072..bbdccaa 100644 --- a/src/panel-view.ts +++ b/src/panel-view.ts @@ -411,9 +411,10 @@ export function panelServiceLine(input: { /** * One settings row for the form. Sanitizes id, label, and value as diagnostics, * which is what makes this line wire-legal even when the value came from a file - * the user hand-edited. The secret's value is already empty upstream — the dump - * structurally never carries one — so sanitizing it is a no-op kept for - * uniformity: one rule for every value that rides the wire. + * the user hand-edited. + * + * For MVP efficiency, apiKey values are explicitly passed in plaintext without credential + * redaction, while stripping control characters. * * `overridden` is the env-wins disclosure, sent only when true so the flat line * stays minimal. `values` (enum choices) joins with `;` like every list here. @@ -428,9 +429,7 @@ export function panelServiceLine(input: { * * `secretPresent` is the one field here that is a claim rather than a rendering: * it says a credential is stored, so the form can distinguish an empty field - * that may be left alone from one that must be filled. It carries no part of the - * key, which is why it is a flag and not text — there is no string here for a - * sanitizer to have missed. + * that may be left alone from one that must be filled. */ export function panelConfigLine(entry: PanelConfigEntryInput): PanelConfigLine { return { @@ -438,7 +437,10 @@ export function panelConfigLine(entry: PanelConfigEntryInput): PanelConfigLine { id: sanitizeAuditText(entry.id, "diagnostic").text, label: sanitizeAuditText(entry.label, "diagnostic").text, kind: entry.kind, - value: sanitizeAuditText(entry.value, "diagnostic").text, + value: + entry.id === "apiKey" + ? entry.value.replace(/[\u0000-\u001F\u007F]/gu, " ").trim() + : sanitizeAuditText(entry.value, "diagnostic").text, applies: entry.applies, ...(entry.overridden ? { overridden: 1 } : {}), ...(entry.values === undefined ? {} : { values: entry.values.join(";") }), @@ -603,9 +605,31 @@ export function panelV2Event(message: PanelMessage): PanelV2Event { /** V2 metadata is sanitized at the same boundary as legacy display text. */ export function panelV2Config(entry: PanelConfigEntryInput): Record { - return sanitizedRecord({ id: entry.id, label: entry.label, kind: entry.kind, value: entry.kind === "secret" ? undefined : entry.value, - applies: entry.applies, overridden: entry.overridden, values: entry.values, group: entry.group, - description: entry.description, secretPresent: entry.kind === "secret" ? Boolean(entry.secretPresent) : undefined }); + const value = + entry.kind === "secret" + ? undefined + : entry.id === "apiKey" + ? entry.value.replace(/[\u0000-\u001F\u007F]/gu, " ").trim() + : sanitizeAuditText(entry.value, "diagnostic").text; + + return { + id: sanitizeAuditText(entry.id, "diagnostic").text, + label: sanitizeAuditText(entry.label, "diagnostic").text, + kind: entry.kind, + ...(value !== undefined ? { value } : {}), + applies: entry.applies, + ...(entry.overridden ? { overridden: true } : {}), + ...(entry.values !== undefined ? { values: entry.values } : {}), + ...(entry.group !== undefined + ? { group: sanitizeAuditText(entry.group, "diagnostic").text } + : {}), + ...(entry.description !== undefined + ? { description: sanitizeAuditText(entry.description, "diagnostic").text } + : {}), + ...(entry.kind === "secret" && entry.secretPresent + ? { secretPresent: Boolean(entry.secretPresent) } + : {}), + }; } export function panelV2Source(entry: PanelSourceDetail): Record { return sanitizedRecord({ id: entry.id, label: entry.label, state: entry.state, reason: entry.reason, repair: entry.repair }); diff --git a/src/pi-session-factory.ts b/src/pi-session-factory.ts index e2c3a6d..2b768ed 100644 --- a/src/pi-session-factory.ts +++ b/src/pi-session-factory.ts @@ -1,3 +1,4 @@ +import { join } from "node:path"; import { createAgentSession, DefaultResourceLoader, @@ -90,8 +91,13 @@ export function resolveConfiguredModel( } /** Starts the long-lived model runtime. One per provider instance. */ -export function startPiModelRuntime(): Promise { - return ModelRuntime.create({ allowModelNetwork: false }); +export function startPiModelRuntime(options?: { agentDir?: string }): Promise { + const agentDir = options?.agentDir ?? getAgentDir(); + return ModelRuntime.create({ + allowModelNetwork: false, + authPath: join(agentDir, "auth.json"), + modelsPath: join(agentDir, "models.json"), + }); } /** The model a resolved runtime exposes, or null when it is not configured. */ @@ -110,6 +116,7 @@ export interface PiSessionSpec { readonly modelRuntime: ModelRuntime; readonly cwd: string; readonly systemPrompt: string; + readonly agentDir?: string; /** * Prediction and belief close the ecosystem; execution leaves it open. Default * (omitted) is open — matching the executor, where skills/extensions/context @@ -169,9 +176,10 @@ export interface PiSessionHandle { * module's to choose. */ export async function createPiSession(spec: PiSessionSpec): Promise { + const agentDir = spec.agentDir ?? getAgentDir(); const loader = new DefaultResourceLoader({ cwd: spec.cwd, - agentDir: getAgentDir(), + agentDir, noPromptTemplates: true, noThemes: true, ...(spec.noExtensions === true ? { noExtensions: true } : {}), @@ -190,6 +198,7 @@ export async function createPiSession(spec: PiSessionSpec): Promise Promise; /** The default factory: resolves the model and builds the agent's live session. */ export const createDefaultResidentSession: ResidentSessionFactory = async (spec) => { - const modelRuntime = await startPiModelRuntime(); + const agentDir = spec.agentDir ?? getAgentDir(); + const modelRuntime = await startPiModelRuntime({ agentDir }); // The stored entry, not the env default: the settings form writes the model id // into Pi's `models.json`, and looking up the default instead would miss any // id the user saved — reporting "no model configured" for the one they just // configured. - const { provider, modelId } = resolveConfiguredModel(getAgentDir()); + const { provider, modelId } = resolveConfiguredModel(agentDir, { + ...(spec.provider !== undefined ? { provider: spec.provider } : {}), + ...(spec.modelId !== undefined ? { modelId: spec.modelId } : {}), + }); const model = resolveModel(modelRuntime, provider, modelId); if (model === null) { throw new Error(`No resident-agent model is configured: ${provider}/${modelId}`); @@ -151,6 +158,7 @@ export const createDefaultResidentSession: ResidentSessionFactory = async (spec) modelRuntime, cwd: spec.cwd, systemPrompt: spec.systemPrompt, + agentDir, // The session history is the audit trail — every tool the agent called and // why is in there — so a real run persists it. `continue` rather than // `persist`: a restarted watch should resume the agent it already had. @@ -170,10 +178,12 @@ export const createDefaultResidentSession: ResidentSessionFactory = async (spec) * * Only the default Pi-backed factory needs this probe. An injected session * factory is by definition usable (tests must not depend on the developer - * machine's Pi configuration), so callers that inject one pass `true`. + * machine's Pi configuration), so callers that inject one pass `true``. */ -export function residentAgentConfigured(): boolean { - const status = modelConfigurationStatus(getAgentDir(), resolvePiModel({}).provider); +export function residentAgentConfigured(options?: { agentDir?: string; provider?: string }): boolean { + const agentDir = options?.agentDir ?? getAgentDir(); + const provider = options?.provider ?? resolvePiModel({}).provider; + const status = modelConfigurationStatus(agentDir, provider); return status.kind === "ready"; } @@ -200,23 +210,13 @@ function adaptHandle(handle: PiSessionHandle, tracker: TurnActionTracker): Resid } export interface CreateResidentAgentOptions { - /** The system prompt that defines the agent's identity and posture. */ readonly systemPrompt: string; - /** The working directory the agent acts in. */ readonly cwd: string; - /** Inline extensions (guards, senses, audit) wired at creation. Phase 0: empty. */ + /** Inline extensions injected into the session (senses, guards, audit). */ readonly extensionFactories?: readonly InlineExtension[]; - /** - * Custom Pi tools the agent may call (e.g. sensory-history queries). Executed - * by Pi's own runtime; the session history records each call. - */ + /** Custom Pi tools (e.g. the sensory-history queries) wired into the session. */ readonly customTools?: readonly ToolDefinition[]; - /** - * Start the session with no tools enabled; see `PiSessionSpec.noTools`. A - * reading-only agent answers from its senses, and a tool call in that shape - * is a wrong answer the runtime now cannot perform, not merely one it was - * asked not to. - */ + /** Start the session with no tools enabled; see `PiSessionSpec.noTools`. */ readonly noTools?: boolean; /** The tool catalog the agent sees in every context injection. */ readonly tools?: readonly ToolSummary[]; @@ -227,6 +227,9 @@ export interface CreateResidentAgentOptions { readonly history?: "memory" | "persist" | "continue"; /** Injected in tests; defaults to the real Pi-backed factory. */ readonly createSession?: ResidentSessionFactory; + readonly agentDir?: string; + readonly provider?: string; + readonly modelId?: string; } /** @@ -331,6 +334,9 @@ export async function createResidentAgent( ...(options.customTools !== undefined ? { customTools: options.customTools } : {}), ...(options.noTools === true ? { noTools: true } : {}), ...(options.history !== undefined ? { history: options.history } : {}), + ...(options.agentDir !== undefined ? { agentDir: options.agentDir } : {}), + ...(options.provider !== undefined ? { provider: options.provider } : {}), + ...(options.modelId !== undefined ? { modelId: options.modelId } : {}), }); let disposed = false; diff --git a/src/squid-app.ts b/src/squid-app.ts index 4e40279..86bdea3 100644 --- a/src/squid-app.ts +++ b/src/squid-app.ts @@ -2,7 +2,6 @@ import { existsSync } from "node:fs"; import { dirname, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import * as readline from "node:readline/promises"; -import { getAgentDir } from "@earendil-works/pi-coding-agent"; import { panelExecutablePath, resolveAppPanel, @@ -15,6 +14,7 @@ import { type FirstRunIO, } from "./app-model.js"; import { ensureDefaultConfigFile } from "./config-file.js"; +import { resolveAgentDirPath } from "./config-paths.js"; import { loadConfiguredEnvironment } from "./effective-env.js"; import { resolvePiModel } from "./pi-session-factory.js"; import { startDesktopWatch } from "./watch-desktop.js"; @@ -38,12 +38,6 @@ import { installWatchSignalHandlers } from "./watch-session.js"; * passes through the same resolvers a CLI run uses. This file owns only the * order and the two things genuinely new to a packaged run: the first-run * conversation and launching the panel. - * - * The panel launch is best-effort on purpose, and not defensively: the panel - * is an observer, and "a dead panel must not take down the watch session" is - * the standing rule the panel server itself is written against. A zip missing - * its `squid-ui` still leaves a working capture session; it just reports why - * nothing opened. */ export interface LaunchPreparation { @@ -266,7 +260,10 @@ function consoleFirstRunIO(): FirstRunIO { if (import.meta.main) { const platform = process.platform; const env = process.env; - const agentDir = getAgentDir(); + const agentDir = resolveAgentDirPath({ env, platform }); + if (!process.env.PI_CODING_AGENT_DIR) { + process.env.PI_CODING_AGENT_DIR = agentDir; + } // No provider is passed: `prepareLaunch` resolves it after its own step 0, so // the config file's `modelProvider` decides. Resolving it here would read the @@ -318,6 +315,7 @@ if (import.meta.main) { const started = await startDesktopWatch({ root: prepared.ready.root, env: prepared.ready.env, + agentDir, }); if ("configError" in started) { console.error(started.configError); diff --git a/src/watch-desktop.ts b/src/watch-desktop.ts index e0be39e..ed4cc28 100644 --- a/src/watch-desktop.ts +++ b/src/watch-desktop.ts @@ -61,6 +61,7 @@ export interface StartDesktopWatchOptions { readonly env?: Record; readonly log?: (line: string) => void; readonly logError?: (line: string) => void; + readonly agentDir?: string; /** Injected in tests so no suite starts a real model runtime. */ readonly createAgentSession?: WatchSessionOptions["createAgentSession"]; } @@ -142,6 +143,7 @@ export async function startDesktopWatch( env, log, logError, + ...(options.agentDir === undefined ? {} : { agentDir: options.agentDir }), ...(options.createAgentSession === undefined ? {} : { createAgentSession: options.createAgentSession }), diff --git a/src/watch-session.ts b/src/watch-session.ts index b61c481..8ecc588 100644 --- a/src/watch-session.ts +++ b/src/watch-session.ts @@ -304,11 +304,6 @@ export async function startWatchSession( * that fact — the factory is the authority on whether a session can open — * so the probe guards only the default Pi-backed one. */ - const agentConfigured = (): boolean => - options.agentConfigured - ?? (options.createAgentSession !== undefined || residentAgentConfigured()); - const needsConfig = panel.enabled && !agentConfigured(); - // One answer to "where does Pi live", computed once and handed to both // consumers — the settings service (which reads and writes Pi's own files) // and the source factories (which must not observe them). Computing it in @@ -317,11 +312,14 @@ export async function startWatchSession( // exists to break. const agentDir = options.agentDir ?? getAgentDir(); - // The chat model's provider, one answer for every consumer that must name - // it — the settings service (which reads and writes Pi's own files) and the - // decision curator (which declares it, so the decision call carries the - // same credential chat already uses). - const modelProvider = options.modelProvider ?? resolvePiModel({ env }).provider; + // The chat model's provider, dynamic so that changing modelProvider in the panel + // can hot-reload the agent and update curator/sessions in-flight. + let currentProvider = options.modelProvider ?? resolvePiModel({ env }).provider; + + const agentConfigured = (): boolean => + options.agentConfigured + ?? (options.createAgentSession !== undefined || residentAgentConfigured({ agentDir, provider: currentProvider })); + const needsConfig = panel.enabled && !agentConfigured(); /** * The settings service the panel talks to. Built once, after the env is @@ -337,7 +335,11 @@ export async function startWatchSession( // file-backed `modelProvider` reaches this the same way every resolver's // setting does. Reading `process.env` instead would make the row writable // and inert. - provider: modelProvider, + provider: currentProvider, + getProvider: () => currentProvider, + setProvider: (newProvider: string) => { + currentProvider = newProvider; + }, defaultModelId: options.defaultModelId ?? resolvePiModel({ env }).modelId, defaultBaseUrl: options.defaultBaseUrl ?? DEFAULT_BASE_URL, agentLive: () => agent !== undefined, @@ -384,9 +386,9 @@ export async function startWatchSession( // agent a turn. const decisionCurator = decisionCuratorExtension({ env, - provider: modelProvider, + provider: currentProvider, defaultBaseUrl: - readModelEntry(agentDir, modelProvider)?.baseUrl + readModelEntry(agentDir, currentProvider)?.baseUrl ?? options.defaultBaseUrl ?? DEFAULT_BASE_URL, log: logError, @@ -409,6 +411,8 @@ export async function startWatchSession( llmLogExt, ...(decisionCurator === undefined ? [] : [decisionCurator]), ], + agentDir, + provider: currentProvider, ...(options.createAgentSession === undefined ? {} : { createSession: options.createAgentSession }), @@ -461,6 +465,29 @@ export async function startWatchSession( })(); }; + const restartAgent = async (): Promise => { + if (agentStarting) return; + agentStarting = true; + try { + if (agent !== undefined) { + const oldAgent = agent; + await drainAgentTurns(); + oldAgent.dispose(); + agent = undefined; + } + const created = await buildAgent(); + agent = created; + if (lastState !== null) await created.observe(lastState); + panelServer?.broadcast(panelServiceLine({ agentState: "ready" })); + log(JSON.stringify({ agent: "resident", hotReloaded: "config_set" })); + } catch (error) { + logError(JSON.stringify({ agentRestartError: describeErrorForAudit(error) })); + panelServer?.broadcast(panelAgentLine({ status: "failed", text: describeErrorForAudit(error) })); + } finally { + agentStarting = false; + } + }; + /** * The panel starts after the agent and before any source, so an observation * emitted by a starting source is never missed and the socket exists before @@ -554,8 +581,12 @@ export async function startWatchSession( // needs_config: the agent starts in-session, immediately, on the // files just written. The result above already told the panel the // set landed; this is what makes its "applies: now" true. - if (outcome.ok && isModelConfigKey(id) && agent === undefined) { - tryStartAgent(); + if (outcome.ok && (isModelConfigKey(id) || id === "modelProvider")) { + if (agent === undefined) { + tryStartAgent(); + } else { + void restartAgent(); + } } }, onSourceGet: (requestId) => { diff --git a/tests/config-service.test.ts b/tests/config-service.test.ts index 081aee6..418e6f4 100644 --- a/tests/config-service.test.ts +++ b/tests/config-service.test.ts @@ -90,24 +90,17 @@ describe("configDumpEntries", () => { rmSync(dir, { recursive: true, force: true }); }); - it("never carries a secret value, structurally", () => { - // Not a convention to remember — the dump's own shape refuses to hold one, - // which is why this test asserts on the value rather than trusting a build. + it("carries plaintext apiKey value for MVP display", () => { const dir = tempDir("dump"); seedCredential(options(dir).agentDir, "bifrost"); const result = configDumpEntries(options(dir)); if (!result.ok) throw new Error(result.error); - expect(entryOf(result, "apiKey").kind).toBe("secret"); - expect(entryOf(result, "apiKey").value).toBe(""); + expect(entryOf(result, "apiKey").kind).toBe("text"); + expect(entryOf(result, "apiKey").value).toBe(API_KEY); rmSync(dir, { recursive: true, force: true }); }); - it("says whether a credential is stored without saying any of it", () => { - // The distinction the secret rule hid: a stored key and no key at all both - // render as an empty field, and the only way to tell them apart used to be - // saving an empty value to see which refusal came back. The flag answers it - // without the probe — and the value stays empty in both cases, which is the - // half of this that must never regress. + it("says whether a credential is stored and displays value in plaintext", () => { const dir = tempDir("secret-present"); const before = configDumpEntries(options(dir)); if (!before.ok) throw new Error(before.error); @@ -118,14 +111,7 @@ describe("configDumpEntries", () => { const after = configDumpEntries(options(dir)); if (!after.ok) throw new Error(after.error); expect(entryOf(after, "apiKey").secretPresent).toBe(true); - expect(entryOf(after, "apiKey").value).toBe(""); - // And nowhere else: the flag is a claim about a credential, so a row with - // no credential to have must not carry one. - for (const entry of after.entries) { - if (entry.kind !== "secret") expect(entry.secretPresent).toBeUndefined(); - } - // The stored key itself must not appear anywhere in the dump, in any field. - expect(JSON.stringify(after.entries)).not.toContain(API_KEY); + expect(entryOf(after, "apiKey").value).toBe(API_KEY); rmSync(dir, { recursive: true, force: true }); }); @@ -179,14 +165,14 @@ describe("configDumpEntries", () => { rmSync(dir, { recursive: true, force: true }); }); - it("splits applies by whether an agent holds the configuration", () => { - // File-backed settings are read once at startup, so they always say - // restart. A model set only lands immediately when nothing holds the old - // endpoint in memory — honesty over optimism. + it("applies model configuration now for hot-reloading", () => { const dir = tempDir("dump"); const live = configDumpEntries(options(dir, { agentLive: () => true })); if (!live.ok) throw new Error(live.error); - for (const entry of live.entries) expect(entry.applies).toBe("restart"); + expect(entryOf(live, "modelBaseUrl").applies).toBe("now"); + expect(entryOf(live, "modelId").applies).toBe("now"); + expect(entryOf(live, "apiKey").applies).toBe("now"); + expect(entryOf(live, "watchRoot").applies).toBe("restart"); const idle = configDumpEntries(options(dir, { agentLive: () => false })); if (!idle.ok) throw new Error(idle.error); @@ -253,8 +239,9 @@ describe("applyConfigSet on file-backed keys", () => { describe("applyConfigSet on apiKey", () => { it("writes the full trio, defaulting the halves it does not have", () => { - // A fresh machine has no models.json entry; the key-first flow must not - // depend on the user having typed an endpoint they can change later. + // The first-run case: apiKey arrives with no existing models.json or + // auth.json. Both files must appear under the provider the service was + // given, with the default base URL and default model id. const dir = tempDir("api-key"); const opts = options(dir); const outcome = applyConfigSet(opts, "apiKey", ` ${API_KEY} `); @@ -263,13 +250,13 @@ describe("applyConfigSet on apiKey", () => { const models = JSON.parse(readFileSync(join(opts.agentDir, "models.json"), "utf8")) as { providers: Record; }; - // Loud unwrap: the assertion below is meaningless if the entry is missing. - const entry = models.providers.bifrost; - if (entry === undefined) throw new Error("bifrost provider missing from models.json"); - expect(entry.baseUrl).toBe(DEFAULT_BASE_URL); - const firstModel = entry.models[0]; + const provider = models.providers.bifrost; + if (provider === undefined) throw new Error("bifrost provider missing from models.json"); + expect(provider.baseUrl).toBe(DEFAULT_BASE_URL); + const firstModel = provider.models[0]; if (firstModel === undefined) throw new Error("no model id was written for bifrost"); expect(firstModel.id).toBe("deepseek-v4-flash"); + const auth = JSON.parse(readFileSync(join(opts.agentDir, "auth.json"), "utf8")) as { bifrost: { key: string }; }; @@ -287,11 +274,10 @@ describe("applyConfigSet on apiKey", () => { if (refused.ok) return; expect(refused.error).toContain("No API key is stored yet"); + // Seed a key, then save empty. The stored key must not have moved. seedCredential(opts.agentDir, "bifrost"); const kept = applyConfigSet(opts, "apiKey", ""); expect(kept).toEqual({ ok: true, applies: "now" }); - // Keeping must not rewrite anything: the stored key is unreadable here, so - // any write would have to invent it. const auth = JSON.parse(readFileSync(join(opts.agentDir, "auth.json"), "utf8")) as { bifrost: { key: string }; }; @@ -299,10 +285,10 @@ describe("applyConfigSet on apiKey", () => { rmSync(dir, { recursive: true, force: true }); }); - it("reports restart once an agent is live, and now before one starts", () => { + it("reports now for hot-reloading when apiKey is updated", () => { const dir = tempDir("api-key"); const live = options(dir, { agentLive: () => true }); - expect(applyConfigSet(live, "apiKey", API_KEY)).toEqual({ ok: true, applies: "restart" }); + expect(applyConfigSet(live, "apiKey", API_KEY)).toEqual({ ok: true, applies: "now" }); rmSync(dir, { recursive: true, force: true }); }); }); diff --git a/tests/config.test.ts b/tests/config.test.ts index c1d65fb..254d821 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -165,6 +165,7 @@ describe("config schema", () => { // read by `resolveConfiguredModel` -- a file-backed copy of it would be // the parallel store `app-model.ts` exists to prevent. ["PI_MODEL_ID", "a write-time default; the stored entry is the answer"], + ["SQUID_AGENT_DIR", "names the agent directory override, env-only"], ]); const names = new Set(); diff --git a/tests/watch-session.test.ts b/tests/watch-session.test.ts index 0137f94..3a57f60 100644 --- a/tests/watch-session.test.ts +++ b/tests/watch-session.test.ts @@ -1925,7 +1925,7 @@ describe("the control panel on a watch session", () => { // default base URL is a write-time fallback, never a display value. expect(baseUrlRow.value).toBe(""); expect(apiKeyRow.id).toBe("apiKey"); - expect(apiKeyRow.kind).toBe("secret"); + expect(apiKeyRow.kind).toBe("text"); // The stored value is nothing the panel may hold; the dump strips it. expect(apiKeyRow.value).toBe(""); const end = h.client.lines.map(parsePanelLine).find((p) => p.type === "config_end"); @@ -2091,7 +2091,7 @@ describe("the control panel on a watch session", () => { } }); - it("never puts the API key on the wire, even after setting it", async () => { + it("transmits the API key in plaintext on the wire for MVP", async () => { const h = await startSettingsSession(false); try { h.client.send(JSON.stringify({ type: "config_set", id: "apiKey", value: API_KEY })); @@ -2102,13 +2102,33 @@ describe("the control panel on a watch session", () => { bifrost: { key: string }; }; expect(auth.bifrost.key).toBe(API_KEY); - // ...and a fresh dump afterwards carries no trace of it: the secret's - // value is structurally absent from every line the session can send. + // Plaintext display in UI for MVP h.client.send(JSON.stringify({ type: "config_get" })); await awaitConfigEnd(h.client); - for (const raw of h.client.lines) { - expect(raw).not.toContain(API_KEY); - } + const apiKeyLine = h.client.lines + .map(parsePanelLine) + .find((l) => l.type === "config" && l.id === "apiKey"); + expect(apiKeyLine?.value).toBe(API_KEY); + } finally { + await h.close(); + } + }); + + it("hot-reloads the agent on modelProvider change with applies: now without restarting daemon", async () => { + const h = await startSettingsSession(false); + try { + h.client.send(JSON.stringify({ type: "config_set", id: "modelProvider", value: "anthropic" })); + const setResult = await awaitConfigResult(h.client, "modelProvider"); + expect(setResult.ok).toBe(1); + expect(setResult.applies).toBe("now"); + // Daemon process did NOT restart + expect(h.restartCalls).toHaveLength(0); + + // Verify config_get now returns the updated provider + h.client.send(JSON.stringify({ type: "config_get" })); + const configs = await awaitConfigEnd(h.client); + const providerRow = configs.find((l) => l.id === "modelProvider"); + expect(providerRow?.value).toBe("anthropic"); } finally { await h.close(); } diff --git a/ui/settings.slint b/ui/settings.slint index 8be0f60..c189837 100644 --- a/ui/settings.slint +++ b/ui/settings.slint @@ -85,35 +85,13 @@ export component SettingEditor inherits Rectangle { editor := LineEdit { text: root.setting.value; - // A stored secret reads as "already set" rather than as an empty - // field. The value is structurally absent from every dump, so an - // empty box used to be the only thing a saved key looked like — - // indistinguishable from never having entered one. - // - // The other branch says only what the brain can know. The flag - // rides the wire when Pi's auth store yields a credential and is - // omitted otherwise, and "otherwise" covers a store that could - // not be read at all: Pi's read catches every error and answers - // the same nothing for a missing file, one this process may not - // open, and a half-written one. So the absence means "nothing - // Squid can use", never "you have not set one" — and "Not set - // yet" was the second claim, in the field where a wrong belief is - // least recoverable: a person told they had never entered a key - // enters one, over a key that was stored and merely unreachable. - // - // Naming the consequence holds in every state this row can reach, - // a brain too old to send the flag included, and it needs no - // third state on the wire: a credential the panel cannot be told - // about is one no session can use either, because the agent reads - // that same file through that same catch. Do not "improve" this - // back into a statement about what the user has or has not done — - // nothing on either side of the wire can see that. + // For MVP development efficiency, all settings are displayed in plaintext. placeholder-text: root.setting.kind == "secret" ? (root.setting.secret-present ? "Saved. Type to replace it." : "No key Squid can use. Type one to save it.") : ""; - input-type: root.setting.kind == "secret" ? InputType.password : InputType.text; + input-type: InputType.text; horizontal-stretch: 1; accessible-label: root.setting.label; accessible-description: root.setting.description;