Greptile Summary

This PR fixes a bug where hooks configured with deliver: false were still injecting a system event into the main session after a successful run, causing the result to be announced on the last active channel (Telegram, Discord, etc.) — defeating the purpose of silent background automations.

The fix is a minimal one-line guard in dispatchAgentHook (src/gateway/server/hooks.ts): the condition for calling enqueueSystemEvent on the success path is tightened from !result.delivered to !result.delivered && value.deliver !== false. The error path intentionally still surfaces to the main session (documented with an inline comment), preserving debuggability.

Three new integration tests are added to server.hooks.test.ts covering the key scenarios:

• Success path with deliver: false via a hook mapping (no system event injected)
• Error path with deliver: false (error still surfaced to main session)
• Direct /hooks/agent endpoint with deliver: false (no system event injected)

Notes:

• The fix is correct and well-scoped. The strict !== false comparison is appropriate — deliver: undefined preserves the existing behaviour.
• Tests 1 and 3 use setTimeout(200) for negative assertions (no system event), which is a common pattern for fire-and-forget async but carries a small risk of flakiness under CI load. See inline comment for details.

Confidence Score: 4/5

• Safe to merge — fix is minimal, correct, and well-tested; only risk is minor test flakiness in slow CI environments.
• The production code change is a single, well-reasoned guard that cleanly addresses the root cause without affecting any other code paths. Error surfacing is preserved as documented. Three new tests cover all stated scenarios. The slight deduction is for the setTimeout(200)-based negative assertions in two tests, which could produce non-deterministic results under CI resource contention.
• No files require special attention; the setTimeout(200) pattern in server.hooks.test.ts (lines 547 and 607) is worth monitoring in CI for intermittent failures.

This is a comment left during a code review.
Path: src/gateway/server.hooks.test.ts
Line: 546-547

Comment:
**Fixed-delay wait for negative assertions may be flaky in slow CI**

Both the suppression tests (for mapping and direct `/hooks/agent`) rely on a bare `setTimeout(200)` to allow the `void` async dispatch to complete before asserting `peekSystemEvents(...).length === 0`. With an immediately-resolving mock this is almost always fine, but under heavy CPU contention in CI the microtask queue may not have fully drained by the time the timer fires, which could produce a spurious pass (the system event simply hadn't arrived yet rather than being truly suppressed).

Consider a lightweight poll-until-idle helper, or — since `cronIsolatedRun` is already asserted to have been called — awaiting the completion of the underlying task more deterministically:

```typescript
// One option: poll until the mock has been called, with a shorter timeout
const deadline = Date.now() + 2000;
while (cronIsolatedRun.mock.calls.length === 0 && Date.now() < deadline) {
  await new Promise((r) => setTimeout(r, 10));
}
```

At minimum, adding a comment explaining why 200 ms is sufficient (mock resolves synchronously in the next microtask) would help future readers understand the intent.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: b0edd79}

@vincentkoc Mind taking a look at this one when you get a chance? It’s a very small hooks fix + tests, and it unblocks a pretty practical Gmail webhook automation case: deliver: false currently still leaks a system event into the main session, so “silent” email labeling flows still notify the user. This patch makes silent hooks actually silent while preserving error surfacing. If it looks good, would love a review/merge.

Closing this as duplicate or superseded after Codex automated review.

Close #49234 as superseded. Current main still has the original one-bit !result.delivered hook fallback, but #49234's narrow guard was flagged for hiding non-throw status:"error" hook failures. Open PR #55761 now tracks the same remaining deliver:false leak with the broader announcement-policy fix and tests.

Best possible solution:

Close #49234 as superseded and keep the remaining hooks work on #55761, where the fix should separate main-session announcement policy from delivery state, keep successful deliver:false hooks silent, preserve returned error visibility, and land with deterministic regression tests.

What I checked:

• Current main still has the old fallback: At current main SHA 54f8e41, dispatchAgentHook still injects a main-session system event whenever !result.delivered; there is no deliver:false, status !== "ok", deliveryAttempted, or explicit announce policy guard here. (src/gateway/server/hooks.ts:97, 54f8e4145e43)
• Both direct and mapped hook paths depend on that fallback: Direct /hooks/agent dispatch passes the normalized deliver value into dispatchAgentHook, and mapped hooks do the same via resolveHookDeliver(mapped.action.deliver), so the fallback policy in server/hooks.ts controls both paths covered by fix(hooks): suppress system event injection when deliver is false #49234. (src/gateway/server-http.ts:739, 54f8e4145e43)
• Current result type has delivery state but no explicit announce policy: RunCronAgentTurnResult currently exposes delivered and deliveryAttempted; the broader announceToMain policy described in fix(hooks): suppress silent shared-hook fallback while preserving error surfacing #55761 is not on main yet. (src/cron/isolated-agent/run.types.ts:3, 54f8e4145e43)
• Superseding related PR: The provided GitHub context shows open PR fix(hooks): suppress silent shared-hook fallback while preserving error surfacing #55761 (fix(hooks): suppress silent shared-hook fallback while preserving error surfacing, head b78bb80) explicitly covers the same deliver:false hook leak, lists fix(hooks): suppress system event injection when deliver is false #49234/fix(hooks): suppress system event injection when deliver is false [AI-assisted] #36332 as the narrow PR lineage, and addresses the fix(hooks): suppress system event injection when deliver is false #49234 review concern by always surfacing non-throw status:"error" results. (b78bb805d47b)

So I’m closing this here and keeping the remaining discussion on the canonical linked item.

Codex Review notes: model gpt-5.5, reasoning high; reviewed against 54f8e4145e43.