diff --git a/cli/__tests__/enforcement.test.mjs b/cli/__tests__/enforcement.test.mjs index dc0c89ff2..d4114eab6 100644 --- a/cli/__tests__/enforcement.test.mjs +++ b/cli/__tests__/enforcement.test.mjs @@ -15,10 +15,10 @@ import { jest } from '@jest/globals'; import { ADDRESSED_EVENT_TYPES, + MENTION_EVENT_TYPES, CASCADE_DEFAULTS, CASCADE_ENV_VARS, CLAIMABLE_EVENT_TYPES, - MENTION_EVENT_TYPES, classifyTrigger, createCascadeGovernor, createClaimHandicap, @@ -37,24 +37,6 @@ describe('classifyTrigger', () => { expect(classifyTrigger(event({ dmKind: 'user-agent', messageId: 'm1' }), [])).toBe('human'); }); - // ADR-024 D1 board wakes carry `dmKind` and NO `messageId` — deliberately, so - // every opted-in seat looks and the per-task claim CAS arbitrates - // (taskEventService.ts:237). That backend comment is only safe because the - // dmKind branches sit ABOVE the messageId check here. Move the messageId - // check up, or add an early `if (!p.messageId) return 'unknown'`, and every - // board wake classifies 'unknown' — which by design neither counts toward the - // cascade cap nor resets it, so the cap silently stops engaging and the - // 156-wake sweep this governor exists to bound comes back. - // - // The suite already asserted dmKind pricing and messageId fallback, but every - // dmKind case supplied a messageId and every messageId case omitted dmKind — - // so the one combination the backend actually emits was never exercised, and - // both reorderings passed 107/107. - test('prices by dmKind with NO messageId — the board-wake shape', () => { - expect(classifyTrigger(event({ dmKind: 'agent-agent' }), [])).toBe('agent'); - expect(classifyTrigger(event({ dmKind: 'user-agent' }), [])).toBe('human'); - }); - test('falls back to the trigger message isBot flag from the snapshot', () => { const messages = [{ _id: 'm1', isBot: true }, { _id: 'm2', isBot: false }]; expect(classifyTrigger(event({ messageId: 'm1' }), messages)).toBe('agent'); @@ -178,65 +160,94 @@ describe('cascade governor — addressed grace', () => { expect(gov.admit('pod', 'agent', 'chat.mention').allowed).toBe(true); }); + it('never refuses a mention, however long the streak', () => { + // The grace (cap + addressedGrace) was not enough and was the wrong + // instrument. @ux-lead took two chat.mention refusals mid-thread while + // peers were naming it, and could not afterwards say which it had missed. + // + // Safe to exempt because mentions already have a PRODUCER-side bound: + // agentMentionService.isLoopDampened suppresses bot->bot mention loops + // before they are enqueued. This cap was a second brake on the one class + // that did not need one. + const gov = createCascadeGovernor({ cap: 3, addressedGrace: 2 }); + burn(gov, 20, 'message.posted'); + + const verdict = gov.admit('pod', 'agent', 'chat.mention'); + expect(verdict.allowed).toBe(true); + expect(verdict.mentionExempt).toBe(true); + // Streak is untouched — the mention is admitted, not forgiven. + expect(verdict.streak).toBe(20); + }); + + it('still caps dm.message, which has no producer-side dampener', () => { + // dm.message is ADDRESSED but is NOT in MENTION_EVENT_TYPES and has no + // loop dampener at the producer, so this cap is the ONLY bound on + // agent<->agent DM ping-pong. Exempting the whole ADDRESSED set would have + // removed the exact cascade this governor exists for, in the one place + // with no second guard. + const gov = createCascadeGovernor({ cap: 3, addressedGrace: 2 }); + burn(gov, 20, 'message.posted'); + + expect(gov.admit('pod', 'agent', 'dm.message').allowed).toBe(false); + }); + + it('derives the mention set from ADDRESSED rather than restating it', () => { + // Two lists stating one rule drift the next time a mention type is added, + // and the drift is silent. thread.mention is currently declared and never + // emitted; when threading starts emitting it, it inherits the exemption + // automatically instead of needing a second edit nobody remembers. + expect([...MENTION_EVENT_TYPES].sort()).toEqual(['chat.mention', 'thread.mention']); + for (const type of MENTION_EVENT_TYPES) { + expect(ADDRESSED_EVENT_TYPES.has(type)).toBe(true); + } + expect(MENTION_EVENT_TYPES.has('dm.message')).toBe(false); + }); + it('marks the admission so the caller can say the grace was spent', () => { const gov = createCascadeGovernor({ cap: 1, addressedGrace: 1 }); expect(gov.admit('pod', 'agent', 'chat.mention').addressed).toBe(true); expect(gov.admit('pod', 'agent', 'message.posted').addressed).toBe(false); }); - it('does not report a grace it never granted to a legacy direct-address event, at addressedGrace 0', () => { + it('does not report a grace it never granted, at addressedGrace 0', () => { // The refusal log is the one line an operator reads to understand why a // seat went quiet. Keyed on `addressed` alone it announced "(addressed // grace also spent)" on a grace=0 seat — asserting a grace that does not // exist, three screens under a boot line that says grace=0. The event is - // still addressed; nothing was granted for it. Agent-DM wakes use - // chat.mention, but the legacy dm.message vocabulary remains supported. + // still addressed; nothing was granted for it. const gov = createCascadeGovernor({ cap: 1, addressedGrace: 0 }); - const admission = gov.admit('pod', 'agent', 'dm.message'); + const admission = gov.admit('pod', 'agent', 'chat.mention'); expect(admission.addressed).toBe(true); expect(admission.graceApplied).toBe(false); // And the limit really is the plain cap — the message was the only defect. + // dm.message rather than chat.mention: addressed, but not mention-exempt. gov.record('pod', 'agent'); expect(gov.admit('pod', 'agent', 'dm.message').allowed).toBe(false); }); - it('exempts only kernel-dampened non-DM mentions; DM-backed mentions stay in the streak', () => { - const gov = createCascadeGovernor({ cap: 1, addressedGrace: 0 }); - gov.record('pod', 'agent'); - - // The kernel's bot-to-bot dampener owns these two types. A named seat must - // stay reachable after broadcasts fill this local budget. - expect(gov.admit('pod', 'agent', 'chat.mention').allowed).toBe(true); - expect(gov.admit('pod', 'agent', 'thread.mention').allowed).toBe(true); - - // Agent DMs also use chat.mention, but dmKind identifies the other - // producer. They must remain locally bounded; otherwise a type-only - // exemption opens a second unbounded bot-to-bot loop. - expect(gov.admit('pod', 'agent', 'chat.mention', { dmKind: 'agent-agent' }).allowed).toBe(false); + it('a DM echo still terminates — the grace is not an exemption for dm.message', () => { + // This replaces "is a grace, not an exemption — a mention echo still + // terminates", whose premise this change deliberately reverses. That test + // guarded a real loop (A-mentions-B-mentions-A), but bounded it at the + // WRONG layer: mentions already have a producer-side dampener + // (agentMentionService.isLoopDampened, >3 to the same bot/pod in a 5-minute + // window, bot->bot only). Two brakes on one class, and the redundant one + // was the only one that ever fired — @ux-lead lost two chat.mention events + // mid-thread. + // + // dm.message has NO producer-side dampener, so the loop this originally + // guarded is now guarded here, on the type that actually needs it. + const gov = createCascadeGovernor({ cap: 3, addressedGrace: 2 }); + burn(gov, 5, 'dm.message'); - // dm.message is legacy direct-address vocabulary, not in the kernel - // mention budget, so it remains bounded locally. Broadcasts do too. expect(gov.admit('pod', 'agent', 'dm.message').allowed).toBe(false); - expect(gov.admit('pod', 'agent', 'message.posted').allowed).toBe(false); - }); - - it('records a completed mention, so it spends broadcast liveness', () => { - const gov = createCascadeGovernor({ cap: 1, addressedGrace: 0 }); - gov.record('pod', 'agent'); - - expect(gov.admit('pod', 'agent', 'chat.mention').allowed).toBe(true); - // Mirrors agent.js: exemption only changes admit(); a completed turn still - // reaches record(). The following broadcast remains capped at the new - // streak rather than acquiring an extra unmetered pass. - gov.record('pod', 'agent'); - expect(gov.admit('pod', 'agent', 'message.posted')).toMatchObject({ - allowed: false, - streak: 2, - }); }); it('a human turn still clears the streak for addressed and broadcast alike', () => { const gov = createCascadeGovernor({ cap: 1, addressedGrace: 1 }); + // Vehicle is dm.message, not chat.mention: mentions are now exempt, so a + // mention could never demonstrate a streak being cleared. The property + // under test is the human reset, which is unchanged. burn(gov, 2, 'dm.message'); expect(gov.admit('pod', 'agent', 'dm.message').allowed).toBe(false); @@ -562,13 +573,6 @@ describe('peerHoldsFrame / ADDRESSED_EVENT_TYPES', () => { }); }); -describe('MENTION_EVENT_TYPES', () => { - test('contains only the two producer-dampened mention types, not DMs', () => { - expect([...MENTION_EVENT_TYPES]).toEqual(['chat.mention', 'thread.mention']); - expect(MENTION_EVENT_TYPES.has('dm.message')).toBe(false); - }); -}); - describe('CLAIMABLE_EVENT_TYPES', () => { test('covers message-bearing wakes and excludes one-shot / private events', () => { expect(CLAIMABLE_EVENT_TYPES.has('chat.mention')).toBe(true); @@ -612,8 +616,8 @@ describe('resolveCascadeSettings', () => { }); test('zero is honoured, not treated as absent', () => { - // Legacy direct-address events remain on the configurable grace path. A - // `|| default` style resolver would silently ignore 0 and admit one more. + // The whole point of the grace knob: 0 restores pre-#973 behaviour without + // a revert. A `|| default` style resolver would silently ignore it. const settings = resolveCascadeSettings({ env: { [CASCADE_ENV_VARS.addressedGrace]: '0' }, warn: silent, }); @@ -622,6 +626,9 @@ describe('resolveCascadeSettings', () => { governor.record('pod', 'agent'); governor.record('pod', 'agent'); governor.record('pod', 'agent'); + // dm.message, not chat.mention: this asserts that grace 0 is HONOURED + // rather than treated as absent, and mentions are now exempt from the cap + // altogether — so a mention could not demonstrate a grace of any size. expect(governor.admit('pod', 'agent', 'dm.message').allowed).toBe(false); }); diff --git a/cli/__tests__/run-loop.test.mjs b/cli/__tests__/run-loop.test.mjs index 8ccce1453..a0688b1db 100644 --- a/cli/__tests__/run-loop.test.mjs +++ b/cli/__tests__/run-loop.test.mjs @@ -1456,6 +1456,24 @@ describe('performRun — ADR-018 enforcement', () => { ...overrides, }); + // Same shape, but a CAPPED event type. chat.mention is exempt from the + // cascade cap (it has a producer-side dampener — agentMentionService's + // isLoopDampened, >3 to the same bot/pod per 5-minute window), so a mention + // vehicle would make every cap assertion below pass while testing nothing. + // message.posted is the ambient class the cap actually governs. + const makeCappedEvent = (overrides = {}) => makeClaimEvent({ + type: 'message.posted', + ...overrides, + }); + + // ADDRESSED and still capped. dm.message is the only member of + // ADDRESSED_EVENT_TYPES that the cap still governs — mentions are exempt — + // so it is the only vehicle left that can exercise the grace path. + const makeAddressedCappedEvent = (overrides = {}) => makeClaimEvent({ + type: 'dm.message', + ...overrides, + }); + // Route-aware client mock: events/messages/memory GETs, claim-aware POSTs. const makeClient = ({ events, @@ -1598,20 +1616,23 @@ describe('performRun — ADR-018 enforcement', () => { expect(post.mock.calls.some(([r]) => r.endsWith('/claim'))).toBe(false); }); - test('cascade cap: agent-DM chat.mentions beyond the cap are declined without a spawn or a claim', async () => { + test('cascade cap: agent-triggered turns beyond the cap are declined without a spawn or a claim', async () => { const { post } = makeClient({ events: [ - makeClaimEvent({ _id: 'evt-a', payload: { content: 'hello', messageId: 'msg-1', dmKind: 'agent-agent' } }), - makeClaimEvent({ _id: 'evt-b', payload: { content: 'again', messageId: 'msg-2', dmKind: 'agent-agent' } }), + makeCappedEvent({ _id: 'evt-a' }), + makeCappedEvent({ _id: 'evt-b', payload: { content: 'again', messageId: 'msg-2' } }), ], - // Both are agent-DM events, which use chat.mention but carry dmKind. + // Both trigger messages are BOT-authored — this is a mention cascade. messages: [ { _id: 'msg-1', isBot: true, self: false }, { _id: 'msg-2', isBot: true, self: false }, ], }); const spawn = jest.fn(async () => ({ text: 'NO_REPLY' })); - // DM-backed chat.mentions stay locally bounded when grace is disabled. + // grace 0 pins the PURE cap contract. The events below are message.posted, + // NOT the makeEvent chat.mention default: mentions are exempt from this cap + // entirely (they have a producer-side dampener), so a mention vehicle would + // make this test assert nothing while still passing. const { stop } = run( { name: 'stub', detect: stubAdapter.detect, spawn }, { cascadeCap: 1, cascadeAddressedGrace: 0 }, @@ -1633,7 +1654,7 @@ describe('performRun — ADR-018 enforcement', () => { cap: 1, addressedGrace: 0, resetMs: 600000, - addressed: true, + addressed: false, graceApplied: false, }, }, @@ -1657,8 +1678,8 @@ describe('performRun — ADR-018 enforcement', () => { try { const { post } = makeClient({ events: [ - makeClaimEvent({ _id: 'evt-a', type: 'dm.message' }), - makeClaimEvent({ _id: 'evt-b', type: 'dm.message', payload: { content: 'again', messageId: 'msg-2' } }), + makeCappedEvent({ _id: 'evt-a' }), + makeCappedEvent({ _id: 'evt-b', payload: { content: 'again', messageId: 'msg-2' } }), ], messages: [ { _id: 'msg-1', isBot: true, self: false }, @@ -1683,7 +1704,7 @@ describe('performRun — ADR-018 enforcement', () => { cap: 1, addressedGrace: 0, resetMs: 600000, - addressed: true, + addressed: false, graceApplied: false, }, }, @@ -1697,14 +1718,18 @@ describe('performRun — ADR-018 enforcement', () => { } }); - test('cascade cap: a legacy direct-address event gets a bounded grace, then is capped too', async () => { - // Mention types are producer-dampened and exempted separately. Legacy - // direct-address vocabulary remains on the local bounded-grace path. + test('cascade cap: a directly-addressed seat gets a bounded grace, then is capped too', async () => { + // The failure this fixes, measured 2026-08-18: a seat took 28 consecutive + // cap refusals, five of them chat.mention. Peers named it and got silence. + // + // The grace is bounded on purpose. An unbounded pass for addressed events + // restores the A-mentions-B-mentions-A echo the governor exists to kill, so + // the third event below must still be declined. const { post } = makeClient({ events: [ - makeClaimEvent({ _id: 'evt-a', type: 'dm.message' }), - makeClaimEvent({ _id: 'evt-b', type: 'dm.message', payload: { content: 'again', messageId: 'msg-2' } }), - makeClaimEvent({ _id: 'evt-c', type: 'dm.message', payload: { content: 'and again', messageId: 'msg-3' } }), + makeAddressedCappedEvent({ _id: 'evt-a' }), + makeAddressedCappedEvent({ _id: 'evt-b', payload: { content: 'again', messageId: 'msg-2' } }), + makeAddressedCappedEvent({ _id: 'evt-c', payload: { content: 'and again', messageId: 'msg-3' } }), ], messages: [ { _id: 'msg-1', isBot: true, self: false }, @@ -1742,49 +1767,6 @@ describe('performRun — ADR-018 enforcement', () => { ); }); - test('cascade: a kernel-dampened mention bypasses the cap but still spends broadcast liveness', async () => { - const { post } = makeClient({ - events: [ - // First broadcast fills the local cap without entering a claim race. - makeEvent({ - _id: 'evt-broadcast-before', - type: 'message.posted', - payload: { content: 'ambient work', dmKind: 'agent-agent' }, - }), - // The named bot-to-bot mention remains live even though the cap is - // full. Its completed turn must still record before the next wake. - makeClaimEvent({ - _id: 'evt-mentioned', - payload: { content: '@my-stub please weigh in', messageId: 'msg-mentioned' }, - }), - makeEvent({ - _id: 'evt-broadcast-after', - type: 'message.posted', - payload: { content: 'ambient work again', dmKind: 'agent-agent' }, - }), - ], - messages: [{ _id: 'msg-mentioned', isBot: true, self: false }], - }); - const spawn = jest.fn(async () => ({ text: 'NO_REPLY' })); - const { stop } = run( - { name: 'stub', detect: stubAdapter.detect, spawn }, - { cascadeCap: 1, cascadeAddressedGrace: 0 }, - ); - await drainMicrotasks(); - stop(); - - // The mention is the second spawn. Before the exemption it was refused at - // the cap; without the completion-time record the final broadcast would - // instead be admitted. - expect(spawn).toHaveBeenCalledTimes(2); - expect(post).toHaveBeenCalledWith( - '/api/agents/runtime/events/evt-broadcast-after/ack', - expect.objectContaining({ - result: expect.objectContaining({ outcome: 'no_action', reason: 'cascade-cap' }), - }), - ); - }); - test('losing a claim on a HUMAN message still resets the cascade streak', async () => { // Regression for the starvation bug: `record()` lives at the end of // runTurn, and the claim stand-down returns before runTurn — so the seat @@ -1840,8 +1822,8 @@ describe('performRun — ADR-018 enforcement', () => { // human trying to explain a silent seat. const { post } = makeClient({ events: [ - makeClaimEvent({ _id: 'evt-a', type: 'dm.message' }), - makeClaimEvent({ _id: 'evt-b', type: 'dm.message', payload: { content: 'again', messageId: 'msg-2' } }), + makeAddressedCappedEvent({ _id: 'evt-a' }), + makeAddressedCappedEvent({ _id: 'evt-b', payload: { content: 'again', messageId: 'msg-2' } }), ], messages: [ { _id: 'msg-1', isBot: true, self: false }, diff --git a/cli/package.json b/cli/package.json index 9b663a856..d8e62778a 100644 --- a/cli/package.json +++ b/cli/package.json @@ -2,7 +2,7 @@ "name": "@commonlyai/cli", "version": "0.1.14", "license": "Apache-2.0", - "description": "The Commonly CLI — connect agents, manage pods, iterate fast", + "description": "The Commonly CLI \u2014 connect agents, manage pods, iterate fast", "type": "module", "main": "./src/index.js", "bin": { diff --git a/cli/src/lib/enforcement.js b/cli/src/lib/enforcement.js index a916f44a2..4d88722cc 100644 --- a/cli/src/lib/enforcement.js +++ b/cli/src/lib/enforcement.js @@ -62,18 +62,37 @@ export const classifyTrigger = (event, recentMessages) => { // race is a free stand-down. export const ADDRESSED_EVENT_TYPES = new Set(['chat.mention', 'thread.mention', 'dm.message']); -// These are the two event types the kernel's bot-to-bot mention dampener -// bounds as one shared budget. Keep the wrapper out of that same loop's -// admission path: named seats need to respond even when broadcasts have -// exhausted their cascade budget. The backend cannot be a runtime import of -// the published CLI, so cli/__tests__/mention-event-types.contract.test.mjs -// imports both modules and pins the two lists together. -// -// Agent-DM wakes also arrive as chat.mention, but carry payload.dmKind. They -// stay on this governor's bounded path: event type alone does not identify the -// producer that owns the kernel mention budget. dm.message is legacy consumer -// vocabulary and remains on that same ordinary addressed-grace path. -export const MENTION_EVENT_TYPES = new Set(['chat.mention', 'thread.mention']); +/** + * The subset of ADDRESSED that already has a PRODUCER-side bound, and is + * therefore exempt from this consumer-side cap. + * + * Derived, never retyped. @pod-architect's condition when ruling on the shape: + * two lists stating one rule drift the next time a mention type is added, and + * the drift is silent. Anything ending `.mention` is a mention; that is the + * rule, so it is written once as the rule. + * + * Why mentions specifically. `agentMentionService.isLoopDampened` counts events + * of `$in: MENTION_EVENT_TYPES` per (target, instance, pod) inside a window and + * suppresses bot→bot mention loops before they are ever enqueued. So a mention + * cascade is already bounded at the source, and this cap is a second brake on + * the one class that does not need it — while being the brake that actually + * fires. @ux-lead took two `chat.mention` refusals mid-thread while peers were + * naming it, and could not afterwards say which mentions it had missed. + * + * Why NOT the whole ADDRESSED set: `dm.message` has no producer-side dampener, + * so exempting it would remove the ONLY bound on agent↔agent DM ping-pong — + * precisely the cascade this governor exists for, in the one place with no + * second guard. @pod-architect caught that in their own first proposal. + * + * Note for whoever revisits: `thread.mention` is currently DECLARED AND NEVER + * EMITTED, so today this exempts exactly one live type, `chat.mention`. That is + * deliberate — when threading (#1045) starts emitting `thread.mention`, it + * inherits the exemption automatically, which is the correct default for a + * type that will carry the same producer-side dampener. + */ +export const MENTION_EVENT_TYPES = new Set( + [...ADDRESSED_EVENT_TYPES].filter((type) => type.endsWith('.mention')), +); // ── cascade governor ──────────────────────────────────────────────────────── @@ -226,24 +245,9 @@ export const createCascadeGovernor = ({ }; return { - admit(podId, trigger, eventType, payload = {}) { + admit(podId, trigger, eventType) { if (trigger !== 'agent') return { allowed: true, streak: 0, addressed: false }; const s = stateFor(podId); - const addressed = ADDRESSED_EVENT_TYPES.has(eventType); - if (MENTION_EVENT_TYPES.has(eventType) && !payload?.dmKind) { - // Named mentions are bounded upstream by the kernel's shared - // bot-to-bot mention dampener. A DM emits chat.mention too, but its - // dmKind identifies a different producer, so it remains locally - // bounded. Every admitted turn still reaches record() after it - // completes, consuming broadcast liveness rather than creating an - // unmetered second loop. - return { - allowed: true, - streak: s.streak, - addressed, - graceApplied: false, - }; - } // Being NAMED outranks a mechanical brake — the same judgement the claim // path already makes forty lines down in agent.js. Without this, a peer // can @mention a capped seat and get silence, with no signal to either @@ -251,9 +255,37 @@ export const createCascadeGovernor = ({ // 51 wakes and 28 consecutive cap refusals, five of them chat.mention, // and answered none of them. // - // Legacy direct-address events retain a GRACE, not an exemption. They - // still count toward the streak, so they terminate at cap + - // addressedGrace instead of never. + // The grace was not enough, and the reason is that it was the wrong + // instrument. A MENTION is exempt outright, because it already has a + // producer-side bound: `agentMentionService.isLoopDampened` suppresses + // bot→bot mention loops before they are enqueued. Capping mentions here + // is a second brake on the one class that does not need one — and it is + // the brake that fires. @ux-lead lost two `chat.mention` events mid- + // thread while peers were naming it. + // + // This does NOT restore the A-mentions-B echo the earlier comment warned + // about: that echo is exactly what `isLoopDampened` kills, at the source, + // and it kills it for bot→bot only, so a HUMAN naming a seat is never + // dampened at either layer. Which is the intended behaviour — being named + // by a person should always reach the seat. + // + // `dm.message` stays capped. It is ADDRESSED but has no producer-side + // dampener, so this cap is the only bound on agent↔agent DM ping-pong. + // Exempting the whole ADDRESSED set would have removed it. + // + // A mention still COUNTS toward the streak via record() — it just cannot + // be refused. So a mention storm still tightens the budget for ambient + // wakes; it simply never silences the person doing the naming. + if (MENTION_EVENT_TYPES.has(eventType)) { + return { + allowed: true, + streak: s.streak, + addressed: true, + graceApplied: false, + mentionExempt: true, + }; + } + const addressed = ADDRESSED_EVENT_TYPES.has(eventType); const limit = addressed ? cap + addressedGrace : cap; // `addressed` describes the EVENT; `graceApplied` describes what this // governor actually did with it. They diverge whenever addressedGrace is