diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index cf1726279c..7ba48cab65 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,11 +2,16 @@ ## [Unreleased] -## [0.12.15] - 2026-08-06 +### Added + +- Workflow settings (ralplan `maxIterations`/`autoHandoff`/`maxReviewPassesPerLane`, deep-interview `ambiguityThreshold`, ultragoal `nudgeBudget`) now resolve through one shared five-layer precedence: project `.gjc/config.yml` → project `.gjc/settings.json` → user `/config.yml` → legacy `/settings.json` → built-in default. `config.yml` values previously ignored by the workflow runtimes now take effect (`gjc config set gjc.ralplan.maxIterations 7` is honored by ralplan); project configuration beats user configuration, fixing deep-interview's former user-YAML-first inversion. -## [0.12.14] - 2026-08-06 +### Changed -## [0.12.13] - 2026-08-06 +- Registered `gjc.ultragoal.nudgeBudget` in the public settings schema (default 10, non-negative integer). +- The legacy config-root `~/.gjc/settings.json` workflow keys are migrated once into `~/.gjc/agent/config.yml` on the next default-global-scope load (absent-only, atomic marker, no-clobber `.bak`). Invalid strict ralplan legacy values keep the source active so `gjc ralplan` still fails loudly (exit 2); future-schema `config.yml` targets are never touched. +- ralplan settings are strict for all three keys: malformed or invalid explicit settings in any layer/format exit 2 (the former silent `maxIterations` fallback is removed). An invalid strict ralplan value in the target `config.yml` is repaired with a valid legacy value during migration. +- `config.yml` settings use the nested schema form; flat dotted keys are honored only in legacy `settings.json` files so every effective override stays manageable via `Settings`/`gjc config`. ### Fixed - ACP session configuration now emits the spec-defined `category` field on the Mode, Model, and Thinking select options (`mode`, `model`, `thought_level`), so standards-compliant ACP clients such as Paseo discover models, modes, and thinking levels instead of an empty model picker (#3922). diff --git a/packages/coding-agent/src/config/atomic-yaml-patch.ts b/packages/coding-agent/src/config/atomic-yaml-patch.ts index 6d32d3a8bb..8d018a12e5 100644 --- a/packages/coding-agent/src/config/atomic-yaml-patch.ts +++ b/packages/coding-agent/src/config/atomic-yaml-patch.ts @@ -49,7 +49,8 @@ export interface AtomicYamlPatchRevision { export type CasRestoreResult = | { status: "restored"; receipt: CasReceipt } | { status: "conflict"; paths: readonly string[] } - | { status: "discarded" }; + | { status: "discarded" } + | { status: "not-restorable" }; /** * A receipt intentionally exposes only path-level hashes and opaque revisions. @@ -214,14 +215,17 @@ export function atomicYamlPathHash(value: Record, path: string) type YamlReadResult = { current: Record; root: unknown; + /** Raw file content ("" when the file is absent); the CAS basis for writes. */ + raw: string; }; async function readYaml(configPath: string): Promise { try { - const root = YAML.parse(await fs.readFile(configPath, "utf8")); - return { current: record(root) ?? {}, root }; + const raw = await Bun.file(configPath).text(); + const root = YAML.parse(raw); + return { current: record(root) ?? {}, root, raw }; } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return { current: {}, root: undefined }; + if ((error as NodeJS.ErrnoException).code === "ENOENT") return { current: {}, root: undefined, raw: "" }; throw error; } } @@ -240,7 +244,15 @@ async function syncParentDirectory(directory: string): Promise { } } -async function replaceWithRetry(tempPath: string, configPath: string, options: AtomicYamlPatchOptions): Promise { +async function replaceWithRetry( + tempPath: string, + configPath: string, + options: AtomicYamlPatchOptions, + /** Re-verify the target content before EVERY rename attempt (an external + * save between retries on a Windows sharing-violation backoff must not be + * overwritten by a later retry). */ + expectedRaw?: string, +): Promise { const rename = options.rename ?? fs.rename; const sleep = options.sleep ?? (async (delay: number): Promise => await Bun.sleep(delay)); const isWindows = (options.platform ?? process.platform) === "win32"; @@ -248,6 +260,21 @@ async function replaceWithRetry(tempPath: string, configPath: string, options: A for (;;) { attempts++; + if (expectedRaw !== undefined) { + const currentRaw = await Bun.file(configPath) + .text() + .catch((error: unknown) => { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return ""; + throw error; + }); + if (currentRaw !== expectedRaw) { + throw new AtomicYamlConflictError( + configPath, + createHash("sha256").update(expectedRaw).digest("hex"), + createHash("sha256").update(currentRaw).digest("hex"), + ); + } + } try { await rename(tempPath, configPath); return; @@ -269,7 +296,9 @@ async function writeAtomicYaml( configPath: string, value: Record, options: AtomicYamlPatchOptions, -): Promise { + /** Expected current content; re-verified immediately before the rename. */ + expectedRaw?: string, +): Promise { const directory = path.dirname(configPath); const tempPath = path.join(directory, `.${path.basename(configPath)}.${process.pid}.${randomUUID()}.tmp`); try { @@ -280,11 +309,30 @@ async function writeAtomicYaml( } finally { await tempHandle.close(); } - await replaceWithRetry(tempPath, configPath, options); + if (expectedRaw !== undefined) { + // The transaction's CAS guard ran before this write; re-verify right + // before the rename so an external save during the temp write cannot + // be silently overwritten by the replacement. + const currentRaw = await Bun.file(configPath) + .text() + .catch((error: unknown) => { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return ""; + throw error; + }); + if (currentRaw !== expectedRaw) { + throw new AtomicYamlConflictError( + configPath, + createHash("sha256").update(expectedRaw).digest("hex"), + createHash("sha256").update(currentRaw).digest("hex"), + ); + } + } + await replaceWithRetry(tempPath, configPath, options, expectedRaw); await syncParentDirectory(directory); } finally { await fs.rm(tempPath, { force: true }).catch(() => undefined); } + return YAML.stringify(value, null, 2); } function createReceipt( @@ -334,6 +382,8 @@ async function applyPatchesUnderLock( current: Record, patches: readonly AtomicYamlPatch[], options: AtomicYamlPatchOptions, + skipWrite = false, + expectedRaw?: string, ): Promise { if (patches.length === 0) return createReceipt(configPath, [], options); @@ -373,7 +423,7 @@ async function applyPatchesUnderLock( } const changes = [...changesByPath.values()]; - await writeAtomicYaml(configPath, current, options); + if (!skipWrite) await writeAtomicYaml(configPath, current, options, expectedRaw); return createReceipt(configPath, changes, options); } @@ -396,6 +446,169 @@ export function applyAtomicYamlPatchesWithCurrent( }); }); } +export interface AtomicYamlConfigTransaction { + configPath: string; + root: unknown; + current: Readonly>; + /** True once any write op has durably committed (a later CAS rejection then + * leaves the target with partial writes, so recovery artifacts must stay). */ + written: boolean; + applyPatches(patches: readonly AtomicYamlPatch[], options?: AtomicYamlPatchOptions): Promise; + /** + * Delete top-level keys verbatim, including dotted key names (e.g. a flat + * `"gjc.ralplan.maxIterations"` key that the patch grammar would otherwise + * interpret as a nested path). Writes atomically under the same lock. + */ + removeTopLevelKeys(keys: readonly string[], options?: AtomicYamlPatchOptions): Promise; + /** + * Apply patches AND delete top-level keys verbatim in a SINGLE atomic + * write, so an external editor's change cannot land between the two + * operations (external editors do not participate in the file lock). The + * returned receipt is not restorable (the deletions are not journaled). + */ + applyPatchesAndRemoveTopLevelKeys( + patches: readonly AtomicYamlPatch[], + topLevelKeys: readonly string[], + options?: AtomicYamlPatchOptions, + ): Promise; + /** + * Replace the whole document (used to revert the target when a later + * verification fails). Writes atomically under the same lock; the returned + * receipt is not restorable. + */ + replaceCurrent(next: Readonly>, options?: AtomicYamlPatchOptions): Promise; +} + +/** + * Run a caller-owned multi-step mutation under the config file's per-file queue + * and cross-process lock. The current YAML is read once and exposed as + * `root`/`current`; the callback may inspect it, decide patches, and apply them + * (or perform adjacent durable actions such as marker/source transitions) + * without re-acquiring the lock. A YAML parse failure surfaces before the + * callback runs, so no migration action can execute against a malformed target. + */ +export function withAtomicYamlConfigTransaction( + configPath: string, + operation: (transaction: AtomicYamlConfigTransaction) => Promise, +): Promise { + return enqueueAtomicYamlOperation(configPath, async canonicalPath => { + await fs.mkdir(path.dirname(canonicalPath), { recursive: true, mode: 0o700 }); + return await withFileLock(canonicalPath, async () => { + const { current, root, raw } = await readYaml(canonicalPath); + // External editors do not participate in the file lock, so a save + // between the initial read and a write would be silently overwritten. + // Guard every write with a compare-and-swap against the last content + // this transaction read/wrote; on mismatch, fail closed. + let lastKnownRaw: string | null = null; + const casGuard = async (): Promise => { + const expected = lastKnownRaw ?? raw; + const currentRaw = await Bun.file(canonicalPath) + .text() + .catch((error: unknown) => { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return ""; + // An unreadable (EACCES) or transient I/O failure is NOT an + // empty file: fail closed so the write never clobbers a file + // it cannot read. + throw error; + }); + if (currentRaw !== expected) { + throw new AtomicYamlConflictError( + canonicalPath, + createHash("sha256").update(expected).digest("hex"), + createHash("sha256") + .update(currentRaw ?? "") + .digest("hex"), + ); + } + }; + let written = false; + const markWritten = (): void => { + written = true; + }; + return await operation({ + configPath: canonicalPath, + root, + current, + get written(): boolean { + return written; + }, + applyPatches: async (patches, options = {}) => { + for (const patch of patches) assertPatch(patch); + await options.validateRoot?.(root, patches); + await casGuard(); + const receipt = await applyPatchesUnderLock( + canonicalPath, + current, + patches, + options, + false, + lastKnownRaw ?? raw, + ); + if (patches.length > 0) { + lastKnownRaw = YAML.stringify(current, null, 2); + markWritten(); + } + return receipt; + }, + removeTopLevelKeys: async (keys, options = {}) => { + for (const key of keys) delete current[key]; + await casGuard(); + lastKnownRaw = await writeAtomicYaml(canonicalPath, current, options, lastKnownRaw ?? raw); + markWritten(); + // The deleted top-level key values are not journaled, so a + // restore() would vacuously claim success; report honestly that + // the receipt is not restorable. + let discarded = false; + return { + revisions: [], + discard(): void { + discarded = true; + }, + async restore(): Promise { + return discarded ? { status: "discarded" } : { status: "not-restorable" }; + }, + }; + }, + applyPatchesAndRemoveTopLevelKeys: async (patches, topLevelKeys, options = {}) => { + for (const patch of patches) assertPatch(patch); + await options.validateRoot?.(root, patches); + await casGuard(); + await applyPatchesUnderLock(canonicalPath, current, patches, options, true); + for (const key of topLevelKeys) delete current[key]; + lastKnownRaw = await writeAtomicYaml(canonicalPath, current, options, lastKnownRaw ?? raw); + markWritten(); + let discarded = false; + return { + revisions: [], + discard(): void { + discarded = true; + }, + async restore(): Promise { + return discarded ? { status: "discarded" } : { status: "not-restorable" }; + }, + }; + }, + replaceCurrent: async (next, options = {}) => { + for (const key of Object.keys(current)) delete current[key]; + Object.assign(current, next); + await casGuard(); + lastKnownRaw = await writeAtomicYaml(canonicalPath, current, options, lastKnownRaw ?? raw); + markWritten(); + let discarded = false; + return { + revisions: [], + discard(): void { + discarded = true; + }, + async restore(): Promise { + return discarded ? { status: "discarded" } : { status: "not-restorable" }; + }, + }; + }, + }); + }); + }); +} /** * Reserve a FIFO operation for a config file immediately. The patch supplier runs diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index c05f9a6be5..348dae02c7 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -553,6 +553,11 @@ export const SETTINGS_SCHEMA = { default: 1, validate: (value: number) => Number.isInteger(value) && value >= 1 && value <= 10, }, + "gjc.ultragoal.nudgeBudget": { + type: "number", + default: 10, + validate: (value: number) => Number.isInteger(value) && value >= 0, + }, // ──────────────────────────────────────────────────────────────────────── // Appearance diff --git a/packages/coding-agent/src/config/settings.ts b/packages/coding-agent/src/config/settings.ts index 183ef4336c..e7aef3cbcb 100644 --- a/packages/coding-agent/src/config/settings.ts +++ b/packages/coding-agent/src/config/settings.ts @@ -10,6 +10,8 @@ * For tests, `Settings.isolated()` seeds explicit user/global settings: * const isolated = Settings.isolated({ "compaction.enabled": false }); */ + +import { createHash, randomUUID } from "node:crypto"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; @@ -17,6 +19,7 @@ import * as util from "node:util"; import { getAgentDbPath, getAgentDir, + getConfigRootDir, getCustomThemesDir, getProjectDir, isEnoent, @@ -28,6 +31,7 @@ import { YAML } from "bun"; import { type Settings as SettingsCapabilityItem, settingsCapability } from "../capability/settings"; import type { ModelRole } from "../config/model-registry"; import { loadCapability } from "../discovery"; +import { extractWorkflowSetting, type WorkflowSettingKey } from "../gjc-runtime/workflow-settings"; import { isLightTheme, setAutoThemeMapping, setColorBlindMode, setSymbolPreset } from "../modes/theme/theme"; import { type NotificationSettingsReader, @@ -37,6 +41,8 @@ import { import { AgentStorage } from "../session/agent-storage"; import { type EditMode, normalizeEditMode } from "../utils/edit-mode"; import { + type AtomicYamlConfigTransaction, + AtomicYamlConflictError, type AtomicYamlPatch, applyAtomicYamlPatches, applyAtomicYamlPatchesWithCurrent, @@ -46,6 +52,7 @@ import { enqueueAtomicYamlOperation, reserveAtomicYamlUpdateSlot, setByPath, + withAtomicYamlConfigTransaction, } from "./atomic-yaml-patch"; import { isModelSelectorValue, type ModelSelectorValue, normalizeModelSelectorValue } from "./model-selector-value"; @@ -75,6 +82,52 @@ export interface RawSettings { [key: string]: unknown; } +const CONFIG_ROOT_WORKFLOW_MIGRATION_KEYS: readonly WorkflowSettingKey[] = [ + "gjc.deepInterview.ambiguityThreshold", + "gjc.ralplan.autoHandoff", + "gjc.ralplan.maxIterations", + "gjc.ralplan.maxReviewPassesPerLane", + "gjc.ultragoal.nudgeBudget", +]; + +const WORKFLOW_MIGRATION_MARKER_VERSION = 1; + +type WorkflowMigrationMarker = { + version: 1; + status: "pending" | "complete"; + sourcePath: string; + backupPath: string; + targetPath: string; + /** Canonical (realpath) agent dir at migration time; a symlink repointed + * afterwards must not be treated as the same migration target. */ + canonicalTargetDir?: string; + /** `dev:ino` of the target config.yml at migration time; detects a + * same-pathname profile REPLACEMENT (deleted + recreated), which realpath + * alone cannot. */ + canonicalTargetIdentity?: string; + sourceSha256: string; + migratedKeys: WorkflowSettingKey[]; + startedAt: string; + /** The prior source hash (the migration-write ownership basis) when the + * reconcile rewrites the marker as pending; the resume accepts a backup + * matching either the new hash (after refresh) or this prior hash. */ + priorSourceSha256?: string; + /** Per-key sha256 of the values written by an interrupted reconcile; the + * resume recognizes a target matching a recorded repair value as the + * reconcile's own write even after a further source edit. */ + repairValueHashes?: Record; + /** True once the reconcile's target repairs were actually applied (the + * pending marker is rewritten after the CAS-protected apply succeeds); + * only then are repairValueHashes treated as committed-write evidence. */ + repairsApplied?: boolean; + /** Per-key sha256 of the target values BEFORE the interrupted reconcile's + * repairs; the resume recognizes a repair value as committed when the + * target CHANGED from this recorded state (even if the post-apply marker + * rewrite was not reached). */ + preRepairTargetHashes?: Record; + completedAt?: string; +}; + type SettingsPatch = { readonly path: string; readonly value: unknown | undefined; @@ -1125,7 +1178,8 @@ export class Settings implements NotificationSettingsReader { try { if (this.#persist) { this.#storage = await AgentStorage.open(getAgentDbPath(this.#agentDir)); - await this.#migrateFromLegacy(); + await this.#migrateAgentDirAndDatabaseLegacy(); + await this.#migrateConfigRootWorkflowSettings(); this.#global = await this.#loadYaml(this.#configPath!); } if (this.#schemaMigrationPending) @@ -1403,7 +1457,7 @@ export class Settings implements NotificationSettingsReader { logger.warn(`Settings: ${message}`); } - async #migrateFromLegacy(): Promise { + async #migrateAgentDirAndDatabaseLegacy(): Promise { if (!this.#configPath) return; // Check if config.yml already exists @@ -1455,6 +1509,1391 @@ export class Settings implements NotificationSettingsReader { } } + /** + * One-time migration of the machine-global config-root `settings.json` + * (`/settings.json`, normally `~/.gjc/settings.json`) workflow + * keys into the default global agent `config.yml`. Runs only for the default + * global agent scope, inside one critical section on the target config lock, + * and migrates only the five workflow keys that the workflow runtimes read. + * + * The legacy config-root file is an orphan path: only the workflow runtimes + * ever read it, and the earlier Settings migrations never covered it. Keeping + * it read-only forever would leave two settings surfaces in conflict, so a + * valid source is consumed exactly once (absent-only patches, no-clobber + * `.bak`, durable sidecar marker) after which the runtimes' legacy fallback + * still works for a user-recreated file. + */ + async #migrateConfigRootWorkflowSettings(): Promise { + if (!this.#configPath) return; + // Strengthened pairing gate: only the default global agent scope may + // consume the machine-global source. A custom/temporary agentDir + // (`Settings.loadForScope` for SDK or tests) must never touch it. + if (!this.#isDefaultGlobalAgentScope()) return; + + const source = path.resolve(getConfigRootDir(), "settings.json"); + const backup = `${source}.bak`; + const markerPath = `${source}.migrated`; + const target = path.resolve(this.#configPath); + // If the config root is literally the agent dir, the agent-dir migration + // already owns this physical source; never double-rename it. + if (source === path.resolve(path.join(this.#agentDir, "settings.json"))) return; + + // Short-circuit before touching the target config.yml: with no source, + // backup, or marker there is nothing to migrate, and entering the + // transaction would parse the target (aborting settings load on a + // malformed config.yml even when no migration is needed). + const preSourceExists = await this.#pathExists(source); + const preBackupExists = await this.#pathExists(backup); + const preMarkerExists = await this.#pathExists(markerPath); + if (!preSourceExists && !preBackupExists && !preMarkerExists) return; + try { + await withAtomicYamlConfigTransaction(target, async tx => { + // A config.yml written by a NEWER schema version is intentionally + // read-only across Settings; the migration runs before #loadYaml + // sets #futureSchemaVersion, so it must check the target schema + // itself and never patch it or consume the legacy source. + const targetSchemaVersion = (tx.root as Record | null | undefined)?.configSchemaVersion; + if (typeof targetSchemaVersion === "number" && targetSchemaVersion > CONFIG_SCHEMA_VERSION) { + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration skipped: ${target} is a future config schema (configSchemaVersion ${targetSchemaVersion} > ${CONFIG_SCHEMA_VERSION})`, + ); + return; + } + const markerFileExists = await this.#pathExists(markerPath); + let marker = await this.#readWorkflowMigrationMarker(markerPath); + // A structurally valid marker that points at different source/backup/ + // target paths (e.g. the config root moved) must never suppress or + // shortcut the migration; treat it as invalid. + if (marker && !this.#workflowMigrationMarkerPathsMatch(marker, source, backup, target)) { + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration marker at ${markerPath} does not match the current source/backup/target paths; treating it as invalid`, + ); + marker = null; + } + if (marker?.status === "complete") { + // The migration is complete only while the source still matches + // the marker hash. If the user edited/recreated the legacy + // source, the stale migration-owned target values must be + // reconciled with the current source (the resolver already + // reactivates the legacy layer on the hash mismatch, but the + // higher-precedence agent-config value would keep shadowing it). + let currentCompleteSourceHash: string | null = null; + try { + currentCompleteSourceHash = await this.#sha256File(source); + } catch (error) { + if (!isEnoent(error)) { + // A transient read failure (permissions/I-O) is NOT a + // deletion: leave everything unchanged so no + // configuration or recovery data is lost. + this.#warnLegacyFallbackMigration( + `Settings: could not re-read ${source} after migration; leaving source/backup/marker untouched`, + ); + return; + } + // ENOENT = deleted after completion: honor the deletion by + // reverting ONLY the marker-owned target values that still + // match the migration's write (the backup copy); a newer + // `gjc config set` override is never reverted. + let deletionBackupDoc: Record | null = null; + try { + // The backup must still hash to the marker's sourceSha256: + // an edited/rewritten backup is not evidence of what the + // migration wrote, so refuse to roll back on it. + if ((await this.#sha256File(backup)) !== marker.sourceSha256) { + this.#warnLegacyFallbackMigration( + `Settings: the migration backup ${backup} no longer matches the marker hash; leaving source/backup/marker untouched`, + ); + return; + } + deletionBackupDoc = JSON.parse(await Bun.file(backup).text()) as Record; + } catch { + this.#warnLegacyFallbackMigration( + `Settings: could not read the migration backup ${backup} for deletion recovery; leaving source/backup/marker untouched`, + ); + return; + } + const unsets: AtomicYamlPatch[] = []; + const flatKeys: string[] = []; + for (const key of marker.migratedKeys) { + const targetValue = extractWorkflowSetting(tx.root, key, { flat: false }); + const migratedValue = extractWorkflowSetting(deletionBackupDoc, key); + if ( + targetValue.present && + migratedValue.present && + this.#coerceWorkflowScalar(key, migratedValue.value) === targetValue.value + ) { + unsets.push({ path: key, op: "unset" }); + if (Object.hasOwn(tx.root as Record, key)) flatKeys.push(key); + } + } + await tx.applyPatchesAndRemoveTopLevelKeys(unsets, flatKeys); + await fs.promises.rm(backup, { force: true }).catch(() => undefined); + await fs.promises.rm(markerPath, { force: true }).catch(() => undefined); + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow settings deleted after migration (${source}); stale marker-owned target values reverted, user overrides kept, backup removed, marker cleared`, + ); + return; + } + if (currentCompleteSourceHash === marker.sourceSha256) return; + await this.#reconcileMigratedSource({ + tx, + marker, + source, + backup, + markerPath, + target, + currentSourceHash: currentCompleteSourceHash, + }); + return; + } + + const sourceExists = await this.#pathExists(source); + const backupExists = await this.#pathExists(backup); + + // Valid pending marker: crash-recovery proof only. + if (marker?.status === "pending") { + if (backupExists && !sourceExists) { + // The copy path NEVER removes the source, so its absence + // here is an external DELETION: honor it by reverting the + // marker-owned target values, removing the backup, and + // clearing the marker - instead of finalizing and silently + // restoring the deleted overrides. + const backupHash = await this.#sha256File(backup); + if (backupHash !== marker.sourceSha256 && backupHash !== marker.priorSourceSha256) { + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration pending marker cannot be verified (${backup}); leaving for diagnosis/retry`, + ); + return; + } + // A deletion during a reconcile transition (the backup still + // matches priorSourceSha256) must revert EVERY marker-owned + // target - the marker claims them as its repairs. In the + // fresh case (backup matches the marker hash) revert only + // the values still matching the migration write, preserving + // a newer `gjc config set` override. + // A deletion during a reconcile transition accepts the + // prior-hash backup, but only targets matching a verifiable + // migration write (the backup) are reverted: a target the + // user replaced after the transition cannot be verified + // (the repaired values' source is gone) and is preserved. + let deletionBackupDoc: Record | null = null; + try { + deletionBackupDoc = JSON.parse(await Bun.file(backup).text()) as Record; + } catch { + this.#warnLegacyFallbackMigration( + `Settings: could not read the migration backup ${backup} for deletion recovery; leaving source/backup/marker untouched`, + ); + return; + } + const markerOwnedUnsets: AtomicYamlPatch[] = []; + const markerFlatKeys: string[] = []; + for (const key of marker.migratedKeys) { + const targetValue = extractWorkflowSetting(tx.root, key, { flat: false }); + if (!targetValue.present) continue; + const migratedValue = extractWorkflowSetting(deletionBackupDoc, key); + if ( + (migratedValue.present && + this.#coerceWorkflowScalar(key, migratedValue.value) === targetValue.value) || + // A reconcile that COMMITTED its repairs left the recorded + // repair values in the target: the deletion must revert + // them too (they are migration writes, not user + // overrides). The repair is committed only when the + // post-apply flag is set (a mere change from the + // pre-repair state could be a coincidental user + // value). + (marker.repairValueHashes?.[key] !== undefined && + createHash("sha256").update(JSON.stringify(targetValue.value)).digest("hex") === + marker.repairValueHashes[key] && + marker.repairsApplied === true) + ) { + markerOwnedUnsets.push({ path: key, op: "unset" }); + if (Object.hasOwn(tx.root as Record, key)) markerFlatKeys.push(key); + } + } + await tx.applyPatchesAndRemoveTopLevelKeys(markerOwnedUnsets, markerFlatKeys); + await fs.promises.rm(backup, { force: true }).catch(() => undefined); + await fs.promises.rm(markerPath, { force: true }).catch(() => undefined); + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration cleared: ${source} was deleted during pending recovery; marker-owned target values reverted, backup removed`, + ); + return; + } + if (sourceExists && backupExists) { + const sourceStat = await fs.promises.stat(source).catch((error: unknown) => { + // Only ENOENT means absence; a transient permission/I-O + // failure must not be misread as a source edit (which + // would revert marker-owned values and remove recovery + // artifacts). + if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; + throw error; + }); + const sourceHash = sourceStat ? await this.#sha256File(source) : ""; + const backupHash = await this.#sha256File(backup); + if ( + (sourceHash === marker.sourceSha256 && backupHash !== marker.sourceSha256) || + // The source was edited AGAIN during the transition: the + // backup still matches priorSourceSha256, which is + // evidence the reconcile is in progress - resume it + // against the CURRENT source. + (marker.priorSourceSha256 !== undefined && backupHash === marker.priorSourceSha256) + ) { + // The reconcile recorded the CURRENT source hash in a + // PENDING marker: it was interrupted between the pending + // write and the COMPLETE marker (a crash or a backup + // failure). Resume it to completion. + await this.#reconcileMigratedSource({ + tx, + marker, + source, + backup, + markerPath, + target, + currentSourceHash: sourceHash, + }); + return; + } + if (sourceHash === marker.sourceSha256 && backupHash === marker.sourceSha256) { + // Interrupted no-replace move with the target already patched: + // the duplicate source is kept ACTIVE - a path-based unlink + // after the identity check could delete a rename-replaced + // file - and the resolver deactivates the migrated legacy + // layer while the source still matches the marker hash + // (reactivating it on later edits/recreates). Complete only + // when the target actually contains the migrated keys. + if (this.#workflowMigrationTargetSatisfies(tx.root, marker)) { + await this.#writeWorkflowMigrationMarkerAtomic(markerPath, { + ...marker, + status: "complete", + priorSourceSha256: undefined, + repairValueHashes: undefined, + repairsApplied: undefined, + preRepairTargetHashes: undefined, + canonicalTargetDir: await fs.promises + .realpath(path.dirname(target)) + .catch(() => path.dirname(target)), + canonicalTargetIdentity: await this.#statIdentity(path.dirname(target)), + completedAt: new Date().toISOString(), + }); + } else { + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration pending marker has matching source/backup but the target lacks the migrated keys; leaving source and backup untouched`, + ); + } + } else { + if (backupHash === marker.sourceSha256 && sourceHash !== marker.sourceSha256) { + // The user EDITED the still-active source after the + // crash: revert ONLY the marker-owned target values + // that still match the migration's write (the backup + // copy); a newer `gjc config set` override is + // preserved. Remove the backup and the pending marker + // so the next load re-runs fresh against the edited + // source. + let editBackupDoc: Record | null = null; + try { + editBackupDoc = JSON.parse(await Bun.file(backup).text()) as Record; + } catch { + this.#warnLegacyFallbackMigration( + `Settings: could not read the migration backup ${backup} for edited-source recovery; leaving source/backup/marker untouched`, + ); + return; + } + const markerOwnedUnsets: AtomicYamlPatch[] = []; + const markerFlatKeys: string[] = []; + for (const key of marker.migratedKeys) { + const targetValue = extractWorkflowSetting(tx.root, key, { flat: false }); + const migratedValue = extractWorkflowSetting(editBackupDoc, key); + if ( + targetValue.present && + migratedValue.present && + this.#coerceWorkflowScalar(key, migratedValue.value) === targetValue.value + ) { + markerOwnedUnsets.push({ path: key, op: "unset" }); + if (Object.hasOwn(tx.root as Record, key)) markerFlatKeys.push(key); + } + } + await tx.applyPatchesAndRemoveTopLevelKeys(markerOwnedUnsets, markerFlatKeys); + await fs.promises.rm(backup, { force: true }).catch(() => undefined); + await fs.promises.rm(markerPath, { force: true }).catch(() => undefined); + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration pending marker source edited after a crash; stale marker-owned target values reverted, user overrides kept, backup removed, marker cleared`, + ); + } else { + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration pending marker has both source and backup with mismatched hashes; leaving untouched`, + ); + } + } + return; + } + if (!sourceExists && !backupExists) { + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration pending marker without source or backup; leaving for diagnosis`, + ); + return; + } + // pending | yes | no — fall through and re-run the idempotent fresh + // transaction (absent-only patches make re-application harmless). + } else if (sourceExists && backupExists) { + // Absent/invalid marker with a pre-existing backup is ambiguous; + // never consume or overwrite either file. + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration found pre-existing ${backup} without a valid marker; leaving source and backup untouched`, + ); + return; + } else if (!sourceExists && backupExists) { + // Orphan backup: values may already be in the target; keep both + // recoverable and never infer completion from the backup alone. + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration found orphan backup ${backup} without a marker; leaving it untouched`, + ); + return; + } else if (!sourceExists && !backupExists) { + return; + } + + // Fresh transaction (or pending | yes | no re-run): source exists, + // backup absent. All steps run under the target config lock. + let sourceRaw: string; + try { + sourceRaw = await Bun.file(source).text(); + } catch { + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration could not read ${source}; leaving untouched`, + ); + return; + } + const sourceSha256 = createHash("sha256").update(sourceRaw).digest("hex"); + let sourceDoc: unknown; + try { + sourceDoc = JSON.parse(sourceRaw); + } catch { + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration found malformed JSON in ${source}; leaving source/backup/marker unchanged`, + ); + return; + } + // A `null` document root is malformed per the strict resolver (exit + // 2); the migration must not consume it (empty keys + .bak + + // complete marker would silently default). Leave the source active + // so the strict failure stays loud. + if ( + sourceDoc === null || + sourceDoc === undefined || + typeof sourceDoc !== "object" || + Array.isArray(sourceDoc) + ) { + // A `null`/non-object root is malformed per the strict resolver + // (exit 2); the migration must not consume it (empty keys + + // .bak + complete marker would silently default). Under a + // changed-pending recovery (crash after the patch, before the + // backup), clear the stale marker-owned target patches so the + // malformed source is visible to strict ralplan instead of + // being shadowed by the old agent value. + // Only clear patches when a backup verifies they are still the + // migration write; without one the target values may be newer + // overrides and must be preserved. + if (marker?.status === "pending" && backupExists) { + const staleUnsets: AtomicYamlPatch[] = []; + const staleFlatKeys: string[] = []; + for (const key of marker.migratedKeys) { + if (extractWorkflowSetting(tx.current, key, { flat: false }).present) { + staleUnsets.push({ path: key, op: "unset" }); + if (Object.hasOwn(tx.current as Record, key)) staleFlatKeys.push(key); + } + } + await tx.applyPatchesAndRemoveTopLevelKeys(staleUnsets, staleFlatKeys); + } + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration found a malformed root in ${source}; leaving source/backup/marker unchanged`, + ); + return; + } + // A `null`/`~` YAML root is treated by #loadYaml as a malformed + // config (settings stay read-only until repaired), so the migration + // must treat it like the other non-object roots: abort without + // writing or consuming the legacy source. + if (tx.root !== undefined && (tx.root === null || typeof tx.root !== "object" || Array.isArray(tx.root))) { + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration target ${target} has a non-object or null YAML root; not migrating`, + ); + return; + } + const targetDoc = tx.root === undefined ? {} : (tx.root as Record); + const migratedKeys: WorkflowSettingKey[] = []; + const patches: AtomicYamlPatch[] = []; + const flatKeysToRemove: string[] = []; + // A pending marker means a crashed run may have left a STALE patch + // in the target; if the source changed since that marker, the stale + // target value must not suppress the key (it would shadow the edit + // and move it to .bak). Reapply the current source value over it. + const stalePendingOverride = marker?.status === "pending" && sourceSha256 !== marker.sourceSha256; + for (const key of CONFIG_ROOT_WORKFLOW_MIGRATION_KEYS) { + // Only keys the crashed run actually recorded are migration-owned + // in the changed-pending window: they may be reapplied, unset, or + // overridden by the current source, but a key the marker did NOT + // record was skipped because config.yml already held a valid + // higher-precedence user value, which must never be clobbered. + const staleMarkerKey = stalePendingOverride && marker?.migratedKeys.includes(key); + const extracted = extractWorkflowSetting(sourceDoc, key); + if (extracted.malformedParent) { + // A non-mapping workflow parent in the source (e.g. + // `{"gjc":{"ralplan":"broken"}}`) is malformed legacy JSON + // that strict ralplan must fail on (exit 2); completing the + // migration would deactivate the source and silently use + // defaults. Under a changed-pending recovery (crash after + // the patch), first clear the stale marker-owned target + // patches so the malformed source is visible to strict + // ralplan instead of being shadowed by the old agent value. + // Clear the marker-owned target patches so the malformed + // source is visible to strict ralplan (exit 2) instead of + // being shadowed by the stale agent value: fail closed + // even without a backup (a user override preserved in this + // window is the lesser risk than silently hiding the + // malformed explicit settings). + if (marker?.status === "pending") { + const staleUnsets: AtomicYamlPatch[] = []; + const staleFlatKeys: string[] = []; + for (const ownedKey of marker.migratedKeys) { + if (extractWorkflowSetting(tx.current, ownedKey, { flat: false }).present) { + staleUnsets.push({ path: ownedKey, op: "unset" }); + if (Object.hasOwn(tx.current as Record, ownedKey)) { + staleFlatKeys.push(ownedKey); + } + } + } + await tx.applyPatchesAndRemoveTopLevelKeys(staleUnsets, staleFlatKeys); + } + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration aborted: ${source} has a non-mapping parent for ${key}; leaving source/backup/marker unchanged`, + ); + return; + } + if (!extracted.present) { + // A key the user REMOVED from the source (after a crash that + // had already patched config.yml) should drop its stale + // target value, so the deletion is honored. But ownership is + // verifiable only against the migration's backup copy: in + // the pending no-backup recovery the target value may be a + // NEWER `gjc config set` override, so never unset it + // blindly - leave it and warn. + if (staleMarkerKey && extractWorkflowSetting(targetDoc, key, { flat: false }).present) { + if (backupExists) { + patches.push({ path: key, op: "unset" }); + if (Object.hasOwn(targetDoc, key)) flatKeysToRemove.push(key); + } else { + // Ownership is unverifiable: abort the recovery (the + // source stays active) instead of completing with the + // key omitted from migratedKeys, which would + // deactivate the edited source and leave the stale + // target effective permanently. + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration aborted: ${key} removed from ${source} but its target value cannot be verified as the migration write (no backup); keeping the source active`, + ); + return; + } + } + continue; + } + // A *valid* present target value for this key wins: the legacy + // config-root value (valid or not) is never observed by the + // resolver, so skip this key entirely instead of aborting the + // whole migration over a stale overridden value (unless the + // target itself holds a stale patch for a marker-recorded key - + // see above). + const targetValue = extractWorkflowSetting(targetDoc, key, { flat: false }); + if (targetValue.malformedParent) { + // A non-object intermediate in config.yml (e.g. + // `gjc: { ralplan: "repair-me" }`) is malformed user data + // that #loadYaml would report for repair; writing the + // migrated value would silently replace it. Abort and leave + // everything untouched. + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration aborted: ${target} has a non-mapping parent for ${key}; leaving source/backup/marker untouched`, + ); + return; + } + if (targetValue.present && this.#workflowKeyValueIsValid(key, targetValue.value)) { + // A *valid* present target value wins unless the key is a + // stale marker-owned key under a changed-pending recovery + // (staleMarkerKey), where the current source value must be + // reapplied over the stale patch below. + if (!staleMarkerKey) { + // Retry (unchanged source) or a genuine user value: + // still schedule the flat-form cleanup for marker-owned + // keys so a dotted top-level key left by a crash between + // applyPatches and removeTopLevelKeys does not keep + // config.yml rejected by the generated schema - and keep + // the key in the rebuilt migratedKeys so ownership + // survives the marker rewrite. + if (marker?.migratedKeys.includes(key)) { + if (Object.hasOwn(targetDoc, key)) flatKeysToRemove.push(key); + if (!migratedKeys.includes(key)) migratedKeys.push(key); + } + continue; + } + } + // Validate the legacy value BEFORE migrating it. An invalid + // tolerant value (e.g. `"gjc.ultragoal.nudgeBudget": "bad"`) + // must not be copied into the durable config.yml, where + // Settings.load()/config doctor would report it on every + // startup (previously the tolerant runtime simply ignored it + // in settings.json and fell back to the default). + if (!this.#workflowKeyValueIsValid(key, extracted.value)) { + if ( + staleMarkerKey && + backupExists && + extractWorkflowSetting(targetDoc, key, { flat: false }).present + ) { + // Changed-pending recovery: unset the stale crashed patch + // for a marker-recorded key so the current source value + // (valid or invalid, tolerant or strict) is honored - + // never leave a stale target value shadowing it. + patches.push({ path: key, op: "unset" }); + if (Object.hasOwn(targetDoc, key)) flatKeysToRemove.push(key); + } + // Strict ralplan keys must keep the legacy source active only + // when the invalid value would actually be the winning layer: + // consuming it would silently fall back to defaults instead of + // failing loudly (the strict resolver throws exit 2 on the + // invalid value). Tolerant keys are simply skipped. + if (key.startsWith("gjc.ralplan.")) { + // If the unset above was queued, apply it so the invalid + // legacy source is visible (exit 2) instead of being + // shadowed by the stale valid target value. + if ( + staleMarkerKey && + backupExists && + extractWorkflowSetting(targetDoc, key, { flat: false }).present + ) { + patches.push({ path: key, op: "unset" }); + if (Object.hasOwn(targetDoc, key)) flatKeysToRemove.push(key); + } + // Apply ALL queued repairs (earlier keys' unsets and this + // key's unset) before aborting, so no stale target value + // for any marker-recorded key survives in config.yml. + // Only under a changed-pending recovery: in a FRESH + // migration the queued patches are plain SETs for valid + // keys, and applying them on an abort would write + // un-marker'd partial artifacts that the target-wins + // rule would freeze. + if (stalePendingOverride) { + // Apply only MARKER-OWNED repairs: fresh SETs for + // unrecorded keys must not be committed on an abort + // (the marker does not own them; committing would + // shadow later source edits forever via the + // valid-target guard). + const repairPatches = patches.filter(patch => + marker?.migratedKeys.includes(patch.path as WorkflowSettingKey), + ); + const repairFlatKeys = flatKeysToRemove.filter(key => + marker?.migratedKeys.includes(key as WorkflowSettingKey), + ); + // One atomic write for the repairs + flat cleanup. + if (repairPatches.length > 0 || repairFlatKeys.length > 0) { + await tx.applyPatchesAndRemoveTopLevelKeys(repairPatches, repairFlatKeys); + } + } + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration aborted: invalid strict ralplan value for ${key} in ${source}; keeping the legacy source active so gjc ralplan still fails loudly`, + ); + return; + } + continue; + } + // The changed-pending REAPPLY overwrites a present target value; + // without a backup the migration write is unverifiable and the + // target may be an editor's newer value. Abort the recovery + // (keeping the source active) instead of completing with the + // key omitted from migratedKeys, which would deactivate the + // edited source and leave the stale target effective forever. + if (stalePendingOverride && marker?.migratedKeys.includes(key) && targetValue.present && !backupExists) { + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration aborted: ${key} reapplied by the source but its target value cannot be verified as the migration write (no backup); keeping the source active`, + ); + return; + } + migratedKeys.push(key); + // Persist the COERCED value (quoted numeric string -> number), not + // the raw string: a schema-backed config.yml must hold values the + // generated JSON schema accepts and Settings does not need to + // re-coerce on every load. + patches.push({ path: key, op: "set", value: this.#coerceWorkflowScalar(key, extracted.value) }); + // Flat keys are checked before nested ones by + // extractWorkflowSetting, so an invalid flat key (e.g. + // `"gjc.ralplan.maxIterations": bad`) would keep masking the + // migrated nested value after the legacy source is moved to .bak. + // Remove the flat form verbatim (the patch grammar cannot address + // dotted top-level key names). + if (Object.hasOwn(targetDoc, key)) flatKeysToRemove.push(key); + } + + // An invalid/untrusted marker must never suppress migration. Preserve + // its bytes by a no-clobber quarantine; abort if quarantine is + // impossible. (A malformed marker parses to null, so the file's + // existence is the signal, not a non-null marker object.) + if (markerFileExists && marker === null) { + const corruptPath = `${markerPath}.corrupt`; + if (!(await this.#moveLegacySourceNoReplace(markerPath, corruptPath))) { + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration could not quarantine invalid marker ${markerPath}; leaving unchanged`, + ); + return; + } + } + + const startedAt = + marker?.status === "pending" && typeof marker.startedAt === "string" + ? marker.startedAt + : new Date().toISOString(); + await this.#writeWorkflowMigrationMarkerAtomic(markerPath, { + version: WORKFLOW_MIGRATION_MARKER_VERSION, + status: "pending", + sourcePath: source, + backupPath: backup, + targetPath: target, + sourceSha256, + migratedKeys, + startedAt, + }); + + // The legacy source may have been edited since `sourceSha256` was + // computed and the patches built. Re-hash BEFORE writing anything so + // a stale patch never lands in the higher-precedence config.yml, + // and snapshot the target so a late mismatch can revert it. + if ((await this.#sha256File(source)) !== sourceSha256) { + // Nothing was patched: the pending marker's migratedKeys would + // falsely claim ownership of these patches on the next + // changed-pending recovery (staleMarkerKey), letting it + // overwrite a valid user target override. Remove it so the + // next load starts fresh. (Narrow compound edge: a hard-crashed + // run that left stale target patches plus a source re-edit + // inside this recovery window leaves those patches + // indistinguishable from a user override under target-wins; + // the source bytes are preserved in .bak.) + await fs.promises.rm(markerPath, { force: true }).catch(() => undefined); + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration aborted: ${source} changed during migration; marker cleared, source left active for the next load`, + ); + return; + } + const prePatchTargetSnapshot = structuredClone(tx.current); + // Apply the nested patches AND the flat-form cleanup in a single + // atomic write: two separate writes would let an external editor's + // config.yml change (which does not participate in the file lock) + // land between them and be overwritten. + await tx.applyPatchesAndRemoveTopLevelKeys(patches, flatKeysToRemove); + + // Re-hash immediately before the no-replace move; on mismatch, revert + // the target to its pre-patch state so the next load re-runs against + // the current file instead of resolving a stale agent-config value. + let preMoveSourceHash: string | null = null; + try { + preMoveSourceHash = await this.#sha256File(source); + } catch { + // The source was deleted after the patch: revert the target and + // clear the now-obsolete marker so the deletion is honored (the + // later post-copy deletion path does the same). + await tx.replaceCurrent(prePatchTargetSnapshot); + await fs.promises.rm(backup, { force: true }).catch(() => undefined); + await fs.promises.rm(markerPath, { force: true }).catch(() => undefined); + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration aborted: ${source} was deleted during migration; target reverted, backup removed, marker cleared`, + ); + return; + } + if (preMoveSourceHash !== sourceSha256) { + await tx.replaceCurrent(prePatchTargetSnapshot); + await fs.promises.rm(backup, { force: true }).catch(() => undefined); + await fs.promises.rm(markerPath, { force: true }).catch(() => undefined); + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration aborted: ${source} changed during migration; target reverted, backup removed, marker cleared`, + ); + return; + } + if (!(await this.#moveLegacySourceNoReplace(source, backup, sourceSha256))) { + // The target was already patched; revert it so the higher + // precedence config.yml does not shadow the still-active source + // (a `.bak` that appeared in the window would otherwise leave + // the pending yes/yes recovery row warning on mismatch forever). + await tx.replaceCurrent(prePatchTargetSnapshot); + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration could not move ${source} to ${backup} without overwrite; target reverted, pending marker retained for retry`, + ); + return; + } + + // The source may have been edited in the narrow window after the + // pre-move check but before/during the move; verify the bytes we + // actually moved. The source is kept ACTIVE on every path, so on + // mismatch the edit is already live: revert the target and remove + // the now-superseded backup so the next load sees pending + source + // + no backup and re-runs the fresh transaction against the edited + // file (never completing behind a stale hash). + if ((await this.#sha256File(backup)) !== sourceSha256) { + await tx.replaceCurrent(prePatchTargetSnapshot); + await fs.promises.rm(backup, { force: true }).catch(() => undefined); + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration aborted: ${source} changed during migration; target reverted, backup removed, source left active for the next load`, + ); + return; + } + // On the copy fallback (filesystems without hard links) the source + // is deliberately kept ACTIVE, so a same-key edit after the copy is + // shadowed by the higher-precedence patched config.yml; verify the + // source (absent = externally deleted) and revert on + // mismatch, removing the now-superseded backup so the next load + // sees pending + source + no backup and re-runs the fresh + // transaction against the edited file. + let sourceHashAfterMove: string | null = null; + try { + sourceHashAfterMove = await this.#sha256File(source); + } catch (error) { + // The source is kept ACTIVE on every path, so ENOENT can only be + // a concurrent DELETION of the legacy file: honor it by undoing + // the patch and backup and clearing the pending marker (there is + // nothing left to migrate), instead of completing behind the old + // values and silently undoing the deletion. + if (isEnoent(error)) { + await tx.replaceCurrent(prePatchTargetSnapshot); + await fs.promises.rm(backup, { force: true }).catch(() => undefined); + await fs.promises.rm(markerPath, { force: true }).catch(() => undefined); + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration aborted: ${source} was deleted during migration; target reverted, backup removed, marker cleared`, + ); + return; + } + // Non-ENOENT read failure: fail closed (revert + retain + // pending) rather than completing behind a possibly-edited + // source. + await tx.replaceCurrent(prePatchTargetSnapshot); + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration aborted: could not re-read ${source} after the move; target reverted, pending marker retained`, + ); + return; + } + if (sourceHashAfterMove !== null && sourceHashAfterMove !== sourceSha256) { + await tx.replaceCurrent(prePatchTargetSnapshot); + await fs.promises.rm(backup, { force: true }).catch(() => undefined); + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration aborted: ${source} edited after the copy; target reverted, backup removed, source left active for the next load`, + ); + return; + } + + await this.#writeWorkflowMigrationMarkerAtomic(markerPath, { + version: WORKFLOW_MIGRATION_MARKER_VERSION, + status: "complete", + sourcePath: source, + backupPath: backup, + targetPath: target, + canonicalTargetDir: await fs.promises.realpath(path.dirname(target)).catch(() => path.dirname(target)), + canonicalTargetIdentity: await this.#statIdentity(path.dirname(target)), + sourceSha256, + migratedKeys, + startedAt, + completedAt: new Date().toISOString(), + }); + logger.debug("Settings: migrated config-root workflow settings to config.yml", { + source, + target, + migratedKeys, + }); + }); + } catch (error) { + // A CAS rejection means an external editor changed config.yml before + // any patch of this run applied: a pending marker's migratedKeys + // would falsely claim ownership of never-applied patches, so clear it + // A CAS rejection means an external editor changed config.yml before a + // write of THIS run applied. The pending marker is RETAINED: in a + // changed-pending recovery the prior run already patched config.yml + // and the marker is the only evidence that the existing target value + // is migration-written - clearing it would let the stale value pass + // the valid-target guard and complete with the key omitted. (A + // retained marker whose claims were never applied is handled safely + // by the unverifiable-ownership abort, which keeps the source active.) + if (error instanceof AtomicYamlConflictError) { + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration aborted: ${target} changed externally during migration; pending marker retained`, + ); + return; + } + // A malformed target config.yml must not abort settings load: warn and + // leave source/backup/marker untouched so #loadYaml's recoverable + // malformed-config diagnostics still run after the migration returns. + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow migration could not run against ${target}: ${error instanceof Error ? error.message : String(error)}; leaving source/backup/marker untouched`, + ); + } + } + + async #reconcileMigratedSource(params: { + tx: AtomicYamlConfigTransaction; + marker: WorkflowMigrationMarker; + source: string; + backup: string; + markerPath: string; + target: string; + currentSourceHash: string; + }): Promise { + let { tx, marker, source, backup, markerPath, target, currentSourceHash } = params; + // The source changed after completion: validate its root before + // reconciling - a malformed root (null/array) must not be + // accepted as a settings mapping (strict ralplan fails on it). + let currentSourceText: string; + try { + currentSourceText = await Bun.file(source).text(); + // Bind the marker hash to the bytes ACTUALLY read (the editor + // may have saved between the earlier #sha256File and this + // read): the backup and marker must describe the same text. + currentSourceHash = createHash("sha256").update(currentSourceText).digest("hex"); + } catch (error) { + // Only a DELETED source (ENOENT) is left untouched; a transient + // EACCES/EIO read failure propagates so the recovery never + // misreads the source state. + if (!isEnoent(error)) throw error; + this.#warnLegacyFallbackMigration( + `Settings: could not read ${source} for re-migration; leaving source/backup/marker untouched`, + ); + return; + } + let currentSourceDoc: Record; + try { + const parsed = JSON.parse(currentSourceText) as unknown; + if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) { + this.#warnLegacyFallbackMigration( + `Settings: ${source} changed after migration to a non-mapping root; leaving source/backup/marker untouched (strict ralplan fails on it)`, + ); + return; + } + currentSourceDoc = parsed as Record; + } catch { + this.#warnLegacyFallbackMigration( + `Settings: could not parse ${source} for re-migration; leaving source/backup/marker untouched`, + ); + return; + } + let backupDoc: Record | null = null; + try { + // The backup must hash to the marker's sourceSha256 (the OLD + // ownership basis after refresh) OR to the marker's priorSourceSha256 + // (the migration-write basis when the reconcile is resumed before its + // backup refresh). Accepting an arbitrary refreshed backup would let + // an interrupted refresh reclassify a user's target override as + // migration-owned; only these two recorded hashes are accepted. + const reconcileBackupHash = await this.#sha256File(backup); + if (reconcileBackupHash !== marker.sourceSha256 && reconcileBackupHash !== marker.priorSourceSha256) { + this.#warnLegacyFallbackMigration( + `Settings: the migration backup ${backup} no longer matches the marker hash; leaving source/backup/marker untouched`, + ); + return; + } + backupDoc = JSON.parse(await Bun.file(backup).text()) as Record; + } catch { + // No usable backup: cannot verify what the migration wrote; + // leave everything unchanged. + this.#warnLegacyFallbackMigration( + `Settings: could not read the migration backup ${backup}; leaving source/backup/marker untouched`, + ); + return; + } + // Reconcile EVERY supported workflow key: marker-recorded keys + // (only when the target still matches the migration's write) and + // keys newly added to the source after completion (copied when + // the target has no value for them). + const repairPatches: AtomicYamlPatch[] = []; + const newlyPropagatedKeys: WorkflowSettingKey[] = []; + // Marker-owned keys whose target STILL matches the old migration + // write (the backup): a target the user changed after migration + // loses migration ownership. + const retainedOwnedKeys: WorkflowSettingKey[] = []; + const repairFlatKeys: string[] = []; + for (const key of CONFIG_ROOT_WORKFLOW_MIGRATION_KEYS) { + const sourceValue = extractWorkflowSetting(currentSourceDoc, key); + if (sourceValue.malformedParent) { + // A non-mapping workflow parent (e.g. `gjc.ralplan: + // "broken"`) hides EVERY sibling under it, so strict + // ralplan must fail on the malformed explicit source. + // Clear ALL marker-owned targets under the malformed + // parent prefix that still match the migration's write, + // and DROP the accumulated repairs (they were never + // committed, so committing them here would leave + // ownership evidence inconsistent). + const malformedPrefix = key.split(".").slice(0, -1).join("."); + const malformedUnsets: AtomicYamlPatch[] = []; + const malformedFlatKeys: string[] = []; + for (const ownedKey of marker.migratedKeys) { + if (!ownedKey.startsWith(`${malformedPrefix}.`) && ownedKey !== malformedPrefix) continue; + const staleTarget = extractWorkflowSetting(tx.root, ownedKey, { flat: false }); + const backupVal = extractWorkflowSetting(backupDoc, ownedKey); + if ( + staleTarget.present && + backupVal.present && + this.#coerceWorkflowScalar(ownedKey, backupVal.value) === staleTarget.value + ) { + malformedUnsets.push({ path: ownedKey, op: "unset" }); + if (Object.hasOwn(tx.root as Record, ownedKey)) { + malformedFlatKeys.push(ownedKey); + } + } + } + if (malformedUnsets.length > 0 || malformedFlatKeys.length > 0) { + await tx.applyPatchesAndRemoveTopLevelKeys(malformedUnsets, malformedFlatKeys); + } + this.#warnLegacyFallbackMigration( + `Settings: ${source} has a non-mapping parent for ${key} after migration; stale marker-owned values cleared, source/backup/marker left active (strict ralplan fails on it)`, + ); + return; + } + const markerRecorded = marker.migratedKeys.includes(key); + const targetValue = extractWorkflowSetting(tx.root, key, { flat: false }); + const migratedValue = markerRecorded + ? extractWorkflowSetting(backupDoc, key) + : { present: false, value: undefined }; + const targetIsMigrationWrite = + markerRecorded && + targetValue.present && + // The reconcile's own write is proven by the recorded repair value + // (repairValueHashes) or the migration-write backup - NOT by + // equality with the CURRENT source, which a later source edit or a + // coincidental user value could match. + ((migratedValue.present && this.#coerceWorkflowScalar(key, migratedValue.value) === targetValue.value) || + // A target matching a value an interrupted reconcile recorded + // (repairValueHashes) is its own write ONLY when the post-apply + // flag is set: a target that merely CHANGED from the pre-repair + // state could be a coincidental user value (an external + // `gjc config set` before the repair's CAS), so the change + // alone does not prove the reconcile wrote it. + (marker.repairValueHashes?.[key] !== undefined && + createHash("sha256").update(JSON.stringify(targetValue.value)).digest("hex") === + marker.repairValueHashes[key] && + marker.repairsApplied === true)); + if (targetIsMigrationWrite) retainedOwnedKeys.push(key); + if (sourceValue.present) { + // Never copy an invalid edited value into config.yml. For + // a STRICT ralplan key, also clear the stale + // migration-write target so the invalid source is visible + // to strict ralplan (exit 2) instead of being shadowed; + // tolerant keys keep the migration-write (the tolerant + // runtime ignores the invalid value and falls back). + if (!this.#workflowKeyValueIsValid(key, sourceValue.value)) { + if (key.startsWith("gjc.ralplan.") && targetIsMigrationWrite) { + const clearKeys = Object.hasOwn(tx.root as Record, key) ? [key] : []; + await tx.applyPatchesAndRemoveTopLevelKeys([{ path: key, op: "unset" }], clearKeys); + } + this.#warnLegacyFallbackMigration( + `Settings: ${source} has an invalid value for ${key} after migration; leaving source/backup/marker untouched (strict ralplan fails on it)`, + ); + return; + } + // Copy when the target still holds the migration write, or + // when the key was never migrated / is absent from the + // target. A target value that merely EQUALS the source is + // not reclaimed as migration-owned (it may be a user + // override that happens to match). + if (targetIsMigrationWrite || !targetValue.present) { + // A stale migration-write (reapply the current source + // value), a NEWLY ADDED key with no target value, or a + // deleted-and-readded key (target absent again) all + // copy the current source value. A marker-recorded key + // copied back into an absent target REMAINS owned. + if (markerRecorded) retainedOwnedKeys.push(key); + repairPatches.push({ + path: key, + op: "set", + value: this.#coerceWorkflowScalar(key, sourceValue.value), + }); + if (Object.hasOwn(tx.root as Record, key)) repairFlatKeys.push(key); + if (!markerRecorded) newlyPropagatedKeys.push(key); + } + // else: the user edited the target; keep it. + } else if (targetIsMigrationWrite && targetValue.present) { + // The user removed the key from the source AND the + // target still holds the migration's value: honor the + // deletion. + repairPatches.push({ path: key, op: "unset" }); + if (Object.hasOwn(tx.root as Record, key)) repairFlatKeys.push(key); + } + } + // Record the reconcile in a PENDING marker FIRST (new hash + keys + + // the written repair values): a crash or backup failure between here + // and the COMPLETE marker leaves a pending state that the next load + // resumes, so ownership of the reconcile-copied keys survives partial + // writes - and the resume can recognize a target matching a recorded + // repair value even after a further source edit. + const repairValueHashes: Record = {}; + for (const patch of repairPatches) { + if (patch.op === "set") { + repairValueHashes[patch.path] = createHash("sha256").update(JSON.stringify(patch.value)).digest("hex"); + } + } + // The prior basis must be the DURABLE backup hash (a resumed pass's + // marker.sourceSha256 may describe a source the backup never saw). + // The target BEFORE the repairs is the durable basis for recognizing a + // committed repair when the post-apply marker rewrite is not reached. + // Record pre-repair evidence for EVERY repair patch: newly propagated + // and re-added keys are absent before the apply, so they get an explicit + // absent sentinel - any present target then proves the repair committed. + const preRepairTargetHashes: Record = {}; + for (const patch of repairPatches) { + if (patch.op !== "set") continue; + const preRepairValue = extractWorkflowSetting(tx.current, patch.path as WorkflowSettingKey, { flat: false }); + preRepairTargetHashes[patch.path] = preRepairValue.present + ? createHash("sha256").update(JSON.stringify(preRepairValue.value)).digest("hex") + : "absent"; + } + const durableBackupHash = await this.#sha256File(backup); + await this.#writeWorkflowMigrationMarkerAtomic(markerPath, { + ...marker, + status: "pending", + priorSourceSha256: durableBackupHash, + preRepairTargetHashes, + repairValueHashes, + sourceSha256: currentSourceHash, + migratedKeys: [...new Set([...retainedOwnedKeys, ...newlyPropagatedKeys])], + }); + // Snapshot the target before the repairs so they can be rolled + // back if the source changes again before publication. + const preRepairTargetSnapshot = structuredClone(tx.current); + if (repairPatches.length > 0 || repairFlatKeys.length > 0) { + await tx.applyPatchesAndRemoveTopLevelKeys(repairPatches, repairFlatKeys); + // The repairs are COMMITTED: rewrite the pending marker with + // repairsApplied so the resume (and the direct resolver) treat the + // recorded repair values as committed-write evidence - not mere + // intent, which a user override could coincidentally match. + await this.#writeWorkflowMigrationMarkerAtomic(markerPath, { + ...marker, + status: "pending", + priorSourceSha256: durableBackupHash, + repairValueHashes, + preRepairTargetHashes, + repairsApplied: true, + sourceSha256: currentSourceHash, + migratedKeys: [...new Set([...retainedOwnedKeys, ...newlyPropagatedKeys])], + }); + } + // The editor may have saved again during the reconcile: only + // publish when the source still holds the exact bytes we + // reconciled and hashed. + let finalSourceText: string; + try { + finalSourceText = await Bun.file(source).text(); + } catch { + // Roll back the just-applied repairs so the target does not + // hold an intermediate value classified as a user override. + await tx.replaceCurrent(preRepairTargetSnapshot); + // The repairs were rolled back: clear the recorded repair evidence so + // a later load does not treat the (now reverted) target values as + // committed writes. + await this.#writeWorkflowMigrationMarkerAtomic(markerPath, { + ...marker, + status: "pending", + priorSourceSha256: durableBackupHash, + repairValueHashes: undefined, + repairsApplied: undefined, + preRepairTargetHashes: undefined, + sourceSha256: currentSourceHash, + migratedKeys: [...new Set([...retainedOwnedKeys, ...newlyPropagatedKeys])], + }); + this.#warnLegacyFallbackMigration( + `Settings: could not re-read ${source} before publishing the reconciliation; target repairs rolled back, leaving source/backup/marker untouched`, + ); + return; + } + if (finalSourceText !== currentSourceText) { + // Roll back the just-applied repairs: the next load must + // re-reconcile against the NEW source without the target + // holding an intermediate value classified as a user + // override. + await tx.replaceCurrent(preRepairTargetSnapshot); + // Clear the recorded repair evidence (the repairs were rolled back). + await this.#writeWorkflowMigrationMarkerAtomic(markerPath, { + ...marker, + status: "pending", + priorSourceSha256: durableBackupHash, + repairValueHashes: undefined, + repairsApplied: undefined, + preRepairTargetHashes: undefined, + sourceSha256: currentSourceHash, + migratedKeys: [...new Set([...retainedOwnedKeys, ...newlyPropagatedKeys])], + }); + this.#warnLegacyFallbackMigration( + `Settings: ${source} changed again during reconciliation; target repairs rolled back (the next load re-reconciles)`, + ); + return; + } + // REFRESH the backup to the current source (the new + // migration-write basis), then publish the COMPLETE marker only + // after both durable writes succeed - a crash or CAS rejection + // before that leaves the OLD complete marker, so the next load + // re-enters the reconcile (source hash mismatch) instead of + // deactivating the legacy layer over an un-reconciled target. + // Replace the backup atomically (write a temp, then rename): an + // in-place Bun.write could truncate it on a crash, leaving the + // old marker with an unverifiable backup. + const backupDir = path.dirname(backup); + const backupTemp = path.join(backupDir, `.${path.basename(backup)}.${process.pid}.${randomUUID()}.tmp`); + try { + await Bun.write(backupTemp, currentSourceText); + await fs.promises.rename(backupTemp, backup); + } finally { + await fs.promises.rm(backupTemp, { force: true }).catch(() => undefined); + } + await this.#writeWorkflowMigrationMarkerAtomic(markerPath, { + ...marker, + // The resume enters with a pending marker; the repairs and backup + // refresh succeeded, so publish COMPLETE. + status: "complete", + priorSourceSha256: undefined, + repairValueHashes: undefined, + repairsApplied: undefined, + preRepairTargetHashes: undefined, + canonicalTargetDir: await fs.promises.realpath(path.dirname(target)).catch(() => path.dirname(target)), + canonicalTargetIdentity: await this.#statIdentity(path.dirname(target)), + sourceSha256: currentSourceHash, + migratedKeys: [...new Set([...retainedOwnedKeys, ...newlyPropagatedKeys])], + completedAt: new Date().toISOString(), + }); + this.#warnLegacyFallbackMigration( + `Settings: config-root workflow settings changed after migration (${source}); reconciled the current values`, + ); + return; + } + + async #statIdentity(filePath: string): Promise { + const st = await fs.promises.stat(filePath).catch(() => null); + return st ? `${st.dev}:${st.ino}` : undefined; + } + + #isDefaultGlobalAgentScope(): boolean { + return ( + path.resolve(this.#agentDir) === path.resolve(getAgentDir()) && + path.resolve(getAgentDir()) === path.resolve(path.join(getConfigRootDir(), "agent")) + ); + } + + #workflowMigrationTargetSatisfies(root: unknown, marker: WorkflowMigrationMarker): boolean { + if (root === undefined || root === null) return marker.migratedKeys.length === 0; + if (typeof root !== "object" || Array.isArray(root)) return false; + const doc = root as Record; + return marker.migratedKeys.every(key => extractWorkflowSetting(doc, key, { flat: false }).present); + } + #workflowKeyValueIsValid(key: WorkflowSettingKey, value: unknown): boolean { + const def = SETTINGS_SCHEMA[key] as + | { type?: string; validate?: (value: number) => boolean; values?: readonly unknown[] } + | undefined; + if (!def) return true; + let candidate: unknown = value; + candidate = this.#coerceWorkflowScalar(key, candidate); + switch (def.type) { + case "enum": + return def.values !== undefined && def.values.includes(candidate); + case "number": + return def.validate !== undefined + ? def.validate(candidate as number) + : typeof candidate === "number" && Number.isFinite(candidate); + case "boolean": + return typeof candidate === "boolean"; + case "string": + return typeof candidate === "string"; + default: + return true; + } + } + #workflowMigrationMarkerPathsMatch( + marker: WorkflowMigrationMarker, + source: string, + backup: string, + target: string, + ): boolean { + return ( + path.resolve(marker.sourcePath) === path.resolve(source) && + path.resolve(marker.backupPath) === path.resolve(backup) && + path.resolve(marker.targetPath) === path.resolve(target) + ); + } + /** + * Mirror the resolver/Settings scalar coercion for a workflow key: a quoted + * numeric string for a number setting (e.g. `maxIterations: "9"`) becomes + * the number 9. Used both for validity checks and for what the migration + * persists into config.yml. + */ + #coerceWorkflowScalar(key: WorkflowSettingKey, value: unknown): unknown { + const def = SETTINGS_SCHEMA[key] as { type?: string } | undefined; + if ( + def?.type === "number" && + typeof value === "string" && + value.trim() !== "" && + Number.isFinite(Number(value)) + ) { + return Number(value); + } + return value; + } + + async #readWorkflowMigrationMarker(markerPath: string): Promise { + let raw: string; + try { + raw = await Bun.file(markerPath).text(); + } catch (error) { + // Only ENOENT means no marker; a transient EACCES/EIO read failure + // must propagate so a valid pending marker is never quarantined as + // corrupt and its ownership evidence lost. + if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; + throw error; + } + try { + const parsed = JSON.parse(raw) as Record; + if (parsed.version !== WORKFLOW_MIGRATION_MARKER_VERSION) return null; + if (parsed.status !== "pending" && parsed.status !== "complete") return null; + if ( + typeof parsed.sourcePath !== "string" || + typeof parsed.backupPath !== "string" || + typeof parsed.targetPath !== "string" || + typeof parsed.sourceSha256 !== "string" || + !/^[0-9a-f]{64}$/.test(parsed.sourceSha256) || + typeof parsed.startedAt !== "string" || + Number.isNaN(Date.parse(parsed.startedAt)) || + !Array.isArray(parsed.migratedKeys) || + !parsed.migratedKeys.every( + key => + typeof key === "string" && (CONFIG_ROOT_WORKFLOW_MIGRATION_KEYS as readonly string[]).includes(key), + ) + ) { + return null; + } + if ( + parsed.status === "complete" && + (typeof parsed.completedAt !== "string" || Number.isNaN(Date.parse(parsed.completedAt))) + ) { + return null; + } + return parsed as WorkflowMigrationMarker; + } catch { + return null; + } + } + + async #writeWorkflowMigrationMarkerAtomic(markerPath: string, marker: WorkflowMigrationMarker): Promise { + const serialized = JSON.stringify(marker, null, 2); + const directory = path.dirname(markerPath); + const tempPath = path.join(directory, `.${path.basename(markerPath)}.${process.pid}.${randomUUID()}.tmp`); + try { + await Bun.write(tempPath, serialized); + await fs.promises.rename(tempPath, markerPath); + } finally { + await fs.promises.rm(tempPath, { force: true }).catch(() => undefined); + } + } + + async #moveLegacySourceNoReplace( + source: string, + destination: string, + expectedSourceSha256?: string, + ): Promise { + try { + await fs.promises.lstat(destination); + return false; // Never overwrite an existing destination. + } catch (error) { + if (!isEnoent(error)) return false; + } + if (expectedSourceSha256 !== undefined) { + // USER-DATA move: use an INDEPENDENT copy (never a hard link - a kept + // source and a hard-linked backup share an inode, so a later in-place + // edit or truncation of the still-active legacy file would mutate the + // backup and the marker hash would no longer preserve the migrated + // bytes) and keep the source ACTIVE (never unlink; a path-based unlink + // after a non-atomic identity check could delete a rename-replaced + // file). The caller re-verifies the source before the complete marker. + try { + await fs.promises.copyFile(source, destination, fs.constants.COPYFILE_EXCL); + } catch { + return false; + } + let copiedSourceHash: string | null = null; + try { + copiedSourceHash = await this.#sha256File(source); + } catch { + // The source was deleted right after the copy: remove the copy and + // report failure so the caller reverts the target (the caller's + // later guarded recheck would otherwise be bypassed by the throw). + await fs.promises.rm(destination, { force: true }); + return false; + } + if (copiedSourceHash !== expectedSourceSha256) { + await fs.promises.rm(destination, { force: true }); + return false; + } + return true; + } + // Internal artifacts (marker quarantine): capture the inode, hard-link + // (copy fallback), and remove the source name only while it is still the + // inode we verified. + let sourceIno: number | undefined; + try { + sourceIno = (await fs.promises.stat(source)).ino; + } catch { + return false; + } + try { + // Atomic same-directory no-clobber move via hard link + unlink. + await fs.promises.link(source, destination); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "EEXIST") return false; + // Filesystems without hard links: a no-clobber copy. COPYFILE_EXCL + // fails with EEXIST if the destination appears, so it can never + // replace an existing `.bak`/quarantine - unlike a raw rename, which + // would overwrite a destination created after the lstat above. + try { + await fs.promises.copyFile(source, destination, fs.constants.COPYFILE_EXCL); + } catch { + return false; + } + } + if (!(await this.#legacySourceStillVerified(source, sourceIno))) { + await fs.promises.rm(destination, { force: true }); + return false; + } + try { + await fs.promises.rm(source, { force: true }); + } catch { + return false; + } + return true; + } + /** + * True only if `path` still refers to the same inode that was verified + * earlier and (when an expected hash is given) still holds the verified + * bytes. Used immediately before any unlink of a legacy source so a + * concurrent rename-style save or in-place edit is never consumed. + */ + async #legacySourceStillVerified( + path: string, + expectedIno: number, + expectedSourceSha256?: string, + ): Promise { + const stat = await fs.promises.stat(path).catch(() => null); + if (!stat || stat.ino !== expectedIno) return false; + if (expectedSourceSha256 !== undefined && (await this.#sha256File(path)) !== expectedSourceSha256) return false; + return true; + } + + async #pathExists(target: string): Promise { + try { + await fs.promises.lstat(target); + return true; + } catch (error) { + // Only ENOENT means absence; a transient EACCES/EIO failure must + // propagate so recovery never mistakes it for a deletion/removal. + if ((error as NodeJS.ErrnoException).code === "ENOENT") return false; + throw error; + } + } + + async #sha256File(target: string): Promise { + const raw = await Bun.file(target).arrayBuffer(); + return createHash("sha256").update(Buffer.from(raw)).digest("hex"); + } + #hasCustomThemeFile(name: string): boolean { try { return fs.existsSync(path.join(getCustomThemesDir(this.#agentDir), `${name}.json`)); diff --git a/packages/coding-agent/src/defaults/gjc/skills/deep-interview/SKILL.md b/packages/coding-agent/src/defaults/gjc/skills/deep-interview/SKILL.md index 6d99015bb3..4b8dce683a 100644 --- a/packages/coding-agent/src/defaults/gjc/skills/deep-interview/SKILL.md +++ b/packages/coding-agent/src/defaults/gjc/skills/deep-interview/SKILL.md @@ -104,14 +104,16 @@ Complete this phase before Phase 1, before brownfield exploration, before GJC st 1. **Prefer pre-resolved native state**: - First inspect active deep-interview state with `gjc deep-interview read --json`. - If state contains a finite numeric `threshold` and a non-empty `threshold_source`, use those values, set ``, ``, and ``, and skip optional settings-file reads. This is the normal `/skill:deep-interview` path because the native hook already resolved settings quietly before loading the skill. -2. **Only if native state lacks a resolved threshold, read threshold settings in runtime precedence order**: - - YAML config first: read the **single** modern config path the environment selects — `$GJC_CODING_AGENT_DIR/config.yml` when `GJC_CODING_AGENT_DIR` is set, else `$GJC_CONFIG_DIR/agent/config.yml` when `GJC_CONFIG_DIR` is set, else `~/.gjc/agent/config.yml`. Do not cascade through the other YAML locations when the selected one is absent or invalid. - - Then JSON settings: project settings `./.gjc/settings.json`, then user settings `[$GJC_CONFIG_DIR|~/.gjc]/settings.json`. - - Read `gjc.deepInterview.ambiguityThreshold` only from files that are known to exist; optional config/settings-file absence is expected and must not be surfaced as failed `Read` calls. - - Do not probe arbitrary ancestor candidates such as `../../.gjc/settings.json`; use the current project `.gjc/settings.json` and user settings only. +2. **Only if native state lacks a resolved threshold, read threshold settings in the runtime precedence order** (one shared resolver; first valid value wins): + 1. project `.gjc/config.yml` + 2. project `.gjc/settings.json` + 3. user `/config.yml` (normally `~/.gjc/agent/config.yml`, honoring `GJC_CODING_AGENT_DIR`/`PI_CODING_AGENT_DIR` and XDG) + 4. user `/settings.json` (normally `~/.gjc/settings.json`; legacy, last-resort fallback) + 5. built-in default (`0.05` before resolution-flag fallback) + - `config.yml` uses the nested (schema) form - `gjc: { deepInterview: { ambiguityThreshold } }`; flat dotted keys (`gjc.deepInterview.ambiguityThreshold`) are honored only in legacy `settings.json` files. Project configuration beats user configuration. The reported `threshold_source` is the canonical path of the winning file, or the default. The legacy config-root `settings.json` is migrated once into the default global agent `config.yml` (absent-only). Invalid optional settings files continue to the next layer or the default (tolerant) — never a failed `Read`; do not probe arbitrary ancestor candidates. 3. **Resolve threshold and source**: - Use the first valid configured value in the precedence order above; otherwise use the mode default when a resolution flag was passed: `--quick` = `0.6`, `--standard` = `0.5`, `--deep` = `0.35`; with no resolution flag, use the base default `0.05`. - - Set these run variables exactly: ``, ``, and `` (for example `GJC_CODING_AGENT_DIR/config.yml`, `$GJC_CONFIG_DIR/agent/config.yml`, `~/.gjc/agent/config.yml`, `./.gjc/settings.json`, `[$GJC_CONFIG_DIR|~/.gjc]/settings.json`, or the selected mode default). + - Set these run variables exactly: ``, ``, and `` (for example `./.gjc/config.yml`, `./.gjc/settings.json`, `/config.yml` such as `~/.gjc/agent/config.yml`, `~/.gjc/settings.json`, or the selected mode default). 4. **Emit the required first line to the user before any other interview announcement**: ``` diff --git a/packages/coding-agent/src/defaults/gjc/skills/ralplan/SKILL.md b/packages/coding-agent/src/defaults/gjc/skills/ralplan/SKILL.md index 860cae1a1d..53e7224c8b 100644 --- a/packages/coding-agent/src/defaults/gjc/skills/ralplan/SKILL.md +++ b/packages/coding-agent/src/defaults/gjc/skills/ralplan/SKILL.md @@ -91,7 +91,7 @@ The consensus workflow: e. Re-join Architect and Critic verdicts for the same revised Planner artifact/pass (including a fresh disposition stage if new conflicts appear) f. Repeat this loop until Critic returns `OKAY` **and** Architect is `CLEAR`/`APPROVE` for the same Planner artifact/pass, or 5 iterations are reached g. If 5 iterations are reached without Critic `OKAY` plus Architect `CLEAR`/`APPROVE`, **stop opening further planner/revision passes**. Preserve the best version as a terminal `PLANNING-STUCK` result; do not route it to automatic or explicit execution. - h. **Runtime budget (#3165):** native `gjc ralplan --write` refuses a new `planner`/`revision` that would open consensus iteration **> max** (default **5**, overridable via `gjc.ralplan.maxIterations` in project/user `.gjc/settings.json`, integer 1..20). Cap uses the same iteration definition as the HUD (`planner`/`revision` openers in `index.jsonl`). Overflow exits **3**, prints operator-visible **`PLANNING-STUCK`** on stdout (and stderr detail; JSON includes `planning_stuck: true`), and still allows `architect`/`critic` within an already-opened pass plus `post-interview`/`adr`/`final` so the best plan can be escalated to `pending approval` without dispatch. A new `--run-id` starts a fresh budget. + h. **Runtime budget (#3165):** native `gjc ralplan --write` refuses a new `planner`/`revision` that would open consensus iteration **> max** (default **5**, overridable via `gjc.ralplan.maxIterations`, integer 1..20, using the workflow-settings precedence below). Cap uses the same iteration definition as the HUD (`planner`/`revision` openers in `index.jsonl`). Overflow exits **3**, prints operator-visible **`PLANNING-STUCK`** on stdout (and stderr detail; JSON includes `planning_stuck: true`), and still allows `architect`/`critic` within an already-opened pass plus `post-interview`/`adr`/`final` so the best plan can be escalated to `pending approval` without dispatch. A new `--run-id` starts a fresh budget. 6. **Post-ralplan interview** (intent reconciliation gate): After the review join gate has both Critic `OKAY` and Architect `CLEAR`/`APPROVE` for the same Planner artifact/pass, and before the plan is finalized, reconcile the consensus plan against the user's actual intent. The goal is to make sure ralplan did not silently bake in assumptions that conflict with what the user wants. a. **Collect open items** from the run: every assumption the Planner/Architect/Critic resolved by assumption rather than by stated fact, every ambiguity flagged during review, and every decision the loop made without explicit user input. Source these from the persisted `planner`/`architect`/`critic`/`revision` stage artifacts, not from memory. b. **Cross-check prior context for conflicts**: glob `.gjc/_session-{sessionid}/specs/deep-interview-*.md` and other prior specs/plans/context relevant by topic. For each, list points where the consensus plan contradicts, weakens, or expands beyond a previously crystallized decision, constraint, or non-goal. Cite the conflicting artifact and line/section. @@ -126,7 +126,29 @@ The consensus workflow: - On cap: exit code **3**, marker **`PLANNING-STUCK`** (stdout), no silent re-loop, no automatic or explicit ultragoal/team dispatch. Opener budget is `max(index.jsonl openers, on-disk stage-*-{planner,revision}.md count)` so a missing/empty/malformed ledger cannot fail open after prior openers. - Headless/CI: treat `PLANNING-STUCK` / exit 3 as terminal planning failure for orchestration/watchdogs. - Interactive: retain the best existing plan as a terminal planning result; residual critic findings stay as caveats. -- Override example (project `.gjc/settings.json`): +- **Workflow settings precedence** — ralplan reads all of its settings + (`gjc.ralplan.maxIterations`, `gjc.ralplan.maxReviewPassesPerLane`, + `gjc.ralplan.autoHandoff`) through one shared resolver in this exact order + (first valid value wins): + 1. project `.gjc/config.yml` + 2. project `.gjc/settings.json` + 3. user `/config.yml` (normally `~/.gjc/agent/config.yml`, honoring + `GJC_CODING_AGENT_DIR`/`PI_CODING_AGENT_DIR` and XDG) + 4. user `/settings.json` (normally `~/.gjc/settings.json`; + legacy, last-resort fallback) + 5. built-in default + + `config.yml` uses the nested (schema) form — `gjc: { ralplan: { maxIterations } }`; + flat dotted keys (`gjc.ralplan.maxIterations`) are honored only in legacy + `settings.json` files. Project configuration beats user configuration. The + reported `source` is the canonical path of the winning file, or `default`. The + legacy config-root `settings.json` is migrated once into the default global + agent `config.yml` (absent-only, default-global-agent-scope only). **Malformed or + invalid explicit settings in any layer/format exit 2** + (`invalid ralplan settings at : `) — including + `gjc.ralplan.maxIterations`, whose former silent fallback to the default is + removed. +- Override example (project `.gjc/settings.json`; the same keys work in project `.gjc/config.yml` and the user layers): ```json { @@ -141,7 +163,7 @@ The consensus workflow: ## Per-lane review budget (operator contract) - Default: **1** Architect pass and **1** Critic pass per opener iteration. -- Override via `gjc.ralplan.maxReviewPassesPerLane`: project `.gjc/settings.json` overrides user settings; the value is an integer **1..10** registered in the public settings schema. +- Override via `gjc.ralplan.maxReviewPassesPerLane` (integer **1..10**, registered in the public settings schema) using the workflow-settings precedence above; project overrides user. - On overflow: exit code **3** with the **`PLANNING-STUCK`** marker and lane-specific JSON/stderr detail. - `post-interview`, `adr`, and `final` are always allowed. - Identical re-writes dedupe without stuck-signaling — including after a crash between artifact write and ledger append: the identical retry repairs the missing ledger row and returns the dedupe receipt. diff --git a/packages/coding-agent/src/defaults/gjc/skills/ultragoal/SKILL.md b/packages/coding-agent/src/defaults/gjc/skills/ultragoal/SKILL.md index 5c8ece70a1..e0d8a29537 100644 --- a/packages/coding-agent/src/defaults/gjc/skills/ultragoal/SKILL.md +++ b/packages/coding-agent/src/defaults/gjc/skills/ultragoal/SKILL.md @@ -18,6 +18,23 @@ Use when the user asks for `ultragoal`, `create-goals`, `complete-goals`, durabl - `.gjc/_session-{sessionid}/ultragoal/ledger.jsonl` (checkpoint and structured steering audit events) Existing aggregate plans with the legacy enumerated objective are migrated to the stable pointer objective on read, persisted to `goals.json`, retained in `gjcObjectiveAliases` for already-active hidden goal reconciliation, and audited with an `aggregate_objective_migrated` ledger entry. +- **Nudge budget setting** — the per-story give-up budget + (`gjc.ultragoal.nudgeBudget`, default **10**, non-negative integer) is read + through one shared resolver in this exact order (first valid value wins): + 1. project `.gjc/config.yml` + 2. project `.gjc/settings.json` + 3. user `/config.yml` (normally `~/.gjc/agent/config.yml`, honoring + `GJC_CODING_AGENT_DIR`/`PI_CODING_AGENT_DIR` and XDG) + 4. user `/settings.json` (normally `~/.gjc/settings.json`; + legacy, last-resort fallback) + 5. built-in default + `config.yml` uses the nested (schema) form - `gjc: { ultragoal: { nudgeBudget } }`; + flat dotted keys (`gjc.ultragoal.nudgeBudget`) are honored only in legacy + `settings.json` files. Project configuration beats user configuration. The reported + `source` is the canonical path of the winning file, or `default`. The legacy + config-root `settings.json` is migrated once into the default global agent + `config.yml` (absent-only). Invalid optional settings files continue to the + next layer or the default (tolerant). ## Corrupt current-session state recovery diff --git a/packages/coding-agent/src/gjc-runtime/deep-interview-runtime.ts b/packages/coding-agent/src/gjc-runtime/deep-interview-runtime.ts index c1212634fc..d66a87d5fc 100644 --- a/packages/coding-agent/src/gjc-runtime/deep-interview-runtime.ts +++ b/packages/coding-agent/src/gjc-runtime/deep-interview-runtime.ts @@ -1,8 +1,6 @@ import { createHash, randomBytes } from "node:crypto"; import * as fs from "node:fs/promises"; import * as path from "node:path"; -import { getConfigRootDir } from "@gajae-code/utils"; -import { YAML } from "bun"; import { syncSkillActiveState } from "../skill-state/active-state"; import { deriveDeepInterviewHud } from "../skill-state/workflow-hud"; import { WORKFLOW_STATE_VERSION } from "../skill-state/workflow-state-contract"; @@ -25,6 +23,7 @@ import { resolveGjcSessionForWrite, writeSessionActivityMarker } from "./session import { runNativeStateCommand } from "./state-runtime"; import { appendJsonl, readExistingStateForMutation, writeArtifact, writeWorkflowEnvelopeAtomic } from "./state-writer"; import { assertSafePathComponent, CommandError, flagValue, hasFlag } from "./workflow-cli-common"; +import { resolveWorkflowSetting } from "./workflow-settings"; export * from "./deep-interview-recorder"; @@ -346,62 +345,32 @@ interface DeepInterviewSpecWriteSummary { }; } -async function readSettingsAmbiguityThreshold( - settingsPath: string, -): Promise<{ threshold: number; source: string } | undefined> { - let raw: string; - try { - raw = await fs.readFile(settingsPath, "utf-8"); - } catch (error) { - const err = error as NodeJS.ErrnoException; - if (err.code === "ENOENT") return undefined; - return undefined; - } - let parsed: unknown; - try { - parsed = JSON.parse(raw); - } catch { - return undefined; - } - const candidate = (parsed as { gjc?: { deepInterview?: { ambiguityThreshold?: unknown } } })?.gjc?.deepInterview - ?.ambiguityThreshold; - if (typeof candidate !== "number" || !Number.isFinite(candidate) || candidate <= 0 || candidate > 1) { - return undefined; - } - return { threshold: candidate, source: settingsPath }; -} - -function modernSettingsPath(): string { - const configDir = process.env.GJC_CODING_AGENT_DIR?.trim() || process.env.PI_CODING_AGENT_DIR?.trim(); - if (configDir) return path.join(configDir, "config.yml"); - return path.join(getConfigRootDir(), "agent", "config.yml"); -} - -async function readModernSettingsAmbiguityThreshold(): Promise<{ threshold: number; source: string } | undefined> { - const modernConfigPath = modernSettingsPath(); - let parsed: unknown; - try { - parsed = YAML.parse(await fs.readFile(modernConfigPath, "utf-8")); - } catch { - return undefined; - } - const candidate = (parsed as { gjc?: { deepInterview?: { ambiguityThreshold?: unknown } } })?.gjc?.deepInterview - ?.ambiguityThreshold; - if (typeof candidate !== "number" || !Number.isFinite(candidate) || candidate <= 0 || candidate > 1) - return undefined; - return { threshold: candidate, source: modernConfigPath }; -} - +/** + * Resolve the configured ambiguity threshold through the shared five-layer + * resolver: project `.gjc/config.yml` > project `.gjc/settings.json` > user + * `getAgentDir()/config.yml` > legacy config-root `settings.json` > default. + * Project configuration beats user configuration, and invalid optional files + * continue to lower layers (tolerant contract). Returns `undefined` when the + * resolver falls back to the default so the resolution flags (`--quick`/ + * `--standard`/`--deep`) still apply. + */ async function resolveConfiguredAmbiguityThreshold( cwd: string, ): Promise<{ threshold: number; source: string } | undefined> { - const modernValue = await readModernSettingsAmbiguityThreshold(); - if (modernValue) return modernValue; - const projectSettings = path.join(cwd, ".gjc", "settings.json"); - const projectValue = await readSettingsAmbiguityThreshold(projectSettings); - if (projectValue) return projectValue; - const userSettings = path.join(getConfigRootDir(), "settings.json"); - return await readSettingsAmbiguityThreshold(userSettings); + const resolution = await resolveWorkflowSetting(cwd, "gjc.deepInterview.ambiguityThreshold", { + defaultValue: DEFAULT_AMBIGUITY_THRESHOLD, + parse: value => { + if (typeof value !== "number" || !Number.isFinite(value) || value <= 0 || value > 1) { + return { + kind: "invalid", + reason: "expected gjc.deepInterview.ambiguityThreshold to be a number in (0, 1]", + }; + } + return { kind: "valid", value }; + }, + }); + if (resolution.source === "default") return undefined; + return { threshold: resolution.value, source: resolution.source }; } function englishLanguagePreference(): DeepInterviewLanguagePreference { diff --git a/packages/coding-agent/src/gjc-runtime/ralplan-runtime.ts b/packages/coding-agent/src/gjc-runtime/ralplan-runtime.ts index 7bff0b047b..d5820ef483 100644 --- a/packages/coding-agent/src/gjc-runtime/ralplan-runtime.ts +++ b/packages/coding-agent/src/gjc-runtime/ralplan-runtime.ts @@ -2,7 +2,6 @@ import { createHash, randomBytes } from "node:crypto"; import type { Dirent } from "node:fs"; import * as fs from "node:fs/promises"; import * as path from "node:path"; -import { getConfigRootDir } from "@gajae-code/utils"; import { syncSkillActiveState } from "../skill-state/active-state"; import { buildRalplanHudSummary } from "../skill-state/workflow-hud"; import { WORKFLOW_STATE_VERSION } from "../skill-state/workflow-state-contract"; @@ -43,6 +42,12 @@ import { import { probeGjcTeamAvailability } from "./team-runtime"; import { assertSafePathComponent, CommandError, flagValue, hasFlag } from "./workflow-cli-common"; import { getSkillManifest } from "./workflow-manifest"; +import { + resolveWorkflowSetting, + WorkflowSettingError, + type WorkflowSettingKey, + type WorkflowSettingParseResult, +} from "./workflow-settings"; /** * Native implementation of `gjc ralplan`. * @@ -387,38 +392,56 @@ function parseMaxIterationsValue(value: unknown): number | null { return parseBoundedPositiveInteger(value, RALPLAN_MAX_ITERATIONS_LIMIT); } -async function readSettingsMaxIterations(settingsPath: string): Promise { +/** Adapt a nullable parser to the shared resolver's parse-result shape. */ +function workflowSettingParse( + parse: (value: unknown) => T | null, + reason: string, +): (value: unknown) => WorkflowSettingParseResult { + return value => { + const parsed = parse(value); + return parsed === null ? { kind: "invalid", reason } : { kind: "valid", value: parsed }; + }; +} + +/** + * Resolve a strict ralplan setting through the shared five-layer resolver. + * Malformed/invalid explicit sources in any layer/format fail closed (exit 2) + * and never fall through to a lower layer or the built-in default. + */ +async function resolveStrictRalplanSetting( + cwd: string, + key: WorkflowSettingKey, + parse: (value: unknown) => WorkflowSettingParseResult, + defaultValue: T, +): Promise<{ value: T; source: string }> { try { - const raw = await Bun.file(settingsPath).text(); - const parsed = JSON.parse(raw) as Record; - const flat = parseMaxIterationsValue(parsed["gjc.ralplan.maxIterations"]); - if (flat !== null) return flat; - const gjc = parsed.gjc; - if (gjc && typeof gjc === "object") { - const ralplan = (gjc as Record).ralplan; - if (ralplan && typeof ralplan === "object") { - return parseMaxIterationsValue((ralplan as Record).maxIterations); - } + const resolution = await resolveWorkflowSetting(cwd, key, { defaultValue, parse, invalidPolicy: "throw" }); + return { value: resolution.value, source: resolution.source }; + } catch (error) { + if (error instanceof WorkflowSettingError) { + throw new RalplanCommandError(2, `invalid ralplan settings at ${error.path}: ${error.reason}`); } - return null; - } catch { - return null; + throw error; } } /** - * Resolve ralplan consensus iteration cap. Project `./.gjc/settings.json` overrides - * user settings, else default 5. + * Resolve the ralplan consensus iteration cap through the shared resolver. + * Project `.gjc/config.yml` and `.gjc/settings.json` beat user layers. */ export async function resolveRalplanMaxIterations(cwd: string): Promise<{ maxIterations: number; source: string }> { - const projectPath = path.join(gjcRoot(cwd), "settings.json"); - const project = await readSettingsMaxIterations(projectPath); - if (project !== null) return { maxIterations: project, source: projectPath }; - const userPath = path.join(getConfigRootDir(), "settings.json"); - const user = await readSettingsMaxIterations(userPath); - if (user !== null) return { maxIterations: user, source: userPath }; - return { maxIterations: RALPLAN_DEFAULT_MAX_ITERATIONS, source: "default" }; + const { value, source } = await resolveStrictRalplanSetting( + cwd, + "gjc.ralplan.maxIterations", + workflowSettingParse( + parseMaxIterationsValue, + `expected gjc.ralplan.maxIterations to be an integer between 1 and ${RALPLAN_MAX_ITERATIONS_LIMIT}`, + ), + RALPLAN_DEFAULT_MAX_ITERATIONS, + ); + return { maxIterations: value, source }; } + function parseRalplanAutoHandoffTarget(value: unknown): RalplanAutoHandoffTarget | undefined { return typeof value === "string" && RALPLAN_AUTO_HANDOFF_TARGETS.has(value as RalplanAutoHandoffTarget) ? (value as RalplanAutoHandoffTarget) @@ -430,81 +453,24 @@ type RalplanAutoHandoffOptions = { teamAvailabilityProbe?: () => { available: true } | { available: false; reason: string }; }; -type RalplanAutoHandoffSetting = - | { kind: "absent" } - | { kind: "valid"; value: RalplanAutoHandoffTarget } - | { kind: "invalid"; reason: string }; - -function parsePresentRalplanAutoHandoff(value: unknown): RalplanAutoHandoffSetting { +function parsePresentRalplanAutoHandoff(value: unknown): WorkflowSettingParseResult { const target = parseRalplanAutoHandoffTarget(value); return target === undefined - ? { - kind: "invalid", - reason: "expected gjc.ralplan.autoHandoff to be one of off, ultragoal, team", - } + ? { kind: "invalid", reason: "expected gjc.ralplan.autoHandoff to be one of off, ultragoal, team" } : { kind: "valid", value: target }; } -function parseRalplanAutoHandoffSettings(parsed: unknown): RalplanAutoHandoffSetting { - if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return { kind: "absent" }; - const settings = parsed as Record; - if (Object.hasOwn(settings, "gjc.ralplan.autoHandoff")) { - return parsePresentRalplanAutoHandoff(settings["gjc.ralplan.autoHandoff"]); - } - const gjc = settings.gjc; - if (!gjc || typeof gjc !== "object" || Array.isArray(gjc)) return { kind: "absent" }; - const ralplan = (gjc as Record).ralplan; - if (!ralplan || typeof ralplan !== "object" || Array.isArray(ralplan)) return { kind: "absent" }; - const ralplanSettings = ralplan as Record; - if (!Object.hasOwn(ralplanSettings, "autoHandoff")) return { kind: "absent" }; - return parsePresentRalplanAutoHandoff(ralplanSettings.autoHandoff); -} - -async function readSettingsAutoHandoff(settingsPath: string): Promise { - let raw: string; - try { - raw = await Bun.file(settingsPath).text(); - } catch (error) { - if (getErrorCode(error) === "ENOENT") return { kind: "absent" }; - return { - kind: "invalid", - reason: `unable to read settings: ${error instanceof Error ? error.message : String(error)}`, - }; - } - let parsed: unknown; - try { - parsed = JSON.parse(raw); - } catch (error) { - return { - kind: "invalid", - reason: `malformed JSON: ${error instanceof Error ? error.message : String(error)}`, - }; - } - return parseRalplanAutoHandoffSettings(parsed); -} - export async function resolveRalplanAutoHandoff( cwd: string, options: RalplanAutoHandoffOptions = {}, ): Promise { - const projectPath = path.join(gjcRoot(cwd), "settings.json"); - const project = await readSettingsAutoHandoff(projectPath); - if (project.kind === "invalid") { - throw new RalplanCommandError(2, `invalid ralplan settings at ${projectPath}: ${project.reason}`); - } - if (project.kind === "valid") { - return resolveRalplanAutoHandoffTarget(project.value, projectPath, options); - } - const userPath = path.join(getConfigRootDir(), "settings.json"); - const user = await readSettingsAutoHandoff(userPath); - if (user.kind === "invalid") { - throw new RalplanCommandError(2, `invalid ralplan settings at ${userPath}: ${user.reason}`); - } - return resolveRalplanAutoHandoffTarget( - user.kind === "valid" ? user.value : "off", - user.kind === "valid" ? userPath : "default", - options, + const { value, source } = await resolveStrictRalplanSetting( + cwd, + "gjc.ralplan.autoHandoff", + parsePresentRalplanAutoHandoff, + "off", ); + return resolveRalplanAutoHandoffTarget(value, source, options); } function resolveRalplanAutoHandoffTarget( @@ -533,78 +499,20 @@ function parseMaxReviewPassesPerLaneValue(value: unknown): number | null { return parseBoundedPositiveInteger(value, RALPLAN_MAX_REVIEW_PASSES_PER_LANE_LIMIT); } -type RalplanReviewPassesPerLaneSetting = - | { kind: "absent" } - | { kind: "valid"; value: number } - | { kind: "invalid"; reason: string }; - -function parsePresentMaxReviewPassesPerLane(value: unknown): RalplanReviewPassesPerLaneSetting { - const parsed = parseMaxReviewPassesPerLaneValue(value); - return parsed === null - ? { - kind: "invalid", - reason: - "expected gjc.ralplan.maxReviewPassesPerLane to be an integer between 1 and " + - RALPLAN_MAX_REVIEW_PASSES_PER_LANE_LIMIT, - } - : { kind: "valid", value: parsed }; -} - -function parseMaxReviewPassesPerLaneSettings(parsed: unknown): RalplanReviewPassesPerLaneSetting { - if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return { kind: "absent" }; - const settings = parsed as Record; - if (Object.hasOwn(settings, "gjc.ralplan.maxReviewPassesPerLane")) { - return parsePresentMaxReviewPassesPerLane(settings["gjc.ralplan.maxReviewPassesPerLane"]); - } - const gjc = settings.gjc; - if (!gjc || typeof gjc !== "object" || Array.isArray(gjc)) return { kind: "absent" }; - const ralplan = (gjc as Record).ralplan; - if (!ralplan || typeof ralplan !== "object" || Array.isArray(ralplan)) return { kind: "absent" }; - const ralplanSettings = ralplan as Record; - if (!Object.hasOwn(ralplanSettings, "maxReviewPassesPerLane")) return { kind: "absent" }; - return parsePresentMaxReviewPassesPerLane(ralplanSettings.maxReviewPassesPerLane); -} - -async function readSettingsMaxReviewPassesPerLane(settingsPath: string): Promise { - let raw: string; - try { - raw = await Bun.file(settingsPath).text(); - } catch (error) { - if (getErrorCode(error) === "ENOENT") return { kind: "absent" }; - return { - kind: "invalid", - reason: `unable to read settings: ${error instanceof Error ? error.message : String(error)}`, - }; - } - let parsed: unknown; - try { - parsed = JSON.parse(raw); - } catch (error) { - return { - kind: "invalid", - reason: `malformed JSON: ${error instanceof Error ? error.message : String(error)}`, - }; - } - return parseMaxReviewPassesPerLaneSettings(parsed); -} - -/** Resolve the per-lane review-pass budget with project-over-user precedence. */ +/** Resolve the per-lane review-pass budget through the shared resolver. */ export async function resolveRalplanMaxReviewPassesPerLane( cwd: string, ): Promise<{ maxReviewPassesPerLane: number; source: string }> { - const projectPath = path.join(gjcRoot(cwd), "settings.json"); - const project = await readSettingsMaxReviewPassesPerLane(projectPath); - if (project.kind === "invalid") { - throw new RalplanCommandError(2, `invalid ralplan settings at ${projectPath}: ${project.reason}`); - } - if (project.kind === "valid") return { maxReviewPassesPerLane: project.value, source: projectPath }; - const userPath = path.join(getConfigRootDir(), "settings.json"); - const user = await readSettingsMaxReviewPassesPerLane(userPath); - if (user.kind === "invalid") { - throw new RalplanCommandError(2, `invalid ralplan settings at ${userPath}: ${user.reason}`); - } - if (user.kind === "valid") return { maxReviewPassesPerLane: user.value, source: userPath }; - return { maxReviewPassesPerLane: RALPLAN_DEFAULT_MAX_REVIEW_PASSES_PER_LANE, source: "default" }; + const { value, source } = await resolveStrictRalplanSetting( + cwd, + "gjc.ralplan.maxReviewPassesPerLane", + workflowSettingParse( + parseMaxReviewPassesPerLaneValue, + `expected gjc.ralplan.maxReviewPassesPerLane to be an integer between 1 and ${RALPLAN_MAX_REVIEW_PASSES_PER_LANE_LIMIT}`, + ), + RALPLAN_DEFAULT_MAX_REVIEW_PASSES_PER_LANE, + ); + return { maxReviewPassesPerLane: value, source }; } function buildPlanningStuckResult(input: { diff --git a/packages/coding-agent/src/gjc-runtime/ultragoal-runtime.ts b/packages/coding-agent/src/gjc-runtime/ultragoal-runtime.ts index b60ece39e6..fee71bf4bc 100644 --- a/packages/coding-agent/src/gjc-runtime/ultragoal-runtime.ts +++ b/packages/coding-agent/src/gjc-runtime/ultragoal-runtime.ts @@ -1,7 +1,6 @@ import * as crypto from "node:crypto"; import * as fs from "node:fs/promises"; import * as path from "node:path"; -import { getConfigRootDir } from "@gajae-code/utils"; import type { WorkflowHudSummary } from "../skill-state/active-state"; import { buildUltragoalHudSummary as buildWorkflowUltragoalHudSummary } from "../skill-state/workflow-hud"; import { renderCliWriteReceipt } from "./cli-write-receipt"; @@ -63,6 +62,8 @@ import { writeGuardedJsonAtomic, } from "./state-writer"; +import { resolveWorkflowSetting } from "./workflow-settings"; + export { captureUltragoalRecoverySnapshot, parseStrictTerminalTranscript, @@ -387,39 +388,22 @@ function parseNudgeBudgetValue(value: unknown): number | null { return typeof value === "number" && Number.isFinite(value) && Number.isInteger(value) && value >= 0 ? value : null; } -async function readSettingsNudgeBudget(settingsPath: string): Promise { - try { - const raw = await Bun.file(settingsPath).text(); - const parsed = JSON.parse(raw) as Record; - // Support both the flat dotted key and a nested gjc.ultragoal.nudgeBudget shape. - const flat = parseNudgeBudgetValue(parsed["gjc.ultragoal.nudgeBudget"]); - if (flat !== null) return flat; - const gjc = parsed.gjc; - if (gjc && typeof gjc === "object") { - const ultragoal = (gjc as Record).ultragoal; - if (ultragoal && typeof ultragoal === "object") { - return parseNudgeBudgetValue((ultragoal as Record).nudgeBudget); - } - } - return null; - } catch { - return null; - } -} - /** - * Resolve the per-story nudge budget. Project `./.gjc/settings.json` overrides the - * user settings (`$GJC_CONFIG_DIR/settings.json` or `~/.gjc/settings.json`), else the - * default. Mirrors the `gjc.deepInterview.ambiguityThreshold` user+project precedence. + * Resolve the per-story nudge budget through the shared five-layer resolver. + * Ultragoal stays tolerant: an invalid optional settings file continues to the + * next layer and finally to the built-in default (10). */ export async function resolveUltragoalNudgeBudget(cwd: string): Promise<{ budget: number; source: string }> { - const projectPath = path.join(gjcRoot(cwd), "settings.json"); - const project = await readSettingsNudgeBudget(projectPath); - if (project !== null) return { budget: project, source: projectPath }; - const userPath = path.join(getConfigRootDir(), "settings.json"); - const user = await readSettingsNudgeBudget(userPath); - if (user !== null) return { budget: user, source: userPath }; - return { budget: DEFAULT_ULTRAGOAL_NUDGE_BUDGET, source: "default" }; + const resolution = await resolveWorkflowSetting(cwd, "gjc.ultragoal.nudgeBudget", { + defaultValue: DEFAULT_ULTRAGOAL_NUDGE_BUDGET, + parse: value => { + const parsed = parseNudgeBudgetValue(value); + return parsed === null + ? { kind: "invalid", reason: "expected gjc.ultragoal.nudgeBudget to be a non-negative integer" } + : { kind: "valid", value: parsed }; + }, + }); + return { budget: resolution.value, source: resolution.source }; } /** diff --git a/packages/coding-agent/src/gjc-runtime/workflow-settings.ts b/packages/coding-agent/src/gjc-runtime/workflow-settings.ts new file mode 100644 index 0000000000..8838770e0f --- /dev/null +++ b/packages/coding-agent/src/gjc-runtime/workflow-settings.ts @@ -0,0 +1,600 @@ +/** + * Single source of precedence for the four workflow settings surfaces. + * + * Every workflow runtime (ralplan, ultragoal, deep-interview) reads its + * settings through {@link resolveWorkflowSetting}; no runtime hand-rolls file + * discovery, YAML/JSON parsing, or key extraction. The precedence is fixed: + * + * 1. project `.gjc/config.yml` + * 2. project `.gjc/settings.json` + * 3. user `/config.yml` (default `~/.gjc/agent/config.yml`) + * 4. user `/settings.json` (legacy, deprecated last resort) + * 5. built-in default + * + * Project configuration always beats user configuration, and modern YAML beats + * legacy JSON. Both flat dotted keys (`gjc.ralplan.maxIterations`) and nested + * shapes (`gjc: { ralplan: { maxIterations } }`) are accepted; flat wins when + * both occur in one document. + * + * This module must stay pure and acyclic: it imports only path helpers and the + * pure `gjcRoot`/`dirs` utilities, never `Settings`, discovery/capability + * loaders, or workflow runtimes. All config/agent paths are constructed inside + * each resolver call (never at module scope) because `dirs.ts` caches directory + * resolution at module load. + */ + +import { createHash } from "node:crypto"; +import * as fs from "node:fs/promises"; +import * as path from "node:path"; +import { + getAgentDir, + getConfigRootDir, + isEnoent, + resolveEquivalentPath, + standardizeMacOSPath, +} from "@gajae-code/utils"; +import { YAML } from "bun"; +import { gjcRoot } from "./session-layout"; + +export type WorkflowSettingKey = + | "gjc.deepInterview.ambiguityThreshold" + | "gjc.ralplan.autoHandoff" + | "gjc.ralplan.maxIterations" + | "gjc.ralplan.maxReviewPassesPerLane" + | "gjc.ultragoal.nudgeBudget"; + +export type WorkflowSettingLayer = "project-config" | "project-settings" | "agent-config" | "config-root-settings"; + +export type WorkflowSettingParseResult = { kind: "valid"; value: T } | { kind: "invalid"; reason: string }; + +export type WorkflowSettingDiagnosticStatus = "missing-file" | "empty-document" | "missing-key" | "invalid" | "valid"; + +export interface WorkflowSettingDiagnostic { + layer: WorkflowSettingLayer; + /** Lexical absolute candidate; missing paths stay actionable. */ + path: string; + format: "yaml" | "json"; + status: WorkflowSettingDiagnosticStatus; + classification?: "read" | "syntax" | "shape" | "value"; + reason?: string; +} + +export interface ResolveWorkflowSettingOptions { + defaultValue: T; + parse: (value: unknown) => WorkflowSettingParseResult; + /** Omitted means "continue"; ralplan passes "throw" explicitly. */ + invalidPolicy?: "throw" | "continue"; +} + +export interface WorkflowSettingResolution { + value: T; + /** Canonical realpath for a winning existing file, or "default". */ + source: string; + diagnostics: readonly WorkflowSettingDiagnostic[]; +} + +export type WorkflowSettingInvalidClassification = "read" | "syntax" | "shape" | "value"; + +/** Raised under the strict ("throw") invalid policy; stable properties for callers. */ +export class WorkflowSettingError extends Error { + readonly diagnostic: WorkflowSettingDiagnostic; + readonly path: string; + readonly layer: WorkflowSettingLayer; + readonly classification: WorkflowSettingInvalidClassification; + readonly reason: string; + + constructor( + diagnostic: WorkflowSettingDiagnostic & { + classification: WorkflowSettingInvalidClassification; + reason: string; + }, + ) { + super(`invalid workflow setting at ${diagnostic.path}: ${diagnostic.reason}`); + this.name = "WorkflowSettingError"; + this.diagnostic = diagnostic; + this.path = diagnostic.path; + this.layer = diagnostic.layer; + this.classification = diagnostic.classification; + this.reason = diagnostic.reason; + } +} + +const LAYER_CANDIDATES: ReadonlyArray<{ + layer: WorkflowSettingLayer; + format: "yaml" | "json"; + buildPath: (cwd: string) => string; +}> = [ + { layer: "project-config", format: "yaml", buildPath: cwd => path.resolve(gjcRoot(cwd), "config.yml") }, + { layer: "project-settings", format: "json", buildPath: cwd => path.resolve(gjcRoot(cwd), "settings.json") }, + { layer: "agent-config", format: "yaml", buildPath: () => path.resolve(getAgentDir(), "config.yml") }, + { + layer: "config-root-settings", + format: "json", + buildPath: () => path.resolve(getConfigRootDir(), "settings.json"), + }, +]; + +/** Must match settings.ts WORKFLOW_MIGRATION_MARKER_VERSION. */ +const WORKFLOW_MIGRATION_MARKER_VERSION = 1; +/** Must match settings.ts CONFIG_ROOT_WORKFLOW_MIGRATION_KEYS. */ +const WORKFLOW_MIGRATION_KEYS: readonly string[] = [ + "gjc.deepInterview.ambiguityThreshold", + "gjc.ralplan.autoHandoff", + "gjc.ralplan.maxIterations", + "gjc.ralplan.maxReviewPassesPerLane", + "gjc.ultragoal.nudgeBudget", +]; + +/** + * True when the one-time config-root migration has completed AND the source is + * still the migrated file: the `.migrated` marker passes the same + * version/shape checks as the migration's own reader, has status "complete", + * points at the same path, and the current source bytes still match the + * marker's sourceSha256. A completed migration deactivates the legacy source + * (removing a migrated target key returns to the default, not the legacy + * value), but a later edit or recreate of settings.json changes the bytes and + * REACTIVATES the documented legacy fallback. Malformed, version-mismatched, + * path-mismatched, source-mismatched, or target-profile-mismatched markers do + * not deactivate: the migration only copied values into the DEFAULT agent + * config, so a custom agentDir profile that never received the migrated value + * keeps the legacy fallback active. + */ +async function isConfigRootMigrationComplete(sourcePath: string): Promise { + const markerPath = `${sourcePath}.migrated`; + let raw: string; + try { + raw = await Bun.file(markerPath).text(); + } catch (error) { + // Only ENOENT means no marker; a transient EACCES/EIO read failure + // must propagate so a valid marker is never treated as absent. + if ((error as NodeJS.ErrnoException).code === "ENOENT") return false; + throw error; + } + try { + const marker = JSON.parse(raw) as Record; + if ( + marker.version !== WORKFLOW_MIGRATION_MARKER_VERSION || + marker.status !== "complete" || + typeof marker.sourcePath !== "string" || + path.resolve(marker.sourcePath) !== path.resolve(sourcePath) || + typeof marker.backupPath !== "string" || + path.resolve(marker.backupPath) !== path.resolve(`${sourcePath}.bak`) || + typeof marker.targetPath !== "string" || + path.resolve(marker.targetPath) !== path.resolve(getAgentDir(), "config.yml") || + typeof marker.sourceSha256 !== "string" || + !/^[0-9a-f]{64}$/.test(marker.sourceSha256) || + typeof marker.startedAt !== "string" || + Number.isNaN(Date.parse(marker.startedAt)) || + typeof marker.completedAt !== "string" || + Number.isNaN(Date.parse(marker.completedAt)) || + !Array.isArray(marker.migratedKeys) || + !marker.migratedKeys.every(key => typeof key === "string" && WORKFLOW_MIGRATION_KEYS.includes(key)) + ) { + return false; + } + // A symlinked agent dir REPOINTED after migration changes the canonical + // target identity. The marker stores the canonical agent dir at migration + // time; compare the CURRENT canonical dir against it (comparing two + // current resolutions alone cannot detect a repoint). + if (typeof marker.canonicalTargetDir === "string") { + const currentCanonicalAgentDir = await fs.realpath(getAgentDir()).catch(() => getAgentDir()); + if (marker.canonicalTargetDir !== currentCanonicalAgentDir) { + return false; + } + } + // A same-pathname profile REPLACEMENT (deleted + recreated) changes the + // target config.yml's dev:ino; the marker records the identity at + // migration time. + if (typeof marker.canonicalTargetIdentity === "string") { + const currentTargetIdentity = await fs.stat(getAgentDir()).catch(() => null); + if ( + currentTargetIdentity && + `${currentTargetIdentity.dev}:${currentTargetIdentity.ino}` !== marker.canonicalTargetIdentity + ) { + return false; + } + } + try { + // Hash the raw source bytes (Bun.file contract; matches the + // migration's raw-Buffer hash). + const sourceRaw = await Bun.file(sourcePath).arrayBuffer(); + return createHash("sha256").update(Buffer.from(sourceRaw)).digest("hex") === marker.sourceSha256; + } catch { + return false; + } + } catch { + return false; + } +} + +/** + * Matches Settings.#coerceWorkflowScalar: the migration writes quoted numerics + * as numbers into config.yml, so the ownership comparison must coerce the + * backup's raw JSON value before comparing against the agent-config value. + * Only autoHandoff is a string key among the migrated workflow settings. + */ +function coerceWorkflowScalar(key: WorkflowSettingKey, value: unknown): unknown { + if ( + key !== "gjc.ralplan.autoHandoff" && + typeof value === "string" && + value.trim() !== "" && + Number.isFinite(Number(value)) + ) { + return Number(value); + } + return value; +} +/** + * When a completed migration's legacy source was EDITED or DELETED afterwards + * (the marker hash no longer matches), the agent-config layer still holds the + * migration-written value for the marker's keys. Direct workflow commands + * (`gjc ralplan`/`deep-interview`/`ultragoal`) invoke the runtime without + * running Settings' reconcile, so the resolver must disregard those + * migration-owned agent values - otherwise the edited legacy value is shadowed + * and an invalid strict edit cannot exit 2. + * + * Returns the owned keys whose CURRENT agent-config value still matches the + * migration's write (the backup copy), or null when there is no stale + * DEFAULT-profile complete marker. A key whose agent value the user edited + * after migration is NOT owned (it is a genuine override). The marker's + * targetPath must be the current agent config: a custom agentDir profile that + * never received the migration is never suppressed. + */ +async function getStaleMigrationOwnedKeys(sourcePath: string): Promise | null> { + const markerPath = `${sourcePath}.migrated`; + let raw: string; + try { + raw = await Bun.file(markerPath).text(); + } catch (error) { + // Only ENOENT means no marker; a transient EACCES/EIO read failure + // must propagate so a valid marker is never treated as absent. + if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; + throw error; + } + try { + const marker = JSON.parse(raw) as Record; + if ( + marker.version !== WORKFLOW_MIGRATION_MARKER_VERSION || + (marker.status !== "complete" && marker.status !== "pending") || + typeof marker.sourcePath !== "string" || + path.resolve(marker.sourcePath) !== path.resolve(sourcePath) || + typeof marker.backupPath !== "string" || + path.resolve(marker.backupPath) !== path.resolve(`${sourcePath}.bak`) || + typeof marker.targetPath !== "string" || + path.resolve(marker.targetPath) !== path.resolve(getAgentDir(), "config.yml") || + typeof marker.sourceSha256 !== "string" || + !Array.isArray(marker.migratedKeys) + ) { + return null; + } + // A same-pathname profile REPLACEMENT changes the target config.yml's + // dev:ino; the marker records the identity at migration time. + if (typeof marker.canonicalTargetIdentity === "string") { + const currentTargetIdentity = await fs.stat(getAgentDir()).catch(() => null); + if ( + currentTargetIdentity && + `${currentTargetIdentity.dev}:${currentTargetIdentity.ino}` !== marker.canonicalTargetIdentity + ) { + return null; + } + } + // A symlinked agent dir repointed after migration changes the canonical + // target identity: the marker must have been created for the CURRENT + // canonical agent dir, or the new profile never received the migration. + if (typeof marker.canonicalTargetDir === "string") { + const currentCanonicalAgentDir = await fs.realpath(getAgentDir()).catch(() => getAgentDir()); + if (marker.canonicalTargetDir !== currentCanonicalAgentDir) { + return null; + } + } + let sourceRaw: string | null = null; + try { + sourceRaw = await Bun.file(sourcePath).text(); + } catch (error) { + if (!isEnoent(error)) return null; + // Deleted source: the migration treats deletion as a request to drop + // the copied values, so all migration-owned agent values are stale. + } + // A completed migration whose source matches is fully done. A PENDING + // reconcile marker (priorSourceSha256) recorded the edited source hash + // BEFORE repairing the target: the agent config still holds the OLD + // migration value, so the stale-owned keys must still be computed from + // the prior backup, not treated as completed. + if ( + sourceRaw !== null && + createHash("sha256").update(sourceRaw).digest("hex") === marker.sourceSha256 && + marker.priorSourceSha256 === undefined + ) { + return null; + } + let backupRaw: string; + try { + // The backup must hash to the marker's sourceSha256 (completed) OR to + // priorSourceSha256 (a pending reconcile before its refresh): an + // altered backup is not evidence of what the migration wrote. + const backupBytes = await Bun.file(marker.backupPath).arrayBuffer(); + const backupHash = createHash("sha256").update(Buffer.from(backupBytes)).digest("hex"); + if (backupHash !== marker.sourceSha256 && backupHash !== marker.priorSourceSha256) { + return null; + } + // Parse the SAME bytes that were verified (a second read could observe + // a different revision if the file changed between the two reads). + backupRaw = Buffer.from(backupBytes).toString("utf8"); + } catch { + return null; + } + const backupDoc = JSON.parse(backupRaw) as unknown; + let agentRaw: string; + try { + agentRaw = await Bun.file(path.resolve(getAgentDir(), "config.yml")).text(); + } catch { + return null; + } + const agentDoc = YAML.parse(agentRaw) as unknown; + const owned = new Set(); + for (const key of marker.migratedKeys) { + if (typeof key !== "string" || !WORKFLOW_MIGRATION_KEYS.includes(key)) continue; + const migrated = extractWorkflowSetting(backupDoc, key as WorkflowSettingKey); + const agentValue = extractWorkflowSetting(agentDoc, key as WorkflowSettingKey, { flat: false }); + // Only treat the agent value as migration-owned while it still matches + // the migration's write (the backup): a value the user edited after + // migration is a genuine override and must win. + const repairHashes = marker.repairValueHashes as Record | undefined; + if ( + agentValue.present && + ((migrated.present && + agentValue.value === coerceWorkflowScalar(key as WorkflowSettingKey, migrated.value)) || + // A reconcile that COMMITTED its repairs left the recorded + // repair values in the agent config: honor them even when the + // backup is not yet refreshed. A mere change from the + // pre-repair state is NOT proof (a coincidental user value + // could match). + (marker.repairsApplied === true && + repairHashes?.[key as string] !== undefined && + createHash("sha256").update(JSON.stringify(agentValue.value)).digest("hex") === + repairHashes?.[key as string])) + ) { + owned.add(key as WorkflowSettingKey); + } + } + return owned; + } catch { + return null; + } +} + +/** + * Extract a workflow key from a parsed settings document. Flat dotted keys are + * honored only for legacy JSON settings files (settings.json) - config.yml uses + * the nested (schema) form, so the public Settings/config CLI path (which + * addresses nested paths) can manage every effective override. Flat keys are + * checked before the nested `gjc: { ... }` shape (flat wins); an explicitly + * present `undefined` value counts as present. + */ +export function extractWorkflowSetting( + document: unknown, + key: WorkflowSettingKey, + options: { flat?: boolean } = {}, +): { present: boolean; value: unknown; malformedParent?: boolean } { + if (!document || typeof document !== "object" || Array.isArray(document)) { + return { present: false, value: undefined }; + } + const settings = document as Record; + if (options.flat !== false && Object.hasOwn(settings, key)) return { present: true, value: settings[key] }; + + const segments = key.split("."); + if (segments.length < 2 || segments[0] !== "gjc") return { present: false, value: undefined }; + // A PRESENT but non-mapping `gjc` (or any intermediate segment) is a + // malformed parent, not a missing key: strict callers must surface it as an + // invalid shape instead of silently continuing to a lower layer/default. + const hasGjc = Object.hasOwn(settings, "gjc"); + const gjc = settings.gjc; + if (gjc === null || typeof gjc !== "object" || Array.isArray(gjc)) { + return hasGjc + ? { present: false, value: undefined, malformedParent: true } + : { present: false, value: undefined }; + } + let current: unknown = gjc; + for (let index = 1; index < segments.length; index++) { + const record = current as Record; + if (!Object.hasOwn(record, segments[index]!)) return { present: false, value: undefined }; + const next = record[segments[index]!]; + if (index < segments.length - 1 && (next === null || typeof next !== "object" || Array.isArray(next))) { + return { present: false, value: undefined, malformedParent: true }; + } + current = next; + } + return { present: true, value: current }; +} + +/** + * Resolve a workflow setting across the fixed five-layer precedence. Returns the + * first valid configured value, otherwise {@link options.defaultValue} with + * `source: "default"`. Diagnostics are retained for unit tests and optional + * logging; runtime public wrappers expose their existing compact result shapes. + */ +export async function resolveWorkflowSetting( + cwd: string, + key: WorkflowSettingKey, + options: ResolveWorkflowSettingOptions, +): Promise> { + const invalidPolicy = options.invalidPolicy ?? "continue"; + const diagnostics: WorkflowSettingDiagnostic[] = []; + + const invalid = ( + layer: WorkflowSettingLayer, + candidatePath: string, + format: "yaml" | "json", + classification: WorkflowSettingInvalidClassification, + reason: string, + ): WorkflowSettingDiagnostic & { classification: WorkflowSettingInvalidClassification; reason: string } => ({ + layer, + path: candidatePath, + format, + status: "invalid", + classification, + reason, + }); + // Stale migration ownership is relevant only when considering the + // agent-config layer; compute it lazily there so a transient marker read + // failure cannot block higher-precedence project configuration. + let staleMigrationOwnedKeys: ReadonlySet | null | undefined; + + for (const candidate of LAYER_CANDIDATES) { + const candidatePath = candidate.buildPath(cwd); + // Direct workflow commands never run Settings' reconcile: when the + // config-root legacy source was edited after a completed migration, the + // agent-config layer still holds the stale migration-written value for + // the marker's keys - disregard it so the edited legacy value is + // effective (an invalid strict edit must exit 2). + if (candidate.layer === "agent-config") { + if (staleMigrationOwnedKeys === undefined) { + staleMigrationOwnedKeys = await getStaleMigrationOwnedKeys( + path.resolve(getConfigRootDir(), "settings.json"), + ); + } + if (staleMigrationOwnedKeys?.has(key)) { + continue; + } + } + // A completed one-time migration deactivates the legacy config-root + // source: never fall back to its stale values (removing a migrated + // target key returns to the default, not the legacy value). + if (candidate.layer === "config-root-settings" && (await isConfigRootMigrationComplete(candidatePath))) { + continue; + } + + let raw: string; + try { + raw = await Bun.file(candidatePath).text(); + } catch (error) { + if (isEnoent(error)) { + diagnostics.push({ + layer: candidate.layer, + path: candidatePath, + format: candidate.format, + status: "missing-file", + }); + continue; + } + const reason = error instanceof Error ? error.message : String(error); + const diagnostic = invalid(candidate.layer, candidatePath, candidate.format, "read", reason); + if (invalidPolicy === "throw") throw new WorkflowSettingError(diagnostic); + diagnostics.push(diagnostic); + continue; + } + + const trimmed = raw.trim(); + // Only a genuinely EMPTY file is "no explicit settings" - and empty + // content is valid YAML (an empty document) but INVALID JSON, so an + // empty settings.json must fall through to JSON.parse and strict ralplan + // fails closed (exit 2) on the malformed explicit layer. The literal + // text `undefined` likewise falls through to JSON.parse. + if (trimmed === "" && candidate.format !== "json") { + diagnostics.push({ + layer: candidate.layer, + path: candidatePath, + format: candidate.format, + status: "empty-document", + }); + continue; + } + + let parsed: unknown; + try { + parsed = candidate.format === "yaml" ? YAML.parse(raw) : JSON.parse(raw); + } catch { + // Stable, caller-agnostic reason; the underlying parse detail is not + // part of the runtime error contract. + const diagnostic = invalid( + candidate.layer, + candidatePath, + candidate.format, + "syntax", + candidate.format === "json" ? "malformed JSON" : "malformed YAML", + ); + if (invalidPolicy === "throw") throw new WorkflowSettingError(diagnostic); + diagnostics.push(diagnostic); + continue; + } + + // Only an EMPTY document (no content) is "no explicit settings": a + // parsed YAML/JSON `null` root is malformed per Settings.#loadYaml + // (which keeps the config read-only until repaired), so the strict + // contract must fail closed on it instead of continuing to defaults. + if (parsed === undefined) { + diagnostics.push({ + layer: candidate.layer, + path: candidatePath, + format: candidate.format, + status: "empty-document", + }); + continue; + } + if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) { + const diagnostic = invalid( + candidate.layer, + candidatePath, + candidate.format, + "shape", + `expected a settings mapping, got ${parsed === null ? "null" : Array.isArray(parsed) ? "an array" : typeof parsed}`, + ); + if (invalidPolicy === "throw") throw new WorkflowSettingError(diagnostic); + diagnostics.push(diagnostic); + continue; + } + + const extracted = extractWorkflowSetting(parsed, key, { flat: candidate.format === "json" }); + if (extracted.malformedParent) { + // A present non-mapping parent (e.g. `gjc: "invalid"` or + // `gjc: { ralplan: [] }`) is a malformed explicit layer: strict + // ralplan fails closed (exit 2) instead of silently treating the key + // as missing and falling to a lower layer/default. + const diagnostic = invalid( + candidate.layer, + candidatePath, + candidate.format, + "shape", + `expected a settings mapping for ${key}, got a non-mapping parent`, + ); + if (invalidPolicy === "throw") throw new WorkflowSettingError(diagnostic); + diagnostics.push(diagnostic); + continue; + } + if (!extracted.present) { + diagnostics.push({ + layer: candidate.layer, + path: candidatePath, + format: candidate.format, + status: "missing-key", + }); + continue; + } + + // Mirror Settings' schema scalar coercion before the workflow parser: a + // quoted numeric string for a number workflow key (e.g. + // `gjc.ralplan.maxIterations: "7"`) is coerced to a number, exactly as + // reconcileSettingsSchema treats number settings. Enum workflow keys + // never carry numeric strings, so the coercion is a no-op there. + const coercedValue = + typeof extracted.value === "string" && + extracted.value.trim() !== "" && + Number.isFinite(Number(extracted.value)) + ? Number(extracted.value) + : extracted.value; + const parsedValue = options.parse(coercedValue); + if (parsedValue.kind === "valid") { + return { + value: parsedValue.value, + source: standardizeMacOSPath(resolveEquivalentPath(candidatePath)), + diagnostics, + }; + } + + const diagnostic = invalid(candidate.layer, candidatePath, candidate.format, "value", parsedValue.reason); + if (invalidPolicy === "throw") throw new WorkflowSettingError(diagnostic); + diagnostics.push(diagnostic); + } + + return { value: options.defaultValue, source: "default", diagnostics }; +} diff --git a/packages/coding-agent/test/config/atomic-yaml-patch.test.ts b/packages/coding-agent/test/config/atomic-yaml-patch.test.ts index f7c6da29fb..98734e8583 100644 --- a/packages/coding-agent/test/config/atomic-yaml-patch.test.ts +++ b/packages/coding-agent/test/config/atomic-yaml-patch.test.ts @@ -9,6 +9,7 @@ import { AtomicYamlReplaceError, applyAtomicYamlPatches, atomicYamlPathHash, + withAtomicYamlConfigTransaction, } from "../../src/config/atomic-yaml-patch"; const temporaryDirectories: string[] = []; @@ -154,4 +155,98 @@ describe("atomic YAML patches", () => { const directoryEntries = await fs.readdir(path.dirname(configPath)); expect(directoryEntries.filter(entry => entry.endsWith(".tmp"))).toEqual([]); }); + test("transaction exposes root/current and applies patches under the lock", async () => { + const configPath = await configPathForTest(); + await fs.writeFile(configPath, YAML.stringify({ external: { keep: true } }, null, 2)); + + let observedRoot: unknown; + let observedCurrent: Record | undefined; + const result = await withAtomicYamlConfigTransaction(configPath, async tx => { + observedRoot = structuredClone(tx.root); + observedCurrent = structuredClone(tx.current); + await tx.applyPatches([{ path: "settings.first", op: "set", value: "A" }]); + await tx.applyPatches([{ path: "settings.second", op: "set", value: "B" }]); + return "done"; + }); + + expect(result).toBe("done"); + expect(observedRoot).toEqual({ external: { keep: true } }); + expect(observedCurrent).toEqual({ external: { keep: true } }); + expect(await readYaml(configPath)).toEqual({ + external: { keep: true }, + settings: { first: "A", second: "B" }, + }); + }); + + test("transaction surfaces a parse failure before the callback runs", async () => { + const configPath = await configPathForTest(); + await fs.writeFile(configPath, "broken: [unclosed", "utf8"); + + let callbackRan = false; + await expect( + withAtomicYamlConfigTransaction(configPath, async () => { + callbackRan = true; + return "unreachable"; + }), + ).rejects.toThrow(); + expect(callbackRan).toBe(false); + }); + + test("transaction exposes a scalar/array root without writing", async () => { + const configPath = await configPathForTest(); + await fs.writeFile(configPath, YAML.stringify(["a", "b"], null, 2)); + + let observedRoot: unknown = "unset"; + await withAtomicYamlConfigTransaction(configPath, async tx => { + observedRoot = tx.root; + return "noop"; + }); + + expect(observedRoot).toEqual(["a", "b"]); + expect(YAML.parse(await fs.readFile(configPath, "utf8"))).toEqual(["a", "b"]); + }); + test("transaction removes dotted top-level keys verbatim", async () => { + const configPath = await configPathForTest(); + await fs.writeFile( + configPath, + YAML.stringify({ "gjc.ralplan.maxIterations": "bad", gjc: { ralplan: { maxIterations: 7 } } }, null, 2), + ); + + await withAtomicYamlConfigTransaction(configPath, async tx => { + const receipt = await tx.removeTopLevelKeys(["gjc.ralplan.maxIterations"]); + expect((await receipt.restore()).status).toBe("not-restorable"); + return "done"; + }); + + expect(YAML.parse(await fs.readFile(configPath, "utf8"))).toEqual({ gjc: { ralplan: { maxIterations: 7 } } }); + }); + test("transaction replaces the whole document atomically", async () => { + const configPath = await configPathForTest(); + await fs.writeFile(configPath, YAML.stringify({ old: { keep: false }, theme: { dark: "red" } }, null, 2)); + + await withAtomicYamlConfigTransaction(configPath, async tx => { + await tx.replaceCurrent({ theme: { dark: "blue" } }); + return "done"; + }); + + expect(YAML.parse(await fs.readFile(configPath, "utf8"))).toEqual({ theme: { dark: "blue" } }); + }); + test("an external edit between the transaction read and write is not overwritten", async () => { + const target = await configPathForTest(); + await fs.writeFile(target, YAML.stringify({ a: 1 }, null, 2)); + + await expect( + withAtomicYamlConfigTransaction(target, async tx => { + // Simulate an external editor saving config.yml after the + // transaction read it (external editors do not take the lock). + await fs.writeFile(target, YAML.stringify({ a: 99 }, null, 2)); + await tx.applyPatches([{ path: "b", op: "set", value: 2 }]); + }), + ).rejects.toThrow(/precondition failed/i); + + // The external edit is preserved, not overwritten by the stale snapshot. + const after = YAML.parse(await fs.readFile(target, "utf8")) as Record; + expect(after.a).toBe(99); + expect(after.b).toBeUndefined(); + }); }); diff --git a/packages/coding-agent/test/config/settings-workflow-migration.test.ts b/packages/coding-agent/test/config/settings-workflow-migration.test.ts new file mode 100644 index 0000000000..16c0a16f9a --- /dev/null +++ b/packages/coding-agent/test/config/settings-workflow-migration.test.ts @@ -0,0 +1,1241 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { createHash } from "node:crypto"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { YAML } from "bun"; + +const PROBE = path.join(import.meta.dir, "../fixtures/settings-workflow-migration-probe.ts"); + +const temporaryDirectories: string[] = []; + +async function tempDir(): Promise { + const directory = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-migration-")); + temporaryDirectories.push(directory); + return directory; +} + +afterEach(async () => { + await Promise.all( + temporaryDirectories.splice(0).map(directory => fs.rm(directory, { recursive: true, force: true })), + ); +}); + +type ProbeResult = { + sourceExists: boolean; + backupExists: boolean; + markerExists: boolean; + markerStatus: string | null; + targetValue: unknown; +}; + +async function runProbe( + cwd: string, + options: { home: string; configDir?: string; agentDir?: string }, +): Promise { + const args = [process.execPath, PROBE]; + if (options.agentDir) args.push("--agent-dir", options.agentDir); + const proc = Bun.spawn(args, { + cwd, + env: { ...process.env, HOME: options.home, GJC_CONFIG_DIR: options.configDir ?? ".gjc" }, + stdout: "pipe", + stderr: "pipe", + }); + const [out, err] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]); + if ((await proc.exited) !== 0) throw new Error(`probe failed (exit ${await proc.exited}): ${err}`); + return JSON.parse(out.trim()) as ProbeResult; +} + +async function setupHome( + home: string, + configDir: string, +): Promise<{ configRoot: string; source: string; agentDir: string }> { + const configRoot = path.join(home, configDir); + await fs.mkdir(configRoot, { recursive: true }); + return { + configRoot, + source: path.join(configRoot, "settings.json"), + agentDir: path.join(configRoot, "agent"), + }; +} + +describe("config-root workflow settings migration", () => { + test("migrates the workflow keys into the default agent config.yml exactly once", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source } = await setupHome(home, ".myconfig"); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + + const first = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(first.markerStatus).toBe("complete"); + expect(first.backupExists).toBe(true); + expect(first.sourceExists).toBe(true); // source kept active (shadowed by config.yml) + expect(first.targetValue).toBe(7); + + const second = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(second.markerStatus).toBe("complete"); + expect(second.sourceExists).toBe(true); // still kept (shadowed) after re-load + expect(second.backupExists).toBe(true); + }); + + test("runs even when the target config.yml already exists", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + await fs.writeFile(path.join(agentDir, "config.yml"), YAML.stringify({ theme: { dark: "red-claw" } }, null, 2)); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBe("complete"); + expect(result.targetValue).toBe(7); + expect(result.backupExists).toBe(true); + }); + + test("does not overwrite a modern nested target value (absent-only)", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + await fs.writeFile( + path.join(agentDir, "config.yml"), + YAML.stringify({ gjc: { ralplan: { maxIterations: 9 } } }, null, 2), + ); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.targetValue).toBe(9); // modern nested target wins + expect(result.markerStatus).toBe("complete"); + expect(result.backupExists).toBe(true); + }); + + test("does nothing when the config-root source is absent", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + await setupHome(home, ".myconfig"); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerExists).toBe(false); + expect(result.backupExists).toBe(false); + }); + + test("leaves a malformed source untouched", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source } = await setupHome(home, ".myconfig"); + await fs.writeFile(source, "{ broken json"); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerExists).toBe(false); + expect(result.backupExists).toBe(false); + expect(result.sourceExists).toBe(true); + }); + + test("custom agentDir can never consume the machine-global source", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const otherAgent = await tempDir(); + const { source } = await setupHome(home, ".myconfig"); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + + const result = await runProbe(cwd, { home, configDir: ".myconfig", agentDir: otherAgent }); + expect(result.markerExists).toBe(false); + expect(result.backupExists).toBe(false); + expect(result.sourceExists).toBe(true); + expect(result.targetValue).toBe(null); + }); + + test("a pre-existing .bak without a marker is never consumed or overwritten", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source } = await setupHome(home, ".myconfig"); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + await fs.writeFile(`${source}.bak`, "pre-existing backup"); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerExists).toBe(false); + expect(result.backupExists).toBe(true); + expect(result.sourceExists).toBe(true); + }); + + test("concurrent loads serialize into one migration", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source } = await setupHome(home, ".myconfig"); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + + const [first, second] = await Promise.all([ + runProbe(cwd, { home, configDir: ".myconfig" }), + runProbe(cwd, { home, configDir: ".myconfig" }), + ]); + expect(first.markerStatus).toBe("complete"); + expect(second.markerStatus).toBe("complete"); + expect(first.targetValue).toBe(7); + expect(second.targetValue).toBe(7); + }); + + test("recovers a valid pending marker whose source was already consumed", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + const sourceRaw = JSON.stringify({ "gjc.ralplan.maxIterations": 7 }); + const sourceSha256 = createHash("sha256").update(sourceRaw).digest("hex"); + await fs.mkdir(agentDir, { recursive: true }); + // Simulate a crash after the patch and source move but before finalization. + await fs.writeFile( + path.join(agentDir, "config.yml"), + YAML.stringify({ gjc: { ralplan: { maxIterations: 7 } } }, null, 2), + ); + await fs.writeFile(source, sourceRaw); + await fs.rename(source, `${source}.bak`); + await fs.writeFile( + `${source}.migrated`, + JSON.stringify({ + version: 1, + status: "pending", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: path.join(agentDir, "config.yml"), + sourceSha256, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + // The copy path NEVER moves the source, so backup + no source is an + // external DELETION: the migration reverts the marker-owned target value, + // removes the backup, and clears the marker (instead of finalizing and + // silently restoring the deleted override). + expect(result.markerStatus).toBeNull(); + expect(result.backupExists).toBe(false); + expect(result.targetValue).toBeNull(); + }); + + test("a scalar/array target root aborts without touching anything", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + await fs.writeFile(path.join(agentDir, "config.yml"), JSON.stringify(["a", "b"])); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerExists).toBe(false); + expect(result.backupExists).toBe(false); + expect(result.sourceExists).toBe(true); + }); + test("pending yes/yes with a target that lacks the migrated keys does not delete the source", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + const sourceRaw = JSON.stringify({ "gjc.ralplan.maxIterations": 7 }); + const sourceSha256 = createHash("sha256").update(sourceRaw).digest("hex"); + await fs.mkdir(agentDir, { recursive: true }); + // Target exists but the patch never applied (e.g. a user-created backup + // with identical content): the source must NOT be dropped. + await fs.writeFile(path.join(agentDir, "config.yml"), YAML.stringify({ theme: { dark: "red-claw" } }, null, 2)); + await fs.writeFile(source, sourceRaw); + await fs.writeFile(`${source}.bak`, sourceRaw); + await fs.writeFile( + `${source}.migrated`, + JSON.stringify({ + version: 1, + status: "pending", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: path.join(agentDir, "config.yml"), + sourceSha256, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBe("pending"); // not finalized + expect(result.sourceExists).toBe(true); // source never deleted + expect(result.backupExists).toBe(true); + }); + + test("a complete marker whose paths do not match the current layout is ignored", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source } = await setupHome(home, ".myconfig"); + const sourceRaw = JSON.stringify({ "gjc.ralplan.maxIterations": 7 }); + const sourceSha256 = createHash("sha256").update(sourceRaw).digest("hex"); + // Stale marker pointing at a different config-root layout. + await fs.writeFile(source, sourceRaw); + await fs.writeFile( + `${source}.migrated`, + JSON.stringify({ + version: 1, + status: "complete", + sourcePath: "/elsewhere/settings.json", + backupPath: "/elsewhere/settings.json.bak", + targetPath: "/elsewhere/agent/config.yml", + sourceSha256, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBe("complete"); + expect(result.backupExists).toBe(true); + expect(result.sourceExists).toBe(true); // source kept active (shadowed by config.yml) + expect(result.targetValue).toBe(7); + }); + + test("a malformed marker is quarantined and a fresh migration completes", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source } = await setupHome(home, ".myconfig"); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + await fs.writeFile(`${source}.migrated`, "{ not valid json"); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBe("complete"); + expect(result.backupExists).toBe(true); + expect(result.targetValue).toBe(7); + const quarantined = await fs + .lstat(`${source}.migrated.corrupt`) + .then(() => true) + .catch(() => false); + expect(quarantined).toBe(true); + }); + + test("pending yes/yes with matching hashes and a satisfied target dedupes the source", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + const sourceRaw = JSON.stringify({ "gjc.ralplan.maxIterations": 7 }); + const sourceSha256 = createHash("sha256").update(sourceRaw).digest("hex"); + await fs.mkdir(agentDir, { recursive: true }); + // Crash after patch + move, before finalization: target patched, S and B both present. + await fs.writeFile( + path.join(agentDir, "config.yml"), + YAML.stringify({ gjc: { ralplan: { maxIterations: 7 } } }, null, 2), + ); + await fs.writeFile(source, sourceRaw); + await fs.writeFile(`${source}.bak`, sourceRaw); + await fs.writeFile( + `${source}.migrated`, + JSON.stringify({ + version: 1, + status: "pending", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: path.join(agentDir, "config.yml"), + sourceSha256, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBe("complete"); + expect(result.sourceExists).toBe(true); // source kept active (resolver deactivates it while it matches) + expect(result.backupExists).toBe(true); + expect(result.targetValue).toBe(7); + }); + test("a flat invalid target key is replaced by the valid migrated value", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + // Target uses the accepted flat YAML form with an INVALID value; the flat + // key wins extraction over the nested form, so it must be removed when + // the valid legacy value is migrated to the nested path. + await fs.writeFile( + path.join(agentDir, "config.yml"), + YAML.stringify({ "gjc.ralplan.maxIterations": "bad" }, null, 2), + ); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBe("complete"); + expect(result.backupExists).toBe(true); + expect(result.targetValue).toBe(7); + // The flat invalid key must be gone so resolution sees the nested value. + const parsed = YAML.parse(await fs.readFile(path.join(agentDir, "config.yml"), "utf8")) as Record< + string, + unknown + >; + expect(Object.hasOwn(parsed, "gjc.ralplan.maxIterations")).toBe(false); + }); + test("invalid legacy values are not copied into the durable config.yml", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + await fs.writeFile( + source, + JSON.stringify({ "gjc.ultragoal.nudgeBudget": "bad", "gjc.ralplan.maxIterations": 7 }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBe("complete"); + expect(result.backupExists).toBe(true); + expect(result.targetValue).toBe(7); // valid key migrated + // The invalid nudgeBudget must NOT have been written into config.yml. + const parsed = YAML.parse(await fs.readFile(path.join(agentDir, "config.yml"), "utf8")) as Record< + string, + unknown + >; + const gjc = parsed.gjc as Record | undefined; + expect(gjc?.ultragoal).toBeUndefined(); + }); + test("a malformed target config.yml does not abort settings load when there is nothing to migrate", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + await fs.writeFile(path.join(agentDir, "config.yml"), "gjc: [unclosed", "utf8"); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + // Load must succeed (no throw from the migration), with no marker/backup. + expect(result.markerExists).toBe(false); + expect(result.backupExists).toBe(false); + expect(result.sourceExists).toBe(false); + }); + + test("a malformed target config.yml with a valid source leaves the source untouched", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + await fs.writeFile(path.join(agentDir, "config.yml"), "gjc: [unclosed", "utf8"); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + // Load survives; the migration warns and leaves source/backup/marker untouched. + expect(result.sourceExists).toBe(true); + expect(result.backupExists).toBe(false); + expect(result.markerExists).toBe(false); + }); + + test("an invalid target value does not block the patch: the valid legacy value wins", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + // Target has an INVALID value for the strict key; the legacy source has a valid one. + await fs.writeFile( + path.join(agentDir, "config.yml"), + YAML.stringify({ gjc: { ralplan: { maxIterations: "not-a-number" } } }, null, 2), + ); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBe("complete"); + expect(result.backupExists).toBe(true); + expect(result.targetValue).toBe(7); // valid legacy value patched over the invalid one + }); + test("an invalid strict ralplan legacy value keeps the source active (loud failure preserved)", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + // Invalid STRICT key: consuming the source would silently fall back to + // defaults instead of letting gjc ralplan fail loudly (exit 2). + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": "bad" })); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.sourceExists).toBe(true); // source kept active + expect(result.backupExists).toBe(false); + expect(result.markerExists).toBe(false); + }); + test("a future-schema target config.yml is left read-only (migration skipped)", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + await fs.writeFile( + path.join(agentDir, "config.yml"), + YAML.stringify({ configSchemaVersion: 999, theme: { dark: "red-claw" } }, null, 2), + ); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.sourceExists).toBe(true); // legacy source stays active + expect(result.backupExists).toBe(false); + expect(result.markerExists).toBe(false); + }); + test("a quoted numeric target value is valid and not overwritten by the migration", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + // The resolver/Settings coerce quoted numerics; the migration must too, + // so it neither overwrites this target nor treats the legacy value oddly. + await fs.writeFile( + path.join(agentDir, "config.yml"), + YAML.stringify({ gjc: { ralplan: { maxIterations: "9" } } }, null, 2), + ); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBe("complete"); + expect(result.backupExists).toBe(true); + expect(result.targetValue).toBe("9"); // quoted 9 is valid; legacy 7 not patched over it + }); + test("a valid target override lets the migration proceed past an invalid strict legacy value", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + // The target already carries a VALID maxIterations, so the invalid legacy + // value would never win in the resolver; the migration must not abort on + // it and must still migrate the other valid legacy keys. + await fs.writeFile( + path.join(agentDir, "config.yml"), + YAML.stringify({ gjc: { ralplan: { maxIterations: 9 } } }, null, 2), + ); + await fs.writeFile( + source, + JSON.stringify({ "gjc.ralplan.maxIterations": "bad", "gjc.ultragoal.nudgeBudget": 3 }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBe("complete"); // migration not aborted + expect(result.backupExists).toBe(true); + expect(result.targetValue).toBe(9); // target override preserved + }); + test("a null YAML target root aborts the migration like a malformed config", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + // YAML `null`/`~` root: #loadYaml treats it as malformed (read-only), so + // the migration must not write into it or consume the legacy source. + await fs.writeFile(path.join(agentDir, "config.yml"), "null\n", "utf8"); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.sourceExists).toBe(true); + expect(result.backupExists).toBe(false); + expect(result.markerExists).toBe(false); + }); + test("quoted numeric legacy values are written coerced into config.yml", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": "7" })); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBe("complete"); + expect(result.backupExists).toBe(true); + expect(result.targetValue).toBe(7); // number, not the raw "7" string + }); + test("a null legacy source root keeps the source active (strict failure preserved)", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + // The strict resolver treats a null settings root as an invalid shape + // (exit 2); consuming it via an empty migration would silently default. + await fs.writeFile(source, "null", "utf8"); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.sourceExists).toBe(true); + expect(result.backupExists).toBe(false); + expect(result.markerExists).toBe(false); + }); + test("a changed pending source is reapplied over the stale target patch", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + // A crashed run patched the OLD legacy value into config.yml... + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { maxIterations: 7 } } }, null, 2)); + // ...and the user edited settings.json before the next load. + await fs.writeFile(source, '{"gjc.ralplan.maxIterations":9}', "utf8"); + const oldSourceHash = createHash("sha256").update('{"gjc.ralplan.maxIterations":7}').digest("hex"); + // The pending marker records the OLD source hash (from the crashed run). + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "pending", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + // Ownership is unverifiable without a backup: the recovery ABORTS (the + // source stays active, the marker stays pending) instead of completing + // with the key omitted from migratedKeys. + expect(result.markerStatus).toBe("pending"); + expect(result.sourceExists).toBe(true); + expect(result.targetValue).toBe(7); // unverifiable target kept + }); + test("an in-place source edit after completion does not mutate the backup", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBe("complete"); + // The backup is an INDEPENDENT copy: an in-place edit of the still-active + // source must not mutate the .bak, so the marker hash keeps describing + // the migrated bytes. + const marker = JSON.parse(await fs.readFile(path.join(home, ".myconfig", "settings.json.migrated"), "utf8")) as { + sourceSha256: string; + }; + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 99 }), "utf8"); + const backupRaw = await fs.readFile(`${source}.bak`, "utf8"); + expect(createHash("sha256").update(backupRaw).digest("hex")).toBe(marker.sourceSha256); + }); + test("a removed pending source key drops its stale target value", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + // A crashed run patched maxIterations 7 into config.yml... + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { maxIterations: 7 } } }, null, 2)); + // ...and the user REMOVED the key from settings.json before the next load. + await fs.writeFile(source, "{}", "utf8"); + const oldSourceHash = createHash("sha256").update('{"gjc.ralplan.maxIterations":7}').digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "pending", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + // Ownership is unverifiable without a backup: the recovery ABORTS (the + // source stays active, the marker stays pending) instead of completing. + expect(result.markerStatus).toBe("pending"); + expect(result.sourceExists).toBe(true); + expect(result.targetValue).toBe(7); // unverifiable target kept + }); + test("changed-pending recovery does not clobber unrecorded target overrides", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + // config.yml already carried a valid USER value for maxIterations, so the + // crashed migration did NOT record that key; the source is then edited. + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { maxIterations: 9 } } }, null, 2)); + await fs.writeFile(source, '{"gjc.ralplan.maxIterations":11}'); + const oldSourceHash = createHash("sha256").update('{"gjc.ralplan.maxIterations":7}').digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "pending", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ultragoal.nudgeBudget"], // NOT maxIterations + startedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.targetValue).toBe(9); // the user target override is preserved + }); + + test("changed-pending recovery unsets a stale tolerant patch for an invalid source", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + // The crashed run wrote nudgeBudget 7 into config.yml; the user then set + // the source nudgeBudget to an INVALID value before the retry. + await fs.writeFile(target, YAML.stringify({ gjc: { ultragoal: { nudgeBudget: 7 } } }, null, 2)); + await fs.writeFile(source, '{"gjc.ultragoal.nudgeBudget":"bad"}'); + const oldSourceHash = createHash("sha256").update('{"gjc.ultragoal.nudgeBudget":7}').digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "pending", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ultragoal.nudgeBudget"], + startedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBe("complete"); + // The stale target patch must be gone so the tolerant runtime falls back. + const parsed = YAML.parse(await fs.readFile(target, "utf8")) as Record; + const ultragoal = (parsed.gjc as Record | undefined)?.ultragoal as + | Record + | undefined; + expect(ultragoal?.nudgeBudget).toBe(7); // unverifiable without a backup: kept + }); + + test("changed-pending recovery removes the stale strict patch before aborting", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + // The crashed run wrote maxIterations 7; the user then set the source to + // an INVALID strict value, so the migration must abort but FIRST remove + // the stale target patch (otherwise the stale valid value would shadow + // the invalid legacy source and gjc ralplan would not exit 2). + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { maxIterations: 7 } } }, null, 2)); + await fs.writeFile(source, '{"gjc.ralplan.maxIterations":"bad"}'); + const oldSourceHash = createHash("sha256").update('{"gjc.ralplan.maxIterations":7}').digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "pending", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.sourceExists).toBe(true); // strict failure preserved + const parsed = YAML.parse(await fs.readFile(target, "utf8")) as Record; + const ralplan = (parsed.gjc as Record | undefined)?.ralplan as + | Record + | undefined; + expect(ralplan?.maxIterations).toBe(7); // unverifiable without a backup: kept + }); + test("changed-pending strict abort applies all queued stale-key repairs", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + // The crashed run wrote BOTH a threshold and maxIterations; the user then + // REMOVED the threshold and set maxIterations to an INVALID strict value. + await fs.writeFile( + target, + YAML.stringify( + { gjc: { deepInterview: { ambiguityThreshold: 0.9 }, ralplan: { maxIterations: 7 } } }, + null, + 2, + ), + ); + await fs.writeFile(source, '{"gjc.ralplan.maxIterations":"bad"}'); + const oldSourceHash = createHash("sha256") + .update('{"gjc.deepInterview.ambiguityThreshold":0.9,"gjc.ralplan.maxIterations":7}') + .digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "pending", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.deepInterview.ambiguityThreshold", "gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.sourceExists).toBe(true); // strict failure preserved + const parsed = YAML.parse(await fs.readFile(target, "utf8")) as Record; + const gjc = parsed.gjc as Record | undefined; + const ralplan = gjc?.ralplan as Record | undefined; + expect(ralplan?.maxIterations).toBe(7); // unverifiable without a backup: kept + const deepInterview = gjc?.deepInterview as Record | undefined; + // The removed threshold's ownership is unverifiable without a backup + // (W6MMR): it is left untouched rather than blindly unset. + expect(deepInterview?.ambiguityThreshold).toBe(0.9); + }); + test("the strict abort commits only marker-owned repairs, not fresh unrecorded keys", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + // The marker owns ONLY autoHandoff (processed BEFORE the invalid ralplan + // key); the edited source also adds an UNRECORDED threshold (processed + // even earlier, so its SET is queued) - the strict abort must commit the + // autoHandoff repair but NOT the unrecorded threshold. + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { autoHandoff: "team" } } }, null, 2)); + await fs.writeFile( + source, + '{"gjc.deepInterview.ambiguityThreshold":0.8,"gjc.ralplan.autoHandoff":"off","gjc.ralplan.maxIterations":"bad"}', + ); + const oldSourceHash = createHash("sha256").update('{"gjc.ralplan.autoHandoff":"team"}').digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "pending", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ralplan.autoHandoff"], + startedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.sourceExists).toBe(true); // strict failure preserved + const parsed = YAML.parse(await fs.readFile(target, "utf8")) as Record; + const gjc = parsed.gjc as Record | undefined; + const ralplan = gjc?.ralplan as Record | undefined; + expect(ralplan?.autoHandoff).toBe("team"); // unverifiable without a backup: kept + expect(ralplan?.maxIterations).toBeUndefined(); + const deepInterview = gjc?.deepInterview as Record | undefined; + expect(deepInterview?.ambiguityThreshold).toBeUndefined(); // unrecorded key NOT committed + }); + test("a crash then a source edit recovers via the changed-source repair", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + const oldRaw = '{"gjc.ralplan.maxIterations":7}'; + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { maxIterations: 7 } } }, null, 2)); + await fs.writeFile(`${source}.bak`, oldRaw); // backup matches the marker + await fs.writeFile(source, '{"gjc.ralplan.maxIterations":9}'); // user edited the source + const oldSourceHash = createHash("sha256").update(oldRaw).digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "pending", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + // The stale marker-owned value is reverted, the backup/marker cleared, + // so the edited source becomes effective (fresh re-migration on the next + // load would also re-apply 9). + expect(result.markerStatus).toBeNull(); + expect(result.backupExists).toBe(false); + expect(result.targetValue).toBeNull(); + }); + + test("editing the legacy source after completion re-migrates the current value", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + const oldRaw = '{"gjc.ralplan.maxIterations":7}'; + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { maxIterations: 7 } } }, null, 2)); + await fs.writeFile(`${source}.bak`, oldRaw); // the migration copy (old source) + await fs.writeFile(source, '{"gjc.ralplan.maxIterations":9}'); // user edited after completion + const oldSourceHash = createHash("sha256").update(oldRaw).digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "complete", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + // The stale complete marker is invalidated and the current source value + // (9) is re-migrated over the stale 7. + expect(result.markerStatus).toBe("complete"); + expect(result.targetValue).toBe(9); + }); + + test("a user-edited target value is kept during stale-complete re-migration", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + const oldRaw = '{"gjc.ralplan.maxIterations":7}'; + // The user edited the TARGET to 11 AFTER the migration... + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { maxIterations: 11 } } }, null, 2)); + await fs.writeFile(`${source}.bak`, oldRaw); // migration copy (old value 7) + // ...and the legacy source to 9. + await fs.writeFile(source, '{"gjc.ralplan.maxIterations":9}'); + const oldSourceHash = createHash("sha256").update(oldRaw).digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "complete", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + // The user's NEWER target value (11) is not the migration's write (7), so + // the re-migration must NOT clobber it. + expect(result.markerStatus).toBe("complete"); + expect(result.targetValue).toBe(11); + }); + test("deleting the source after completion keeps a user-edited target override", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + const oldRaw = '{"gjc.ralplan.maxIterations":7}'; + // The user ran `gjc config set` to 11 AFTER the migration, then deleted + // the legacy source. + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { maxIterations: 11 } } }, null, 2)); + await fs.writeFile(`${source}.bak`, oldRaw); // migration copy (7) + const oldSourceHash = createHash("sha256").update(oldRaw).digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "complete", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + }), + ); + // source is absent (deleted) + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + // The user's override (11) is NOT the migration's write (7), so it is kept. + expect(result.markerStatus).toBeNull(); + expect(result.backupExists).toBe(false); + expect(result.targetValue).toBe(11); + }); + + test("a second legacy edit after re-migration is still reconciled", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + const oldRaw = '{"gjc.ralplan.maxIterations":7}'; + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { maxIterations: 7 } } }, null, 2)); + await fs.writeFile(`${source}.bak`, oldRaw); + await fs.writeFile(source, '{"gjc.ralplan.maxIterations":9}'); + const oldSourceHash = createHash("sha256").update(oldRaw).digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "complete", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + }), + ); + + expect((await runProbe(cwd, { home, configDir: ".myconfig" })).targetValue).toBe(9); + // The backup is REFRESHED (not removed) after the first re-migration, so a + // SECOND edit still has a comparison basis. + await fs.writeFile(source, '{"gjc.ralplan.maxIterations":12}'); + expect((await runProbe(cwd, { home, configDir: ".myconfig" })).targetValue).toBe(12); + }); + + test("a workflow key added to the source after completion is migrated", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + const oldRaw = '{"gjc.ralplan.maxIterations":7}'; + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { maxIterations: 7 } } }, null, 2)); + await fs.writeFile(`${source}.bak`, oldRaw); + // The user ADDS a previously absent key (nudgeBudget) after completion. + await fs.writeFile(source, '{"gjc.ralplan.maxIterations":7,"gjc.ultragoal.nudgeBudget":5}'); + const oldSourceHash = createHash("sha256").update(oldRaw).digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "complete", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBe("complete"); + const parsed = YAML.parse(await fs.readFile(target, "utf8")) as Record; + const gjc = parsed.gjc as Record | undefined; + const ultragoal = gjc?.ultragoal as Record | undefined; + expect(ultragoal?.nudgeBudget).toBe(5); // the newly added key is copied + }); + + test("a malformed source parent aborts the initial migration", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + await fs.writeFile(source, '{"gjc":{"ralplan":"broken"}}'); // non-mapping parent + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + // The source stays active (strict ralplan fails on it) - no completion. + expect(result.sourceExists).toBe(true); + expect(result.markerStatus).toBeNull(); + }); + + test("an edited source with an invalid root leaves everything unchanged", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + const oldRaw = '{"gjc.ralplan.maxIterations":7}'; + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { maxIterations: 7 } } }, null, 2)); + await fs.writeFile(`${source}.bak`, oldRaw); + await fs.writeFile(source, "null"); // edited to an invalid root + const oldSourceHash = createHash("sha256").update(oldRaw).digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "complete", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + // The malformed source is not accepted; the target and marker stay intact + // (strict ralplan fails on the malformed source via the resolver). + expect(result.markerStatus).toBe("complete"); + expect(result.targetValue).toBe(7); + }); + test("a second edit to a reconcile-copied key is propagated", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + const oldRaw = '{"gjc.ralplan.maxIterations":7}'; + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { maxIterations: 7 } } }, null, 2)); + await fs.writeFile(`${source}.bak`, oldRaw); + await fs.writeFile(source, '{"gjc.ralplan.maxIterations":7,"gjc.ultragoal.nudgeBudget":5}'); + const oldSourceHash = createHash("sha256").update(oldRaw).digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "complete", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + }), + ); + + // First load copies the newly added nudgeBudget into the target. + const first = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(first.markerStatus).toBe("complete"); + // The SECOND edit to the copied key must be honored (the marker now owns + // the key via migratedKeys). + await fs.writeFile(source, '{"gjc.ralplan.maxIterations":7,"gjc.ultragoal.nudgeBudget":8}'); + const second = await runProbe(cwd, { home, configDir: ".myconfig" }); + const parsed = YAML.parse(await fs.readFile(target, "utf8")) as Record; + const ultragoal = (parsed.gjc as Record | undefined)?.ultragoal as + | Record + | undefined; + expect(ultragoal?.nudgeBudget).toBe(8); + expect(second.markerStatus).toBe("complete"); + }); + test("an invalid edited value is not copied during reconciliation", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + const oldRaw = '{"gjc.ultragoal.nudgeBudget":7}'; + await fs.writeFile(target, YAML.stringify({ gjc: { ultragoal: { nudgeBudget: 7 } } }, null, 2)); + await fs.writeFile(`${source}.bak`, oldRaw); + await fs.writeFile(source, '{"gjc.ultragoal.nudgeBudget":"bad"}'); // invalid edit + const oldSourceHash = createHash("sha256").update(oldRaw).digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "complete", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ultragoal.nudgeBudget"], + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + // The invalid value is NOT written; the stale migration-write stays and + // the marker is not updated (the legacy layer stays reactivated). + const parsed = YAML.parse(await fs.readFile(target, "utf8")) as Record; + const ultragoal = (parsed.gjc as Record | undefined)?.ultragoal as + | Record + | undefined; + expect(ultragoal?.nudgeBudget).toBe(7); // the migration-write, not "bad" + expect(result.markerStatus).toBe("complete"); + }); + + test("pending deletion recovery preserves a user-edited target override", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + const oldRaw = '{"gjc.ralplan.maxIterations":7}'; + // Crash after patch+backup; the user then set 11 via config set and + // deleted the source. + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { maxIterations: 11 } } }, null, 2)); + await fs.writeFile(`${source}.bak`, oldRaw); + const oldSourceHash = createHash("sha256").update(oldRaw).digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "pending", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + expect(result.markerStatus).toBeNull(); + expect(result.backupExists).toBe(false); + expect(result.targetValue).toBe(11); // the user's override is preserved + }); + + test("a deleted-and-readded marker-owned key is reconciled again", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + const oldRaw = '{"gjc.ralplan.maxIterations":7}'; + await fs.writeFile(target, YAML.stringify({ gjc: { ralplan: { maxIterations: 7 } } }, null, 2)); + await fs.writeFile(`${source}.bak`, oldRaw); + const oldSourceHash = createHash("sha256").update(oldRaw).digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "complete", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + }), + ); + + // Load 1: the user REMOVES the key -> the target is unset. + await fs.writeFile(source, "{}"); + expect((await runProbe(cwd, { home, configDir: ".myconfig" })).targetValue).toBeNull(); + // Load 2: the user RE-ADDS the key -> it is copied again. + await fs.writeFile(source, '{"gjc.ralplan.maxIterations":9}'); + expect((await runProbe(cwd, { home, configDir: ".myconfig" })).targetValue).toBe(9); + }); + + test("malformed-parent reconciliation clears the stale marker-owned target", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + const { source, agentDir } = await setupHome(home, ".myconfig"); + await fs.mkdir(agentDir, { recursive: true }); + const target = path.join(agentDir, "config.yml"); + const oldRaw = '{"gjc.ultragoal.nudgeBudget":7}'; + await fs.writeFile(target, YAML.stringify({ gjc: { ultragoal: { nudgeBudget: 7 } } }, null, 2)); + await fs.writeFile(`${source}.bak`, oldRaw); + // The user breaks the ultragoal parent after completion. + await fs.writeFile(source, '{"gjc":{"ultragoal":"broken"}}'); + const oldSourceHash = createHash("sha256").update(oldRaw).digest("hex"); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json.migrated"), + JSON.stringify({ + version: 1, + status: "complete", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: target, + sourceSha256: oldSourceHash, + migratedKeys: ["gjc.ultragoal.nudgeBudget"], + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + }), + ); + + const result = await runProbe(cwd, { home, configDir: ".myconfig" }); + const parsed = YAML.parse(await fs.readFile(target, "utf8")) as Record; + const ultragoal = (parsed.gjc as Record | undefined)?.ultragoal as + | Record + | undefined; + expect(ultragoal?.nudgeBudget).toBeUndefined(); // stale migration-write cleared + expect(result.sourceExists).toBe(true); + }); +}); diff --git a/packages/coding-agent/test/fixtures/settings-workflow-migration-probe.ts b/packages/coding-agent/test/fixtures/settings-workflow-migration-probe.ts new file mode 100644 index 0000000000..a586afa16a --- /dev/null +++ b/packages/coding-agent/test/fixtures/settings-workflow-migration-probe.ts @@ -0,0 +1,69 @@ +/** + * Child-process probe for the config-root workflow-settings migration. + * Runs `Settings.loadForScope` against the current working directory with an + * optional `--agent-dir` override, then reports the resulting file state so + * tests can assert pairing-gate, marker, backup, and migrated-value behavior + * without depending on host directory state. + * + * HOME / GJC_CONFIG_DIR / GJC_CODING_AGENT_DIR are read at module load, so this + * must run as a child process with the environment set before spawn. + */ +import * as fs from "node:fs/promises"; +import * as path from "node:path"; +import { getAgentDir, getConfigRootDir } from "@gajae-code/utils"; +import { YAML } from "bun"; +import { Settings } from "../../src/config/settings"; + +const cwd = process.cwd(); +const agentDirIndex = process.argv.indexOf("--agent-dir"); +const agentDirOverride = agentDirIndex >= 0 ? process.argv[agentDirIndex + 1] : undefined; + +await Settings.loadForScope({ cwd, ...(agentDirOverride ? { agentDir: agentDirOverride } : {}) }); + +const configRoot = getConfigRootDir(); +const source = path.resolve(configRoot, "settings.json"); +const backup = `${source}.bak`; +const markerPath = `${source}.migrated`; +const effectiveAgentDir = agentDirOverride ? path.resolve(agentDirOverride) : getAgentDir(); +const targetConfig = path.resolve(effectiveAgentDir, "config.yml"); + +const exists = async (target: string): Promise => { + try { + await fs.lstat(target); + return true; + } catch { + return false; + } +}; + +let markerStatus: string | null = null; +if (await exists(markerPath)) { + try { + const status = (JSON.parse(await fs.readFile(markerPath, "utf8")) as { status?: unknown }).status; + markerStatus = typeof status === "string" ? status : null; + } catch { + markerStatus = "invalid"; + } +} + +let targetValue: unknown = null; +if (await exists(targetConfig)) { + try { + const root = YAML.parse(await fs.readFile(targetConfig, "utf8")) as Record | null | undefined; + const gjc = root?.gjc as Record | undefined; + const ralplan = gjc?.ralplan as Record | undefined; + if (ralplan && Object.hasOwn(ralplan, "maxIterations")) targetValue = ralplan.maxIterations; + } catch { + // Malformed target YAML: report null; the load itself must have survived. + } +} + +console.log( + JSON.stringify({ + sourceExists: await exists(source), + backupExists: await exists(backup), + markerExists: await exists(markerPath), + markerStatus, + targetValue, + }), +); diff --git a/packages/coding-agent/test/fixtures/workflow-settings-probe.ts b/packages/coding-agent/test/fixtures/workflow-settings-probe.ts new file mode 100644 index 0000000000..3861de423b --- /dev/null +++ b/packages/coding-agent/test/fixtures/workflow-settings-probe.ts @@ -0,0 +1,14 @@ +/** + * Prints the resolved workflow setting for the current working directory. + * Directory/env resolution happens at module load, so this must be a child + * process when HOME/GJC_CONFIG_DIR/GJC_CODING_AGENT_DIR need isolation. + */ +import { resolveWorkflowSetting, type WorkflowSettingKey } from "../../src/gjc-runtime/workflow-settings"; + +const cwd = process.cwd(); +const key = process.argv[2] as WorkflowSettingKey; +const result = await resolveWorkflowSetting(cwd, key, { + defaultValue: "default", + parse: (value: unknown) => ({ kind: "valid" as const, value }), +}); +console.log(JSON.stringify({ value: result.value, source: result.source, diagnostics: result.diagnostics })); diff --git a/packages/coding-agent/test/gjc-runtime/config-root-home-relative.test.ts b/packages/coding-agent/test/gjc-runtime/config-root-home-relative.test.ts index 015ae67159..49eb51947d 100644 --- a/packages/coding-agent/test/gjc-runtime/config-root-home-relative.test.ts +++ b/packages/coding-agent/test/gjc-runtime/config-root-home-relative.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; +import { standardizeMacOSPath } from "@gajae-code/utils"; /** * `GJC_CONFIG_DIR` is documented as "Config root dirname under home", and @@ -48,7 +49,7 @@ describe("config root is resolved under home", () => { const result = (await resolveIn(home, repo, ".myconfig")).ralplan as { maxIterations: number; source: string }; expect(result.maxIterations).toBe(9); - expect(result.source).toBe(path.join(home, ".myconfig", "settings.json")); + expect(result.source).toBe(standardizeMacOSPath(path.join(home, ".myconfig", "settings.json"))); }); it("reads ultragoal settings from /", async () => { @@ -56,7 +57,7 @@ describe("config root is resolved under home", () => { const result = (await resolveIn(home, repo, ".myconfig")).ultragoal as { budget: number; source: string }; expect(result.budget).toBe(7); - expect(result.source).toBe(path.join(home, ".myconfig", "settings.json")); + expect(result.source).toBe(standardizeMacOSPath(path.join(home, ".myconfig", "settings.json"))); }); it("keeps using the default config dir name when unset", async () => { @@ -64,6 +65,6 @@ describe("config root is resolved under home", () => { const result = (await resolveIn(home, repo, undefined)).ralplan as { maxIterations: number; source: string }; expect(result.maxIterations).toBe(4); - expect(result.source).toBe(path.join(home, ".gjc", "settings.json")); + expect(result.source).toBe(standardizeMacOSPath(path.join(home, ".gjc", "settings.json"))); }); }); diff --git a/packages/coding-agent/test/gjc-runtime/deep-interview-runtime.test.ts b/packages/coding-agent/test/gjc-runtime/deep-interview-runtime.test.ts index a94ef4ce2f..f15e43ea92 100644 --- a/packages/coding-agent/test/gjc-runtime/deep-interview-runtime.test.ts +++ b/packages/coding-agent/test/gjc-runtime/deep-interview-runtime.test.ts @@ -602,7 +602,7 @@ describe("native gjc deep-interview runtime", () => { expect(payload.threshold_source).toBe(path.join(root, ".gjc", "settings.json")); }); - it("prefers modern config.yml threshold over legacy project settings.json", async () => { + it("prefers project settings over user config.yml (project beats user)", async () => { const root = await tempDir(); const agentDir = await tempDir(); setAgentDir(agentDir); @@ -619,8 +619,8 @@ describe("native gjc deep-interview runtime", () => { expect(result.status).toBe(0); const payload = JSON.parse(result.stdout ?? "{}"); - expect(payload.threshold).toBeCloseTo(0.2); - expect(payload.threshold_source).toBe(path.join(agentDir, "config.yml")); + expect(payload.threshold).toBeCloseTo(0.08); + expect(payload.threshold_source).toBe(path.join(root, ".gjc", "settings.json")); }); it("--threshold beats project settings.json", async () => { diff --git a/packages/coding-agent/test/gjc-runtime/ralplan-runtime.test.ts b/packages/coding-agent/test/gjc-runtime/ralplan-runtime.test.ts index 484fadf759..373b29660a 100644 --- a/packages/coding-agent/test/gjc-runtime/ralplan-runtime.test.ts +++ b/packages/coding-agent/test/gjc-runtime/ralplan-runtime.test.ts @@ -54,6 +54,16 @@ async function tempDir(): Promise { tempRoots.push(dir); return dir; } +async function seedProjectRalplanMaxIterations(root: string, maxIterations = 5): Promise { + // Project settings beat any user-layer configuration, making the cap + // scenarios hermetic regardless of the developer's ~/.gjc config. + await fs.mkdir(path.join(root, ".gjc"), { recursive: true }); + await fs.writeFile( + path.join(root, ".gjc", "settings.json"), + JSON.stringify({ gjc: { ralplan: { maxIterations } } }), + "utf-8", + ); +} afterEach(async () => { await Promise.all(tempRoots.splice(0).map(dir => fs.rm(dir, { recursive: true, force: true }))); @@ -2035,6 +2045,7 @@ describe("ralplan consensus iteration cap (#3165)", () => { it("rejects a 6th revision opener with PLANNING-STUCK and still allows final", async () => { const root = await tempDir(); + await seedProjectRalplanMaxIterations(root); const runId = "cap-run"; const write = async (stage: string, stageN: number, body: string) => runNativeRalplanCommand( @@ -2116,6 +2127,7 @@ describe("ralplan consensus iteration cap (#3165)", () => { }); it("fails closed when index.jsonl is emptied after max openers (ledger wipe)", async () => { const root = await tempDir(); + await seedProjectRalplanMaxIterations(root); const runId = "wipe-cap"; const write = async (stage: string, stageN: number, body: string) => runNativeRalplanCommand( @@ -2142,6 +2154,7 @@ describe("ralplan consensus iteration cap (#3165)", () => { it("fails closed when index.jsonl is truncated under on-disk openers", async () => { const root = await tempDir(); + await seedProjectRalplanMaxIterations(root); const runId = "trunc-cap"; const write = async (stage: string, stageN: number, body: string) => runNativeRalplanCommand( @@ -2166,6 +2179,7 @@ describe("ralplan consensus iteration cap (#3165)", () => { it("fails closed when index.jsonl is only malformed lines while openers exist on disk", async () => { const root = await tempDir(); + await seedProjectRalplanMaxIterations(root); const runId = "malformed-cap"; const write = async (stage: string, stageN: number, body: string) => runNativeRalplanCommand( @@ -2191,6 +2205,7 @@ describe("ralplan consensus iteration cap (#3165)", () => { it("fails closed when index is deleted but opener stage files remain", async () => { const root = await tempDir(); + await seedProjectRalplanMaxIterations(root); const runId = "delete-index-cap"; const write = async (stage: string, stageN: number, body: string) => runNativeRalplanCommand( @@ -2212,6 +2227,7 @@ describe("ralplan consensus iteration cap (#3165)", () => { it("clean new run_id still allows openers after another run is ledger-stuck", async () => { const root = await tempDir(); + await seedProjectRalplanMaxIterations(root); const write = async (runId: string, stage: string, stageN: number, body: string) => runNativeRalplanCommand( ["--write", "--stage", stage, "--stage_n", String(stageN), "--artifact", body, "--run-id", runId, "--json"], @@ -2775,6 +2791,7 @@ describe("ralplan review lane budget settings", () => { describe("ralplan review lane budget replays", () => { it("refuses only the pathological same-iteration lane retries and preserves final escalation", async () => { const root = await tempDir(); + await seedProjectRalplanMaxIterations(root); const runId = "pathological-replay"; const sequence = [ ["planner", "planner"], @@ -2852,6 +2869,7 @@ describe("ralplan review lane budget replays", () => { describe("ralplan review lane budget rigor and receipts", () => { it("does not parse or demote a justified critic blocker, while exhausted openers remain visibly stuck", async () => { const root = await tempDir(); + await seedProjectRalplanMaxIterations(root); const runId = "rigor-preserved"; expect((await writeRalplanArtifact(root, runId, "planner", 1, "# initial plan")).status).toBe(0); const critic = await writeRalplanArtifact( diff --git a/packages/coding-agent/test/gjc-runtime/workflow-settings.test.ts b/packages/coding-agent/test/gjc-runtime/workflow-settings.test.ts new file mode 100644 index 0000000000..0fe8cf0b88 --- /dev/null +++ b/packages/coding-agent/test/gjc-runtime/workflow-settings.test.ts @@ -0,0 +1,405 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { createHash } from "node:crypto"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { YAML } from "bun"; +import { + extractWorkflowSetting, + resolveWorkflowSetting, + WorkflowSettingError, + type WorkflowSettingKey, +} from "../../src/gjc-runtime/workflow-settings"; + +const KEY: WorkflowSettingKey = "gjc.ralplan.maxIterations"; +const PROBE = path.join(import.meta.dir, "../fixtures/workflow-settings-probe.ts"); + +const stringParse = (value: unknown) => + typeof value === "string" + ? { kind: "valid" as const, value } + : { kind: "invalid" as const, reason: "expected string" }; + +const temporaryDirectories: string[] = []; + +async function tempDir(): Promise { + const directory = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-workflow-settings-")); + temporaryDirectories.push(directory); + return directory; +} + +afterEach(async () => { + await Promise.all( + temporaryDirectories.splice(0).map(directory => fs.rm(directory, { recursive: true, force: true })), + ); +}); + +async function writeProjectSettings(cwd: string, document: unknown): Promise { + const projectDir = path.join(cwd, ".gjc"); + await fs.mkdir(projectDir, { recursive: true }); + const settingsPath = path.join(projectDir, "settings.json"); + await fs.writeFile(settingsPath, JSON.stringify(document, null, 2)); + return settingsPath; +} + +async function writeProjectConfig(cwd: string, document: unknown): Promise { + const projectDir = path.join(cwd, ".gjc"); + await fs.mkdir(projectDir, { recursive: true }); + const configPath = path.join(projectDir, "config.yml"); + await fs.writeFile(configPath, YAML.stringify(document, null, 2)); + return configPath; +} + +async function resolveIn( + cwd: string, + env: Record, + key: string = KEY, +): Promise<{ value: unknown; source: string; diagnostics: unknown[] }> { + const proc = Bun.spawn([process.execPath, PROBE, key], { + cwd, + env: { ...process.env, ...env }, + stdout: "pipe", + stderr: "pipe", + }); + const [out, err] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]); + if ((await proc.exited) !== 0) throw new Error(`probe failed: ${err}`); + return JSON.parse(out.trim()) as { value: unknown; source: string; diagnostics: unknown[] }; +} + +describe("workflow-settings resolver", () => { + test("project .gjc/settings.json wins over the built-in default", async () => { + const cwd = await tempDir(); + // A non-numeric string is preserved (no schema number coercion applies). + await writeProjectSettings(cwd, { "gjc.ralplan.maxIterations": "seven" }); + + const result = await resolveWorkflowSetting(cwd, KEY, { defaultValue: "default", parse: stringParse }); + expect(result.value).toBe("seven"); + expect(result.source).toBe(path.join(cwd, ".gjc", "settings.json")); + }); + + test("project .gjc/config.yml beats project .gjc/settings.json", async () => { + const cwd = await tempDir(); + await writeProjectConfig(cwd, { gjc: { ralplan: { maxIterations: "yaml-wins" } } }); + await writeProjectSettings(cwd, { "gjc.ralplan.maxIterations": "json-loses" }); + + const result = await resolveWorkflowSetting(cwd, KEY, { defaultValue: "default", parse: stringParse }); + expect(result.value).toBe("yaml-wins"); + expect(result.source).toBe(path.join(cwd, ".gjc", "config.yml")); + }); + + test("flat dotted and nested shapes are both extracted, flat wins", async () => { + expect(extractWorkflowSetting({ "gjc.ralplan.maxIterations": 7 }, KEY)).toEqual({ present: true, value: 7 }); + expect(extractWorkflowSetting({ gjc: { ralplan: { maxIterations: 8 } } }, KEY)).toEqual({ + present: true, + value: 8, + }); + expect( + extractWorkflowSetting({ "gjc.ralplan.maxIterations": 7, gjc: { ralplan: { maxIterations: 8 } } }, KEY), + ).toEqual({ + present: true, + value: 7, + }); + expect(extractWorkflowSetting({ ralplan: { maxIterations: 9 } }, KEY)).toEqual({ + present: false, + value: undefined, + }); + expect(extractWorkflowSetting({ gjc: { other: 1 } }, KEY)).toEqual({ present: false, value: undefined }); + expect(extractWorkflowSetting("not-an-object", KEY)).toEqual({ present: false, value: undefined }); + }); + + test("empty documents continue; a null root is a malformed shape (strict throws)", async () => { + const cwd = await tempDir(); + await fs.mkdir(path.join(cwd, ".gjc"), { recursive: true }); + await fs.writeFile(path.join(cwd, ".gjc", "config.yml"), "", "utf8"); // empty YAML -> no explicit settings + await fs.writeFile(path.join(cwd, ".gjc", "settings.json"), "null", "utf8"); // JSON null root -> malformed + + // tolerant: the empty document continues, the null root is an invalid shape + const result = await resolveWorkflowSetting(cwd, KEY, { defaultValue: "default", parse: stringParse }); + expect(result.value).toBe("default"); + expect(result.source).toBe("default"); + expect(result.diagnostics.map(d => d.status)).toContain("empty-document"); + expect(result.diagnostics.map(d => d.status)).toContain("invalid"); + // strict: the malformed explicit layer must fail closed (exit 2 contract) + await expect( + resolveWorkflowSetting(cwd, KEY, { defaultValue: "default", parse: stringParse, invalidPolicy: "throw" }), + ).rejects.toThrow(); + }); + + test("the literal JSON text undefined is malformed (strict throws, tolerant continues)", async () => { + const cwd = await tempDir(); + await fs.mkdir(path.join(cwd, ".gjc"), { recursive: true }); + await fs.writeFile(path.join(cwd, ".gjc", "settings.json"), "undefined", "utf8"); + + // Strict: fail closed (exit 2) on the malformed explicit JSON layer. + await expect( + resolveWorkflowSetting(cwd, KEY, { defaultValue: "default", parse: stringParse, invalidPolicy: "throw" }), + ).rejects.toThrow(); + // Tolerant: continue with an invalid diagnostic (not empty-document). + const result = await resolveWorkflowSetting(cwd, KEY, { defaultValue: "default", parse: stringParse }); + expect(result.value).toBe("default"); + expect(result.diagnostics.map(d => d.status)).not.toContain("empty-document"); + expect(result.diagnostics.map(d => d.status)).toContain("invalid"); + }); + + test("scalar and array roots are invalid shape, continue by default", async () => { + const cwd = await tempDir(); + await fs.mkdir(path.join(cwd, ".gjc"), { recursive: true }); + await fs.writeFile(path.join(cwd, ".gjc", "settings.json"), JSON.stringify(["a", "b"]), "utf8"); + + const result = await resolveWorkflowSetting(cwd, KEY, { defaultValue: "default", parse: stringParse }); + expect(result.value).toBe("default"); + expect(result.diagnostics.map(d => d.status)).toContain("invalid"); + }); + + test("malformed JSON is invalid syntax, continue by default, throw under strict", async () => { + const cwd = await tempDir(); + await fs.mkdir(path.join(cwd, ".gjc"), { recursive: true }); + await fs.writeFile(path.join(cwd, ".gjc", "settings.json"), "{ broken json", "utf8"); + + const continued = await resolveWorkflowSetting(cwd, KEY, { defaultValue: "default", parse: stringParse }); + expect(continued.value).toBe("default"); + expect(continued.diagnostics.find(d => d.layer === "project-settings")?.classification).toBe("syntax"); + + const thrown = await resolveWorkflowSetting(cwd, KEY, { + defaultValue: "default", + parse: stringParse, + invalidPolicy: "throw", + }).catch(error => error); + expect(thrown).toBeInstanceOf(WorkflowSettingError); + expect(thrown.path).toBe(path.join(cwd, ".gjc", "settings.json")); + expect(thrown.classification).toBe("syntax"); + expect(thrown.layer).toBe("project-settings"); + expect(thrown.message).toContain("invalid workflow setting at"); + }); + + test("an invalid present value is invalid/value, continue by default, throw under strict", async () => { + const cwd = await tempDir(); + await writeProjectSettings(cwd, { "gjc.ralplan.maxIterations": 7 }); + + const continued = await resolveWorkflowSetting(cwd, KEY, { defaultValue: "default", parse: stringParse }); + expect(continued.value).toBe("default"); + expect(continued.diagnostics.find(d => d.layer === "project-settings")?.classification).toBe("value"); + + const thrown = await resolveWorkflowSetting(cwd, KEY, { + defaultValue: "default", + parse: stringParse, + invalidPolicy: "throw", + }).catch(error => error); + expect(thrown).toBeInstanceOf(WorkflowSettingError); + expect(thrown.classification).toBe("value"); + expect(thrown.reason).toBe("expected string"); + }); + + test("agent config.yml (GJC_CODING_AGENT_DIR) beats the legacy config-root settings.json", async () => { + const home = await tempDir(); + const agentDir = await tempDir(); + const cwd = await tempDir(); + await fs.writeFile( + path.join(agentDir, "config.yml"), + YAML.stringify({ gjc: { ralplan: { maxIterations: "agent" } } }, null, 2), + ); + await fs.mkdir(path.join(home, ".gjc"), { recursive: true }); + await fs.writeFile( + path.join(home, ".gjc", "settings.json"), + JSON.stringify({ "gjc.ralplan.maxIterations": "root" }), + ); + + const result = await resolveIn(cwd, { + HOME: home, + GJC_CODING_AGENT_DIR: agentDir, + }); + expect(result.value).toBe("agent"); + expect(result.source).toBe(path.join(agentDir, "config.yml")); + }); + test("flat keys are honored only in legacy JSON settings, not config.yml", async () => { + const cwd = await tempDir(); + // config.yml carries only a flat dotted key: it must be IGNORED (the + // nested schema form is the config.yml format), so the nested JSON value + // in settings.json wins. + await writeProjectConfig(cwd, { "gjc.ralplan.maxIterations": "yaml-flat-ignored" }); + await writeProjectSettings(cwd, { gjc: { ralplan: { maxIterations: "json-nested" } } }); + + const result = await resolveWorkflowSetting(cwd, KEY, { defaultValue: "default", parse: stringParse }); + expect(result.value).toBe("json-nested"); + expect(result.source).toBe(path.join(cwd, ".gjc", "settings.json")); + }); + test("a quoted numeric config.yml value is coerced like the Settings schema", async () => { + const cwd = await tempDir(); + // Nested config.yml with a quoted number: reconcileSettingsSchema coerces + // numeric strings for number settings, and the resolver must match it. + await writeProjectConfig(cwd, { gjc: { ralplan: { maxIterations: "7" } } }); + const numberParse = (value: unknown) => + typeof value === "number" + ? { kind: "valid" as const, value } + : { kind: "invalid" as const, reason: "not a number" }; + + const result = await resolveWorkflowSetting(cwd, KEY, { defaultValue: 5, parse: numberParse }); + expect(result.value).toBe(7); + expect(result.source).toBe(path.join(cwd, ".gjc", "config.yml")); + }); + + test("the legacy config-root settings.json is the last fallback before default", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + await fs.mkdir(path.join(home, ".myconfig"), { recursive: true }); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json"), + JSON.stringify({ "gjc.ralplan.maxIterations": "root" }), + ); + + const result = await resolveIn(cwd, { HOME: home, GJC_CONFIG_DIR: ".myconfig" }); + expect(result.value).toBe("root"); + expect(result.source).toBe(path.join(home, ".myconfig", "settings.json")); + }); + test("a completed migration deactivates only the unchanged migrated source", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + await fs.mkdir(path.join(home, ".myconfig"), { recursive: true }); + const source = path.join(home, ".myconfig", "settings.json"); + const sourceRaw = JSON.stringify({ "gjc.ralplan.maxIterations": "root" }); + await fs.writeFile(source, sourceRaw); + // A completed one-time migration marker for this exact source, with the + // matching source hash (the migrated bytes). + await fs.writeFile( + `${source}.migrated`, + JSON.stringify({ + version: 1, + status: "complete", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: path.join(home, ".myconfig", "agent", "config.yml"), + sourceSha256: createHash("sha256").update(sourceRaw).digest("hex"), + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + }), + ); + + // While the source is still the migrated file, the layer is deactivated. + const result = await resolveIn(cwd, { HOME: home, GJC_CONFIG_DIR: ".myconfig" }); + expect(result.value).toBe("default"); + expect(result.source).toBe("default"); + + // A later edit of the legacy file REACTIVATES the documented fallback. + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": "edited" })); + const result2 = await resolveIn(cwd, { HOME: home, GJC_CONFIG_DIR: ".myconfig" }); + expect(result2.value).toBe("edited"); + expect(result2.source).toBe(source); + + // A CUSTOM agentDir profile never received the migrated value, so the + // legacy layer stays active for it (no deactivation by target mismatch). + const customAgent = await tempDir(); + const result3 = await resolveIn(cwd, { + HOME: home, + GJC_CONFIG_DIR: ".myconfig", + GJC_CODING_AGENT_DIR: customAgent, + }); + expect(result3.value).toBe("edited"); // the legacy fallback still works for the custom profile + expect(result3.source).toBe(source); + }); + test("a future-version migration marker does not deactivate the legacy source", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + await fs.mkdir(path.join(home, ".myconfig"), { recursive: true }); + const source = path.join(home, ".myconfig", "settings.json"); + const sourceRaw = JSON.stringify({ "gjc.ralplan.maxIterations": "root" }); + await fs.writeFile(source, sourceRaw); + // A marker from a NEWER GJC version: the resolver must not treat the + // migration as complete (the Settings migration would quarantine it). + await fs.writeFile( + `${source}.migrated`, + JSON.stringify({ + version: 999, + status: "complete", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: path.join(home, ".myconfig", "agent", "config.yml"), + sourceSha256: createHash("sha256").update(sourceRaw).digest("hex"), + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + }), + ); + + const result = await resolveIn(cwd, { HOME: home, GJC_CONFIG_DIR: ".myconfig" }); + expect(result.value).toBe("root"); // legacy fallback stays active + expect(result.source).toBe(source); + }); + + test("full five-layer precedence resolves the topmost project config.yml", async () => { + const home = await tempDir(); + const agentDir = await tempDir(); + const cwd = await tempDir(); + await fs.mkdir(path.join(home, ".myconfig"), { recursive: true }); + await writeProjectConfig(cwd, { gjc: { ralplan: { maxIterations: "project-yaml" } } }); + await writeProjectSettings(cwd, { "gjc.ralplan.maxIterations": "project-json" }); + await fs.writeFile( + path.join(agentDir, "config.yml"), + YAML.stringify({ gjc: { ralplan: { maxIterations: "agent" } } }, null, 2), + ); + await fs.writeFile( + path.join(home, ".myconfig", "settings.json"), + JSON.stringify({ "gjc.ralplan.maxIterations": "root" }), + ); + + const result = await resolveIn(cwd, { HOME: home, GJC_CONFIG_DIR: ".myconfig", GJC_CODING_AGENT_DIR: agentDir }); + expect(result.value).toBe("project-yaml"); + expect(result.source).toBe(path.join(cwd, ".gjc", "config.yml")); + }); + test("an empty settings.json is malformed JSON (strict throws, tolerant continues)", async () => { + const cwd = await tempDir(); + await fs.mkdir(path.join(cwd, ".gjc"), { recursive: true }); + await fs.writeFile(path.join(cwd, ".gjc", "settings.json"), " ", "utf8"); // whitespace-only + + await expect( + resolveWorkflowSetting(cwd, KEY, { defaultValue: "default", parse: stringParse, invalidPolicy: "throw" }), + ).rejects.toThrow(); + const result = await resolveWorkflowSetting(cwd, KEY, { defaultValue: "default", parse: stringParse }); + expect(result.value).toBe("default"); + expect(result.diagnostics.map(d => d.status)).not.toContain("empty-document"); + }); + + test("a malformed parent mapping is an invalid shape (strict throws)", async () => { + const cwd = await tempDir(); + await fs.mkdir(path.join(cwd, ".gjc"), { recursive: true }); + await fs.writeFile(path.join(cwd, ".gjc", "config.yml"), YAML.stringify({ gjc: { ralplan: [] } }, null, 2)); + + await expect( + resolveWorkflowSetting(cwd, KEY, { defaultValue: "default", parse: stringParse, invalidPolicy: "throw" }), + ).rejects.toThrow(); + }); + test("an edited legacy source outranks its stale migration-owned agent value", async () => { + const home = await tempDir(); + const cwd = await tempDir(); + await fs.mkdir(path.join(home, ".myconfig"), { recursive: true }); + await fs.mkdir(path.join(home, ".myconfig", "agent"), { recursive: true }); + const source = path.join(home, ".myconfig", "settings.json"); + const oldRaw = JSON.stringify({ "gjc.ralplan.maxIterations": 7 }); + // The agent config holds the stale MIGRATION-WRITTEN value. + await fs.writeFile( + path.join(home, ".myconfig", "agent", "config.yml"), + YAML.stringify({ gjc: { ralplan: { maxIterations: 7 } } }, null, 2), + ); + await fs.writeFile(`${source}.bak`, JSON.stringify({ "gjc.ralplan.maxIterations": 7 })); // migration copy + // The user EDITED the legacy source after completion. + await fs.writeFile(source, JSON.stringify({ "gjc.ralplan.maxIterations": 9 })); + await fs.writeFile( + `${source}.migrated`, + JSON.stringify({ + version: 1, + status: "complete", + sourcePath: source, + backupPath: `${source}.bak`, + targetPath: path.join(home, ".myconfig", "agent", "config.yml"), + sourceSha256: createHash("sha256").update(oldRaw).digest("hex"), + migratedKeys: ["gjc.ralplan.maxIterations"], + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + }), + ); + + // Direct workflow resolution (no Settings.init) must honor the edit. + const result = await resolveIn(cwd, { HOME: home, GJC_CONFIG_DIR: ".myconfig" }); + expect(result.value).toBe(9); + expect(result.source).toBe(source); + }); +}); diff --git a/packages/utils/src/dirs.ts b/packages/utils/src/dirs.ts index 5490dcd6d7..b857f8a58e 100644 --- a/packages/utils/src/dirs.ts +++ b/packages/utils/src/dirs.ts @@ -90,7 +90,7 @@ export function formatBunRuntimeError(opts: { * On macOS, strip /private prefix only when both paths resolve to the same location. * This preserves aliases like /private/tmp -> /tmp without rewriting unrelated paths. */ -function standardizeMacOSPath(p: string): string { +export function standardizeMacOSPath(p: string): string { if (process.platform !== "darwin" || !p.startsWith("/private/")) return p; const stripped = p.slice("/private".length); try { diff --git a/schemas/config.schema.json b/schemas/config.schema.json index 62065a7e25..2d85b97dd7 100644 --- a/schemas/config.schema.json +++ b/schemas/config.schema.json @@ -403,12 +403,27 @@ "default": "off" }, "maxIterations": { - "type": "number", - "default": 5 + "type": "integer", + "default": 5, + "minimum": 1, + "maximum": 20 }, "maxReviewPassesPerLane": { - "type": "number", - "default": 1 + "type": "integer", + "default": 1, + "minimum": 1, + "maximum": 10 + } + }, + "additionalProperties": false + }, + "ultragoal": { + "type": "object", + "properties": { + "nudgeBudget": { + "type": "integer", + "default": 10, + "minimum": 0 } }, "additionalProperties": false diff --git a/scripts/generate-json-schemas.test.ts b/scripts/generate-json-schemas.test.ts index 4362ff8194..0384c57046 100644 --- a/scripts/generate-json-schemas.test.ts +++ b/scripts/generate-json-schemas.test.ts @@ -47,7 +47,7 @@ describe("generated JSON Schemas", () => { const schema = configSchema() as any; const ralplan = schema.properties.gjc.properties.ralplan; - expect(ralplan.properties.maxReviewPassesPerLane).toMatchObject({ type: "number", default: 1 }); + expect(ralplan.properties.maxReviewPassesPerLane).toMatchObject({ type: "integer", default: 1, minimum: 1, maximum: 10 }); expect(ralplan.additionalProperties).toBe(false); }); diff --git a/scripts/generate-json-schemas.ts b/scripts/generate-json-schemas.ts index f67fd4b21a..9e92ee463f 100644 --- a/scripts/generate-json-schemas.ts +++ b/scripts/generate-json-schemas.ts @@ -110,6 +110,20 @@ function settingDefinitionToJsonSchema(settingPath: string, definition: SettingD schema.minimum = 60_000; schema.maximum = 86_400_000; } + if (settingPath === "gjc.ultragoal.nudgeBudget") { + schema.type = "integer"; + schema.minimum = 0; + } + if (settingPath === "gjc.ralplan.maxIterations") { + schema.type = "integer"; + schema.minimum = 1; + schema.maximum = 20; + } + if (settingPath === "gjc.ralplan.maxReviewPassesPerLane") { + schema.type = "integer"; + schema.minimum = 1; + schema.maximum = 10; + } return schema; }