feat(agent-skill): switch a stored off from before 1.50.0 on, once per data dir

Every App Settings Save up to 1.49.0 wrote `agentSkillEnabled: false`, chosen
or not (the switch loaded an absent key as off and Save sends the whole form),
so the new default never reached an install that had ever saved. On its first
start the server now replaces a stored false with true, once per data dir, and
logs one line when it does. It cannot tell a deliberate off from the old
default; that is accepted and stated in the release notes. An off set after it
ran stays off.

It stores true rather than deleting the key: the page's merge overwrites only
the synced keys the server has, so every device that saved on 1.49 kept its own
false, showed the switch off and wrote false back on its next Save (measured in
the JSDOM harness). True wins on every device, including a browser still on a
cached settings-ui.js. For a server with no key the page now drops the device's
copy as well, so the switch shows the default.

The "done" marker is its own file, `agent-skill-default-migrated.json` in the
data dir: it is not a setting, so GET /api/settings never returns it, no Save
can send it back (the strict schema would refuse it) or drop it (which would
re-arm the migration), and a config-backup restore leaves it alone. It is
checked before anything is read and written only after the settings write
landed. The migration runs in WebServer.start() before the first settings read
and before listen (skipped in test mode, like the other boot-time writes),
writes only through mutateSettingsFile(), drops the 2 s settings cache, and
leaves a settings.json that does not parse alone with no marker, so the next
start after the fix runs it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

src/web/agent-skill-default-migration.ts

@@ -0,0 +1,106 @@
+/**
+ * @fileoverview One-time boot migration that lets the 1.50.0 agent skill default reach installs
+ * that saved App Settings before it.
+ *
+ * `agentSkillEnabled` is ON by default since 1.50.0 (an absent key is on, only an explicit false
+ * is off). Up to 1.49.0 the App Settings switch loaded an absent key as off (`?? false`) and Save
+ * sends the whole form, so every install whose App Settings were ever saved holds an explicit
+ * `false` the user may never have chosen, and the new default would never reach it. On the first
+ * start of a server that has this module, a stored `false` is switched on, exactly once per data
+ * dir. It cannot tell a deliberate off from the old default; that is accepted and stated as a
+ * behaviour change in the release notes. An off set after this ran is never touched again.
+ *
+ * Why a stored `false` becomes `true` rather than being deleted: deleting it would not reach the
+ * page. Every browser that saved App Settings keeps the form in its localStorage blob, and
+ * `loadAppSettingsFromServer()` only overwrites a synced key the server HAS: a key the server
+ * lacks keeps the device's `false`, the switch loads off, and the next Save of any setting writes
+ * `false` back (test/agent-skill-setting-ui.test.ts proves both shapes). A stored `true` wins on
+ * every device, including a browser still running a cached settings-ui.js from before the upgrade.
+ * Every reader treats `true` and an absent key alike (`!== false`).
+ *
+ * Why the "done" marker is its own file (`agent-skill-default-migrated.json` in the data dir)
+ * and not a key in settings.json:
+ *  - it is not a setting, so `GET /api/settings` never returns it, the page never holds it, and
+ *    the page's Save (rebuilt from the form) can neither send it back nor drop it; the `.strict()`
+ *    `SettingsUpdateSchema` never sees it;
+ *  - a PUT or any other settings.json writer cannot erase it, which would re-arm the migration and
+ *    switch a deliberate later off back on;
+ *  - it is not in `CONFIG_BACKUP_FILES` (like `config-backup-id`), so a restore leaves it alone: a
+ *    restored settings.json, even one from before 1.50.0 holding `false`, is what the user asked
+ *    for and stays as restored.
+ * A data dir is one instance, and settings.json is one file for the whole server, so in
+ * multi-user mode this runs once for the server, never per user.
+ *
+ * Order: the marker is checked first (present: nothing is read), the settings change goes through
+ * `mutateSettingsFile()` (serialized, atomic, refuses a file that does not parse), and the marker
+ * is written only after that resolves. A refused or failed settings write writes no marker, so the
+ * migration runs again at the next start (after the user fixed the file). A crash between the two
+ * writes is harmless: the next start finds `true` (nothing to change) and writes the marker.
+ *
+ * Called once from `WebServer.start()`, before the first settings read and before the server
+ * listens, so no request path can see the old value.
+ */
+
+import fs from 'node:fs/promises';
+import { dirname } from 'node:path';
+import { dataPath } from '../config/instance.js';
+import { getErrorMessage } from '../types.js';
+import { mutateSettingsFile, SettingsWriteRefusedError } from './settings-writer.js';
+
+/** The marker's file name in the data dir. Never in CONFIG_BACKUP_FILES (see the file comment). */
+export const AGENT_SKILL_DEFAULT_MARKER = 'agent-skill-default-migrated.json';
+
+/**
+ * - `switched-on`: settings.json held `agentSkillEnabled: false`; it now holds `true`.
+ * - `nothing-to-change`: the key was absent or not `false` (or there is no settings.json); nothing was written to it.
+ * - `already-done`: the marker exists; settings.json was not read.
+ * - `refused`: settings.json could not be read, parsed or written, or the marker could not be checked.
+ *   Nothing changed and no marker was written, so the next start tries again.
+ */
+export type AgentSkillDefaultMigration = 'switched-on' | 'nothing-to-change' | 'already-done' | 'refused';
+
+/** The log line of a migration that switched the skill on. */
+export const AGENT_SKILL_DEFAULT_MIGRATED_LOG =
+  '[agent-skill] cleared the stored off from before 1.50.0 so the new default applies (turn it off again under App Settings, Agents & CLIs, Codeman Agent Skill; that choice is kept)';
+
+export async function migrateAgentSkillDefault(
+  markerPath: string = dataPath(AGENT_SKILL_DEFAULT_MARKER)
+): Promise<AgentSkillDefaultMigration> {
+  try {
+    await fs.lstat(markerPath);
+    return 'already-done';
+  } catch (err) {
+    if ((err as NodeJS.ErrnoException).code !== 'ENOENT') {
+      // Not knowing whether it ran must not switch a deliberate off on a second time.
+      console.warn(`[agent-skill] default migration skipped: cannot check ${markerPath} (${getErrorMessage(err)})`);
+      return 'refused';
+    }
+  }
+
+  let switched = false;
+  try {
+    await mutateSettingsFile((current) => {
+      if (current.agentSkillEnabled !== false) return undefined;
+      switched = true;
+      return { ...current, agentSkillEnabled: true };
+    });
+  } catch (err) {
+    // A refusal is already logged by the writer (`[settings] write refused: ...`), with the fix.
+    const why = err instanceof SettingsWriteRefusedError ? 'settings.json was left as it is' : getErrorMessage(err);
+    console.warn(`[agent-skill] default migration skipped (${why}); it runs again at the next start`);
+    return 'refused';
+  }
+
+  try {
+    await fs.mkdir(dirname(markerPath), { recursive: true });
+    const marker = { migratedAt: new Date().toISOString(), switchedOn: switched };
+    await fs.writeFile(markerPath, `${JSON.stringify(marker, null, 2)}\n`, { mode: 0o600 });
+  } catch (err) {
+    // settings.json is already right; without the marker the next start runs again, which
+    // changes nothing unless the user has switched it off by then.
+    console.warn(`[agent-skill] could not record the default migration at ${markerPath}: ${getErrorMessage(err)}`);
+  }
+
+  if (switched) console.log(AGENT_SKILL_DEFAULT_MIGRATED_LOG);
+  return switched ? 'switched-on' : 'nothing-to-change';
+}

src/web/public/settings-ui.js

@@ -5268,6 +5268,13 @@ Object.assign(CodemanApp.prototype, {
             merged[key] = value;
           }
         }
+        // The agent skill is a synced key that is ON when absent (since 1.50.0). A server
+        // without the key means that default, so this device's own copy goes rather than
+        // standing in for it: up to 1.49.0 the switch loaded an absent key as off, so a
+        // device whose save never reached the server still holds `false`, and its next
+        // Save would write it back. The server's one-time boot migration stores `true` for
+        // installs that had saved `false` (agent-skill-default-migration.ts).
+        if (!('agentSkillEnabled' in appSettings)) delete merged.agentSkillEnabled;
         this.saveAppSettingsToStorage(merged);
 
         // Apply notification prefs from server if present (only if localStorage has none)

src/web/server.ts

@@ -195,6 +195,7 @@ import { resolveTerminalHistoryConfig } from '../config/terminal-history.js';
 import { SseEvent } from './sse-events.js';
 import { MAX_ROUTE_PARAM_LENGTH } from './route-helpers.js';
 import { mutateSettingsFile } from './settings-writer.js';
+import { migrateAgentSkillDefault } from './agent-skill-default-migration.js';
 import { getLatestPlanUsage, setLatestCodexPlanUsage, setLatestCopilotPlanUsage } from './plan-usage-latest.js';
 import { telemetrySignature } from '../usage-telemetry.js';
 import { readCodexPlanUsage, resolveCodexBinaryPath } from '../utils/codex-cli-resolver.js';
@@ -3401,6 +3402,14 @@ export class WebServer extends EventEmitter {
       reconcileUpdateOnBoot();
     }
 
+    // One-time, once per data dir: a stored `agentSkillEnabled: false` written by the switch's
+    // old default (up to 1.49.0) becomes true, so the 1.50.0 default reaches installs that ever
+    // saved App Settings (agent-skill-default-migration.ts). Before the restore below reads
+    // settings (2 s cache) and before listen, so no session create can see the old value.
+    if (!this.testMode && (await migrateAgentSkillDefault()) === 'switched-on') {
+      this._settingsCache = null;
+    }
+
     // Restore mux sessions BEFORE accepting connections
     // This prevents race conditions where clients connect before state is ready
     // CRITICAL: Skip in test mode to prevent tests from picking up user sessions