diff --git a/README.details.md b/README.details.md index bab241d..94ba4b3 100644 --- a/README.details.md +++ b/README.details.md @@ -245,6 +245,41 @@ and at most every 30 seconds of tool activity; watch daemon output for individua live actions. Historical records without timing, or closed/changed Issues whose stop has not been reconciled, show unavailable timing rather than zero. +#### Repairing Roc settings + +`ROC_SETTINGS_INVALID` names the Roc settings location, normally +`~/.config/roc/settings.json`, and distinguishes missing files, invalid JSON, +unsupported fields, invalid cycle/settings data, and read failures. + +- **Missing file:** run `npx roc-it@latest onboard` under the intended OS account. +- **Cannot read:** check that the named path is a regular file, its ownership and + read permissions, and access to its parent directories. Fix access for the + intended account; do not make credentials or settings world-readable. +- **Invalid content:** back up the exact file before editing it locally. For + example, `cp -ip ~/.config/roc/settings.json ~/.config/roc/settings.json.bak` + preserves permissions and asks before overwriting an existing backup; choose + another backup name if one already exists. Keep backups private and do not + paste settings or credentials into Issues or logs. + +Repair JSON syntax first, then review unsupported fields and invalid data against +this version's format. The minimal weekly configuration is +`{"cycle":{"type":"weekly"}}`; daily uses `"daily"`, and custom uses +`{"cycle":{"type":"custom","days":14,"anchorDate":"2026-08-28"}}` with positive +whole days and a real `YYYY-MM-DD` calendar date. Optional `skills.allowlist` is an +array of `{ "name": "…", "source": "…" }` identities with nonempty strings; +`execution.allowUnsandboxed` is a boolean. Optional top-level `models` supports +only `luna`, `terra` and `sol`, each a `provider/modelId` string as described below. +Do not remove a valid `models` mapping. Other fields, including nested extras, +are rejected; review and manually correct misplaced/unsupported fields rather +than blindly deleting them or replacing the entire file with the minimal example. +Diagnostics show at most three fixed public field names and a count for hidden +names, never arbitrary unknown names or configuration values. + +Roc does not rewrite invalid files, and **onboarding reads the same file and +cannot repair it**. After manual repair, retry the failed command (for example, +`npx roc-it@latest task board`). These are Roc settings, not Pi's separate +`~/.pi/agent/settings.json` or `~/.pi/agent/auth.json`; leave credentials untouched. + ### Pi provider setup Onboarding reuses Pi credentials or opens ChatGPT browser authorization. Follow diff --git a/src/settings.ts b/src/settings.ts index 6a0d930..3a709c5 100644 --- a/src/settings.ts +++ b/src/settings.ts @@ -1,6 +1,7 @@ import { readFile, writeFile } from "node:fs/promises"; import { homedir } from "node:os"; import { join } from "node:path"; +import { ZodError } from "zod"; import { type RocSettings, RocSettingsSchema } from "./domain/agile-cycle"; import { AgileError } from "./runtime/errors"; import { prepareSafeFilePath } from "./runtime/safe-file"; @@ -15,29 +16,105 @@ function isMissingSettings(error: unknown): boolean { return error instanceof Error && "code" in error && error.code === "ENOENT"; } -/** Wraps one settings read or validation failure in Roc's stable startup error. */ -function invalidSettings(error: unknown): AgileError { +// Only fixed public schema names may appear in diagnostics, never arbitrary keys. +const publicFieldNames = new Set([ + "cycle", + "type", + "days", + "anchorDate", + "skills", + "allowlist", + "name", + "source", + "execution", + "allowUnsandboxed", + "models", + "luna", + "terra", + "sol", +]); +const repairGuidance = + "Back up this file, then repair it manually using the supported settings format in README.details.md (Progress and recovery) and retry; onboarding reads the same file and cannot repair it."; + +/** Wraps a safe location-specific diagnostic in Roc's stable startup error. */ +function invalidSettings(path: string, diagnostic: string): AgileError { return new AgileError({ code: "ROC_SETTINGS_INVALID", category: "startup", retryable: false, component: "settings", - message: "Run npx roc-it@latest onboard to configure an Agile cycle", - cause: error, + message: `Roc settings at ${path}: ${diagnostic}`, }); } +/** Summarizes schema failures with bounded public names and no values or raw errors. */ +function validationDiagnostic(error: unknown): string { + if (!(error instanceof ZodError)) + return "Invalid settings data; check cycle data, including calendar dates."; + const names = new Set(); + let hidden = 0; + let invalidCycle = false; + let invalidOther = false; + for (const issue of error.issues) { + const keys = + issue.code === "unrecognized_keys" + ? issue.keys + : issue.code === "invalid_key" + ? [issue.path.at(-1)] + : undefined; + if (keys) { + for (const key of keys) { + if (typeof key === "string" && publicFieldNames.has(key)) + names.add(key); + else hidden++; + } + } else if (issue.path[0] === "cycle") invalidCycle = true; + else invalidOther = true; + } + const shown = [...names].sort().slice(0, 3); + hidden += names.size - shown.length; + if (hidden) shown.push(`${hidden} other field name(s) hidden`); + return [ + shown.length ? `Unsupported fields: ${shown.join(", ")}.` : "", + invalidCycle + ? "Invalid cycle data; use daily, weekly, or custom with positive whole days and a valid anchorDate (YYYY-MM-DD)." + : "", + invalidOther + ? "Invalid settings data; check the supported settings types and structure." + : "", + ] + .filter(Boolean) + .join(" "); +} + /** Loads strict settings for repeat onboarding, returning undefined only when absent. */ export async function loadRocSettingsIfPresent( homeRoot = homedir(), ): Promise { + const path = rocSettingsPath(homeRoot); + let source: string; try { - return RocSettingsSchema.parse( - JSON.parse(await readFile(rocSettingsPath(homeRoot), "utf8")), - ); + source = await readFile(path, "utf8"); } catch (error) { if (isMissingSettings(error)) return undefined; - throw invalidSettings(error); + throw invalidSettings( + path, + "Could not read settings. Check that the path is a regular file and that your account has file and parent-directory permissions; retry after fixing access.", + ); + } + let input: unknown; + try { + input = JSON.parse(source); + } catch { + throw invalidSettings(path, `Invalid JSON. ${repairGuidance}`); + } + try { + return RocSettingsSchema.parse(input); + } catch (error) { + throw invalidSettings( + path, + `${validationDiagnostic(error)} ${repairGuidance}`, + ); } } @@ -47,7 +124,10 @@ export async function loadRocSettings( ): Promise { const settings = await loadRocSettingsIfPresent(homeRoot); if (settings !== undefined) return settings; - throw invalidSettings(new Error("Roc settings do not exist")); + throw invalidSettings( + rocSettingsPath(homeRoot), + "Settings file is missing. Run npx roc-it@latest onboard to configure an Agile cycle", + ); } /** Validates and safely writes Roc's global settings. */ diff --git a/test/cli/github-commands.test.ts b/test/cli/github-commands.test.ts index 1c3637e..f9e720e 100644 --- a/test/cli/github-commands.test.ts +++ b/test/cli/github-commands.test.ts @@ -5,7 +5,7 @@ import { join } from "node:path"; import { runCli } from "../../src/cli/run"; import type { CliRuntime, SchedulerRunInput } from "../../src/cli/types"; import { githubTaskSnapshot } from "../../src/github/execution-view"; -import { saveRocSettings } from "../../src/settings"; +import { rocSettingsPath, saveRocSettings } from "../../src/settings"; import { memoryGitHub } from "../helpers/github-native"; test("public task reads use GitHub, preserve legacy files, and scheduler rejects the removed local queue", async () => { @@ -86,6 +86,61 @@ test("public task reads use GitHub, preserve legacy files, and scheduler rejects } }); +test("task board explains invalid settings before remote reads and opens after manual repair", async () => { + const root = await mkdtemp(join(tmpdir(), "roc-board-settings-")); + const remote = memoryGitHub(); + const output: string[] = []; + const errors: string[] = []; + let reads = 0; + const runtime: CliRuntime = { + projectRoot: root, + homeRoot: root, + now: () => new Date("2026-09-08T00:00:00Z"), + async runScheduler() {}, + async readTasks() { + reads++; + const data = await remote.store().list(); + return githubTaskSnapshot(data.tasks, data.diagnostics); + }, + }; + const io = { + out: (line: string) => output.push(line), + err: (line: string) => errors.push(line), + }; + try { + const path = await saveRocSettings({ cycle: { type: "weekly" } }, root); + // Top-level models is supported; type is unsupported at the settings root. + const invalid = Buffer.from( + '{"cycle":{"type":"weekly"},"type":"SECRET_VALUE"}\n', + ); + await writeFile(path, invalid); + expect(await runCli(["task", "board"], io, runtime)).toBe(1); + expect(reads).toBe(0); + expect(output).toEqual([]); + const diagnostic = errors.join("\n"); + expect(diagnostic).toContain("ROC_SETTINGS_INVALID"); + expect(diagnostic).toContain(rocSettingsPath(root)); + expect(diagnostic).toContain("Unsupported fields: type."); + expect(diagnostic).toContain("Back up this file"); + expect(diagnostic).not.toContain("Run npx roc-it@latest onboard"); + expect(diagnostic).not.toContain("SECRET_VALUE"); + expect(await readFile(path)).toEqual(invalid); + + await saveRocSettings( + { cycle: { type: "weekly" }, models: { luna: "provider/model" } }, + root, + ); + errors.length = 0; + expect(await runCli(["task", "board"], io, runtime)).toBe(0); + expect(reads).toBe(1); + expect(output.join("\n")).toContain("GitHub checkpoints"); + expect(output.join("\n")).toContain("Return 42"); + expect(errors).toEqual([]); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + test("task acceptance reads recorded criteria without mutating GitHub state", async () => { const root = await mkdtemp(join(tmpdir(), "roc-task-acceptance-")); const remote = memoryGitHub(); diff --git a/test/cli/run.test.ts b/test/cli/run.test.ts index 4138f7d..8d52b80 100644 --- a/test/cli/run.test.ts +++ b/test/cli/run.test.ts @@ -892,13 +892,56 @@ test("cycle current explains how to create missing settings", async () => { ), ).toBe(1); expect(errors).toEqual([ - "Run npx roc-it@latest onboard to configure an Agile cycle", + `Roc settings at ${rocSettingsPath(homeRoot)}: Settings file is missing. Run npx roc-it@latest onboard to configure an Agile cycle`, ]); } finally { await rm(homeRoot, { recursive: true, force: true }); } }); +test("onboarding preserves invalid settings and requests manual repair before model setup", async () => { + const root = await mkdtemp(join(tmpdir(), "roc-onboard-invalid-")); + const home = await mkdtemp(join(tmpdir(), "roc-onboard-invalid-home-")); + let modelCalls = 0; + try { + const path = await saveRocSettings({ cycle: { type: "weekly" } }, home); + for (const source of [ + '{"cycle":{"type":"weekly"},"type":"SECRET_VALUE"}\n', + '{"cycle":{"type":"SECRET_VALUE",', + ]) { + const before = Buffer.from(source); + await writeFile(path, before); + const { io, output, errors } = interactiveIo(["2"]); + expect( + await runCli( + ["onboard"], + io, + onboardingRuntime({ + projectRoot: root, + homeRoot: home, + configureModel: async () => { + modelCalls++; + return "provider/model"; + }, + }), + ), + ).toBe(1); + expect(modelCalls).toBe(0); + expect(await readFile(path)).toEqual(before); + expect(errors.join("\n")).toContain(rocSettingsPath(home)); + expect(errors.join("\n")).toContain("Back up this file"); + expect(errors.join("\n")).toContain( + "onboarding reads the same file and cannot repair it", + ); + expect(errors.join("\n")).not.toContain("SECRET_VALUE"); + expect(output.join("\n")).not.toContain("Result: Complete"); + } + } finally { + await rm(root, { recursive: true, force: true }); + await rm(home, { recursive: true, force: true }); + } +}); + test("execution refusal and model failure preserve Roc settings and never report ready", async () => { for (const allow of [false, true]) { const root = await mkdtemp(join(tmpdir(), "roc-onboard-failed-")); diff --git a/test/settings.test.ts b/test/settings.test.ts index 6a93e09..8cac00e 100644 --- a/test/settings.test.ts +++ b/test/settings.test.ts @@ -4,12 +4,14 @@ import { mkdtemp, readFile, realpath, + rm, symlink, writeFile, } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { RocSettingsSchema } from "../src/domain/agile-cycle"; +import { AgileError } from "../src/runtime/errors"; import { loadRocSettings, loadRocSettingsIfPresent, @@ -59,9 +61,7 @@ test("saves and loads strict global settings", async () => { path, '{"cycle":{"type":"custom","days":0,"anchorDate":"2026-08-28"}}', ); - await expect(loadRocSettings(homeRoot)).rejects.toThrow( - "Run npx roc-it@latest onboard", - ); + await expect(loadRocSettings(homeRoot)).rejects.toThrow("Invalid cycle data"); }); test("refuses a symbolic-link settings directory", async () => { @@ -121,6 +121,156 @@ test("loads repeat-onboarding settings only when present", async () => { '{"cycle":{"type":"weekly"},"skills":{"allowlist":[{"name":"","source":"backnotprop/pstack"}]}}', ); await expect(loadRocSettingsIfPresent(homeRoot)).rejects.toThrow( - "Run npx roc-it@latest onboard", + "Invalid settings data", ); }); + +test("missing settings recommend onboarding with stable error metadata", async () => { + const home = await mkdtemp(join(tmpdir(), "roc-settings-missing-")); + try { + expect(await loadRocSettingsIfPresent(home)).toBeUndefined(); + await expect(loadRocSettings(home)).rejects.toMatchObject({ + code: "ROC_SETTINGS_INVALID", + category: "startup", + retryable: false, + component: "settings", + message: `Roc settings at ${rocSettingsPath(home)}: Settings file is missing. Run npx roc-it@latest onboard to configure an Agile cycle`, + }); + } finally { + await rm(home, { recursive: true, force: true }); + } +}); + +test("invalid settings give bounded, secret-safe repair guidance without changing bytes", async () => { + const home = await mkdtemp(join(tmpdir(), "roc-settings-diagnostics-")); + const secret = "SECRET_VALUE_DO_NOT_PRINT"; + const secretKey = "SECRET_KEY_DO_NOT_PRINT"; + const cases = [ + { source: `{"${secretKey}":"${secret}",`, diagnostic: "Invalid JSON." }, + { + source: JSON.stringify({ cycle: { type: "weekly" }, type: secret }), + diagnostic: "Unsupported fields: type.", + }, + { + source: JSON.stringify({ + cycle: { type: "weekly" }, + models: { models: secret }, + }), + diagnostic: "Unsupported fields: models.", + }, + { + source: JSON.stringify({ + cycle: { type: "custom", days: 0, anchorDate: "2026-02-30" }, + }), + diagnostic: "Invalid cycle data", + }, + { + source: JSON.stringify({ cycle: { type: secret } }), + diagnostic: "Invalid cycle data", + }, + { + source: JSON.stringify({ + cycle: { type: "custom", days: 1, anchorDate: secret }, + }), + diagnostic: "Invalid settings data; check cycle data", + }, + { + source: JSON.stringify({ + cycle: { type: "weekly" }, + execution: { allowUnsandboxed: secret }, + }), + diagnostic: "Invalid settings data", + }, + { + source: JSON.stringify({ + cycle: { type: "weekly" }, + skills: { + allowlist: [{ name: "", source: secret, [secretKey]: secret }], + }, + }), + diagnostic: + "Unsupported fields: 1 other field name(s) hidden. Invalid settings data", + }, + { + source: JSON.stringify({ + cycle: { type: "weekly" }, + models: { [secretKey]: secret, luna: secret }, + }), + diagnostic: + "Unsupported fields: 1 other field name(s) hidden. Invalid settings data", + }, + { + source: JSON.stringify({ + cycle: { type: "weekly" }, + type: secret, + days: secret, + anchorDate: secret, + allowlist: secret, + ...Object.fromEntries( + Array.from({ length: 100 }, (_, index) => [ + `${secretKey}_${index}`, + secret, + ]), + ), + }), + diagnostic: + "Unsupported fields: allowlist, anchorDate, days, 101 other field name(s) hidden.", + }, + ]; + try { + const path = await saveRocSettings({ cycle: { type: "weekly" } }, home); + for (const { source, diagnostic } of cases) { + const bytes = Buffer.from(` \n${source}\n`); + await writeFile(path, bytes); + for (const load of [loadRocSettings, loadRocSettingsIfPresent]) { + const error = await load(home).catch((error: unknown) => error); + expect(error).toBeInstanceOf(AgileError); + if (!(error instanceof AgileError)) + throw new Error("Expected settings error"); + expect(error.code).toBe("ROC_SETTINGS_INVALID"); + expect(error.message).toContain(rocSettingsPath(home)); + expect(error.message).toContain(diagnostic); + expect(error.message).toContain( + "Back up this file, then repair it manually", + ); + expect(error.message).toContain( + "onboarding reads the same file and cannot repair it", + ); + expect(error.message).not.toContain("Run npx roc-it@latest onboard"); + expect(error.message).not.toContain("Settings file is missing"); + expect(error.message.length).toBeLessThan(path.length + 600); + expect(Bun.inspect(error)).not.toContain(secret); + expect(Bun.inspect(error)).not.toContain(secretKey); + expect(error.cause).toBeUndefined(); + expect(await readFile(path)).toEqual(bytes); + } + } + } finally { + await rm(home, { recursive: true, force: true }); + } +}); + +test("both loaders reject unreadable settings with file and permission checks", async () => { + const home = await mkdtemp(join(tmpdir(), "roc-settings-read-failure-")); + const path = rocSettingsPath(home); + try { + // A directory fails deterministically even when tests have elevated privileges. + await mkdir(path, { recursive: true }); + for (const load of [loadRocSettings, loadRocSettingsIfPresent]) { + const error = await load(home).catch((error: unknown) => error); + expect(error).toBeInstanceOf(AgileError); + if (!(error instanceof AgileError)) + throw new Error("Expected settings error"); + expect(error.code).toBe("ROC_SETTINGS_INVALID"); + expect(error.message).toContain(path); + expect(error.message).toContain("Could not read settings"); + expect(error.message).toContain("regular file"); + expect(error.message).toContain("permissions"); + expect(error.message).not.toContain("onboard"); + expect(error.message).not.toContain("EISDIR"); + expect(error.cause).toBeUndefined(); + } + } finally { + await rm(home, { recursive: true, force: true }); + } +});