浏览代码

fix(v2): retry child permission rules after a failed host call

Review finding on #1194: the applied marker was set before the
ctx.permission.rules() call resolved, so a rejected call permanently
marked the child as handled — a replayed or duplicate session.created
could not retry, stranding the child on inherited session rules instead
of its task policy.

Latch the marker only after the host call succeeds; empty derivations
still latch immediately (a final answer that cannot change between
duplicate events). Concurrent duplicates at worst re-send the same
replace payload, which is idempotent host-side. Regression test
(e-retry) covers retry-after-failure and latch-after-success, written
red-first against the old ordering.
GoldJohnKing 2 天之前
父节点
当前提交
1558e3f5ca
共有 2 个文件被更改,包括 46 次插入6 次删除
  1. 35 2
      src/v2/permission-rules-bridge.test.ts
  2. 11 4
      src/v2/setup.ts

+ 35 - 2
src/v2/permission-rules-bridge.test.ts

@@ -250,13 +250,46 @@ describe('createPermissionRulesBridge', () => {
     await expect(
       bridge.observeSessionCreated(makeChildCreatedEvent({})),
     ).resolves.toBeUndefined();
-    // Strictly-once per sessionID: a duplicate event does not retry the
-    // failed call.
+    // Every retry attempt stays fail-soft; the retry itself is covered
+    // by (e-retry).
     await expect(
       bridge.observeSessionCreated(makeChildCreatedEvent({})),
     ).resolves.toBeUndefined();
   });
 
+  test('(e-retry) a failed application is retried by a duplicate session.created', async () => {
+    // Regression (review on #1194): the applied marker used to be set
+    // before the host call, so a rejected rules() permanently stranded
+    // the child on inherited session rules. Completion must latch only
+    // on success; failure releases the slot for the next event.
+    let attempts = 0;
+    const calls: RulesCall[] = [];
+    const bridge = makeBridge({
+      permission: {
+        rules: async (input) => {
+          attempts += 1;
+          if (attempts === 1) throw new Error('transient host failure');
+          calls.push(input);
+          return {};
+        },
+      },
+    });
+
+    await expect(
+      bridge.observeSessionCreated(makeChildCreatedEvent({})),
+    ).resolves.toBeUndefined();
+    expect(attempts).toBe(1);
+
+    // A duplicate event retries the failed application and succeeds.
+    await bridge.observeSessionCreated(makeChildCreatedEvent({}));
+    expect(attempts).toBe(2);
+    expect(calls).toHaveLength(1);
+
+    // Success latches: further duplicates do not re-apply.
+    await bridge.observeSessionCreated(makeChildCreatedEvent({}));
+    expect(attempts).toBe(2);
+  });
+
   test('malformed events resolve without throwing (fail-soft)', async () => {
     const bridge = makeBridge({
       permission: {

+ 11 - 4
src/v2/setup.ts

@@ -789,15 +789,15 @@ export function createPermissionRulesBridge(
       }
       return;
     }
-    applied.set(sessionID, true);
-    pruneSessionMap(applied);
     const rules = deriveExactPermissionRules(options.permissionForAgent(agent));
     if (rules.length === 0) {
       // Nothing in the task-policy is expressible as an exact match
       // (e.g. a whole-tool read-only policy): an empty replace would add
       // nothing over the static agent permissions, so skip the host
-      // call. Still marked handled — the policy cannot change between
-      // duplicate events.
+      // call. Marked handled here — an empty derivation is a final
+      // answer that cannot change between duplicate events.
+      applied.set(sessionID, true);
+      pruneSessionMap(applied);
       log(
         '[v2][permission-rules] no exact-match rules derivable for child session',
         { sessionID, agent },
@@ -805,6 +805,13 @@ export function createPermissionRulesBridge(
       return;
     }
     await rulesFn({ sessionID, permissions: rules });
+    // Latch only after the host call resolves: a rejected call leaves
+    // the slot free, so a replayed or duplicate session.created retries
+    // instead of stranding the child on inherited session rules
+    // (review on #1194). Concurrent duplicates at worst re-send the
+    // same replace payload — idempotent on the host side.
+    applied.set(sessionID, true);
+    pruneSessionMap(applied);
     log('[v2][permission-rules] applied exact-match rules to child session', {
       sessionID,
       agent,