fix(gate): name every problem a wake rule has, and read gate.json's as it ran

changelog.d/unreadable-gate-files.fixed.md

@@ -0,0 +1,5 @@
+- A wake-rule change no longer destroys a gate.json that won't load. `wake_add_rule` and `wake_remove_rule` (and `addGatePolicy` and `removeGatePolicy`) started from the rules in force when gate.json was there but wasn't JSON or held a policy the gate refused, and they wrote those rules straight over the file, so whatever it held, such as an operator's unfinished edit, was lost. A gate.json that holds a policy the gate refuses is now edited in place instead, with that policy kept as written. A change over one that isn't JSON, or that the gate can't read at all, such as one it lacks permission for, is refused, naming why and what the gate runs meanwhile, and nothing is written over it, so what it holds stays as it is, until it's JSON the gate can read.
+- The gate's files are reloaded when their text changes. At most once a second, gate.json is read again and applied when its text differs from the text whose rules are in force, and the privileged-users file is read again and loaded when its text differs from the text last read. A gate.json that holds the text in force again after failing to load, as after a `chmod` that makes it readable, has its error cleared, and nothing is applied again. gate.json's reload compared only its mtime, so two writes in one clock tick, a write between its read and its stat, or a read that only looked (`getGatePolicyNames`, or a change that wrote nothing) left a new gate.json unapplied until it next changed, and a `chmod` that made it readable again wasn't noticed. `isPrivilegedUser`, which tune-out's wake classification calls for each diverted message, read and parsed the privileged-users file on every call, and while it couldn't be read each call added another copy of its error to `gate_status`'s `errors`. A clock stepped backwards now makes the next check run, rather than holding off both reloads until the clock catches up. A missing file is told apart from one that's there but can't be reached, such as behind a directory the process can't search: only a missing gate.json keeps the rules in force without an error, and only a missing privileged-users file means no one is privileged.
+- A privileged-users file loads entry by entry. An entry of its list that's a non-empty string, or a safe integer (a whole number within 2^53 - 1 of zero), is a user id. Any other entry is skipped and named in the file's error, including a number too large for JSON to read exactly, whose rounded value could be another user's id. A file that isn't JSON, or isn't a list or `{"userIds": [...]}`, is that file's error, and the list that last loaded stays in effect, as for a file that can't be read (at startup, that's no one). Such a file used to load as no one privileged, without a word, and an entry that wasn't an id was loaded as text. An empty author id is never privileged, while the agent sleeps as for tune-out.
+- `gate_status` reads both files again, at most once a second, before it answers, and its `errors` holds each problem once: gate.json's, what the start did with the host's starting rules, and the privileged-users file's. A gate.json load no longer clears the start's notice, which stays until the next start, or the privileged-users file's error, and a privileged-users file that loads again, or is removed, clears its own. The starting rules the start couldn't save stay named until gate.json holds what's in force, as it does once it loads or a change is written. When gate.json's error changes, a `[gate]` log line says it couldn't be read and why, and the `gate:config-error` trace is emitted, not at every read that fails the same way. A gate.json that stopped loading used to reach only the trace.
+- `wake_remove_rule` and `removeGatePolicy` clear the removed rule's state (delivering its pending debounce batch, and dropping its rate-limit and sampling counters) only once gate.json is written. A write that failed used to leave the rule in force with its state already cleared.

src/framework.ts

@@ -682,7 +682,7 @@ const utilityOneLiner = (description: string): string => {
 import { CheckpointManager } from './mcpl/checkpoint-manager.js';
 import { isToolAllowed } from './mcpl/tool-policy.js';
 import { toolArgsProblems } from './tool-args.js';
-import { EventGate, formatShadowWarning, wakeLineText } from './gate/event-gate.js';
+import { EventGate, checkToolInput, formatShadowWarning, shownValue, wakeLineText } from './gate/event-gate.js';
 import type { GateEntryRef, WakeLine } from './gate/types.js';
 import { EPHEMERAL_USAGE_NAME, UsageTracker, persistedUsageProblem, type PersistedUsageState } from './usage/usage-tracker.js';
 import type { SessionUsageSnapshot, UsageUpdatedEvent } from './usage/types.js';
@@ -8101,15 +8101,28 @@ export class AgentFramework {
 
   /**
    * Add or replace (upsert) a gate policy at runtime. Validated and persisted
-   * to gate.json; hot-applied in memory. Throws if no gate is configured or the
-   * policy is invalid. Placement: `'prepend'` puts a rule ahead of EVERYTHING
-   * (including addressed-message wake rules — see EventGate.lintShadows);
-   * `{before: name}` / `{after: name}` anchor it against a specific rule,
-   * which is usually what a sampler or debounce actually wants.
+   * to gate.json; hot-applied in memory. Placement: `'prepend'` puts a rule
+   * ahead of EVERYTHING (including addressed-message wake rules — see
+   * EventGate.lintShadows); `{before: name}` / `{after: name}` anchor it
+   * against a specific rule, which is usually what a sampler or debounce
+   * actually wants. Every rule the change doesn't touch is written back as it
+   * was written. Over a gate.json the gate doesn't use, the change is written
+   * only if gate.json then loads, as when it replaces or removes the rule that
+   * keeps it out. Fields the gate reads nowhere, and that aren't a slip of one
+   * it does, are left out, and named to `onIgnoredFields`, called once the
+   * change is made; a throw from it is logged, not thrown. Throws if no gate is
+   * configured, the policy is invalid, the placement can't be made, gate.json
+   * has no rule list a change could edit, it isn't used and the change doesn't
+   * mend it, or it can't be read, isn't JSON or can't be written
+   * (EventGate.addPolicy). A refusal leaves gate.json as it is; a write that
+   * fails after its open may leave it part-written, and its error says so.
    */
   addGatePolicy(
     rawPolicy: unknown,
-    options?: { position?: 'append' | 'prepend' | { before: string } | { after: string } },
+    options?: {
+      position?: 'append' | 'prepend' | { before: string } | { after: string };
+      onIgnoredFields?: (notes: string[]) => void;
+    },
   ): import('./gate/types.js').GatePolicy {
     if (!this.eventGate) {
       throw new Error('No EventGate configured (FrameworkConfig.gate is unset).');
@@ -8119,7 +8132,10 @@ export class AgentFramework {
 
   /**
    * Remove a gate policy by name at runtime. Returns false if it didn't exist.
-   * Throws if no gate is configured.
+   * Throws if no gate is configured, gate.json has no rule list a change could
+   * edit, it isn't used and the removal doesn't mend it, or it can't be read,
+   * isn't JSON or can't be written (EventGate.removePolicy); the rules in
+   * force then stay as they were.
    */
   removeGatePolicy(name: string): boolean {
     if (!this.eventGate) {
@@ -9081,7 +9097,7 @@ export class AgentFramework {
         '• Watch a FILE/workspace path: match on `mount` + `pathGlob`, e.g. ' +
         '`{ name: "watch-notes", match: { scope: ["workspace:modified"], mount: "project", pathGlob: "notes/*.md" }, behavior: "always" }`.\n' +
         '• Watch a CHANNEL: match on `source` + `channel` (and/or `tagsAny: ["chat:ambient"]`), ' +
-        'e.g. `{ name: "watch-cairn", match: { source: "discord", channel: "discord:*:12345", tagsAny: ["chat:ambient"] }, behavior: { debounce: 60000 } }` ' +
+        'e.g. `{ name: "watch-cairn", match: { source: "discord", channel: "discord:*:12345", tagsAny: ["chat:ambient"] }, debounceMs: 60000 }` ' +
         '(the channel must be subscribed for ambient events to arrive — see channel mode).',
       inputSchema: {
         type: 'object',
@@ -9183,8 +9199,9 @@ export class AgentFramework {
       name: 'wake_remove_rule',
       description:
         'Remove a wake rule (gate.json policy) by name at runtime. Any pending ' +
-        'debounce batch for that rule is delivered first. Returns whether a rule ' +
-        'was removed (false if no rule had that name).',
+        'debounce batch for that rule is still delivered, held while your turn runs, as ' +
+        'its timer would deliver it. Returns whether a rule was removed (false if no ' +
+        'rule had that name).',
       inputSchema: {
         type: 'object',
         properties: {
@@ -28868,7 +28885,14 @@ export class AgentFramework {
    */
   private dispatchWakeRuleToolCall(agentName: string, call: ToolCall): void {
     this.emitTrace({ type: 'tool:started', module: 'gate', tool: call.name, callId: call.id, input: call.input });
-    const finish = (result: ToolResult) => {
+    // What wake_add_rule's input held that does nothing, checkToolInput's notes and
+    // then the gate's on a rule it saved: a refusal names it too, so the call that
+    // follows doesn't count on it.
+    const ignored: string[] = [];
+    const finish = (given: ToolResult) => {
+      // Where notes follow an error that doesn't end in a stop, it gets one, so each reads as a sentence of its own.
+      const said = given.error && !/[.!?]$/.test(given.error) ? `${given.error}.` : given.error;
+      const result = given.isError && ignored.length > 0 ? { ...given, error: [said, ...ignored].join(' ') } : given;
       this.emitTrace({
         type: result.isError ? 'tool:failed' : 'tool:completed',
         module: 'gate', tool: call.name, callId: call.id, durationMs: 0,
@@ -28894,8 +28918,15 @@ export class AgentFramework {
 
       // wake_add_rule — assemble the canonical behavior from the typed fields
       // (exactly one), then let the gate's own validator do the authoritative
-      // range/shape checks.
-      const input = (call.input ?? {}) as {
+      // range/shape checks. Its parameters are read as a rule's fields are: a
+      // null or "" one is left out, a slip of one is refused, and so is a field
+      // of `match`, `rateLimit` or `passiveSample` given on its own, which the
+      // rule would otherwise be saved without; any other name is named in the
+      // result's ignoredFields, and in a refusal's text.
+      const parameters = AgentFramework.WAKE_RULE_TOOLS[0]!.inputSchema.properties as Record<string, unknown>;
+      const input = checkToolInput(
+        'wake_add_rule', (call.input ?? {}) as Record<string, unknown>, parameters, ignored, { match: 'the rule matches every event' },
+      ) as {
         name?: unknown; match?: unknown; resets?: unknown; position?: unknown;
         insertBefore?: unknown; insertAfter?: unknown; passthrough?: unknown;
         behavior?: unknown; debounceMs?: unknown; rateLimit?: unknown; passiveSample?: unknown;
@@ -28922,6 +28953,23 @@ export class AgentFramework {
         });
         return;
       }
+      // `behavior` is "always", "defer" or "skip" in this tool's schema. An
+      // object works too, as gate.json writes it, but in XML tool mode one
+      // arrives as text. Anything else used to be saved as "always", so it's
+      // refused here, in this tool's terms: the gate's own refusal would point
+      // at the object form, which XML mode can't send.
+      const named = input.behavior;
+      if (named !== undefined && named !== 'always' && named !== 'defer' && named !== 'skip'
+        && (named === null || typeof named !== 'object' || Array.isArray(named))) {
+        finish({
+          success: false,
+          error: `wake_add_rule: behavior must be "always", "defer" or "skip", and it's ${shownValue(named)}, `
+            + 'so nothing was saved. For a debounce, a rate limit or sampling, give debounceMs, rateLimit or '
+            + 'passiveSample instead of behavior.',
+          isError: true,
+        });
+        return;
+      }
       const behavior: unknown =
         input.debounceMs !== undefined ? { debounce: input.debounceMs }
         : input.rateLimit !== undefined ? { rate_limit: input.rateLimit }
@@ -28944,6 +28992,30 @@ export class AgentFramework {
         });
         return;
       }
+      // A placement of the wrong type used to be ignored, so the rule was
+      // appended rather than put where it was asked for.
+      if (input.position !== undefined && input.position !== 'append' && input.position !== 'prepend') {
+        finish({
+          success: false,
+          error: `wake_add_rule: position must be "append" or "prepend", and it's ${shownValue(input.position)}, `
+            + 'so nothing was saved. To place the rule next to another, give insertBefore or insertAfter with '
+            + 'that rule\'s name.',
+          isError: true,
+        });
+        return;
+      }
+      for (const field of ['insertBefore', 'insertAfter'] as const) {
+        const anchor = input[field];
+        if (anchor !== undefined && (typeof anchor !== 'string' || anchor.length === 0)) {
+          finish({
+            success: false,
+            error: `wake_add_rule: ${field} must be the name of a rule, and it's ${shownValue(anchor)}, so nothing `
+              + `was saved. The rules are: [${this.getGatePolicyNames().join(', ')}].`,
+            isError: true,
+          });
+          return;
+        }
+      }
       let position: 'append' | 'prepend' | { before: string } | { after: string } | undefined;
       if (typeof input.insertBefore === 'string' && input.insertBefore) {
         position = { before: input.insertBefore };
@@ -28968,7 +29040,10 @@ export class AgentFramework {
       const before = new Map(
         this.eventGate!.probeTable().map((r) => [r.probe, r]),
       );
-      const policy = this.addGatePolicy(rawPolicy, position ? { position } : undefined);
+      const policy = this.addGatePolicy(rawPolicy, {
+        ...(position ? { position } : {}),
+        onIgnoredFields: (notes) => { ignored.push(...notes); },
+      });
       const after = this.eventGate!.probeTable();
       const probes = after.map((r) => {
         const prev = before.get(r.probe);
@@ -28987,6 +29062,7 @@ export class AgentFramework {
         success: true,
         data: {
           added: policy.name,
+          ...(ignored.length > 0 ? { ignoredFields: ignored } : {}),
           policies: this.getGatePolicyNames(),
           ...(shadowWarnings.length > 0 ? { shadowWarnings } : {}),
           probes,