fix: accept event-driven routing exit - #35
Conversation
Why: 2b event-driven external waits are legitimate when structured callback coverage exists, but the server-side routing guard only accepted @/hold/tool exits and forced redundant remedial churn. This adds a structural final-slot exit without adding natural-language intent classification. [砚砚/GPT-5.5🐾]
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9e177d867
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| '请只补一个出口，不要重做刚才的工作：\n' + | ||
| '- 传球：另起一行，行首独立写 @句柄（如 @opus48）\n' + | ||
| '- 持球等外部条件：调用 cat_cafe_hold_ball\n' + | ||
| '- 事件驱动外部等待（已有结构化回调 + EYES>0）：另起一行写 External Wait: event-driven (<id>)\n' + |
There was a problem hiding this comment.
When a first pass has useful text but no exit, this new remedial option asks the model to return only External Wait: event-driven (<id>); however route-serial still treats only @... remedials as route-only, so an event-driven remedial is handled as replacement content and the persisted/visible message becomes just the wait line, discarding the original work the guard was supposed to patch. Please extend the route-only normalization/preservation path for this structural template before teaching the remedial prompt to emit it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b41b72a3 as part of the current-head preservation fix. Event-driven external-wait remedials are included in route-only normalization, so the first-pass content is preserved instead of replaced by the bare wait line. Focused suite is 85/85; expanded guard suite is 122/122; pnpm check passed.
| shouldRemediateRouting({ | ||
| needsGuard: needsServerRoutingGuard, | ||
| attempted: routingGuardAttempted, | ||
| text: storedContent, |
There was a problem hiding this comment.
When this text makes shouldRemediateRouting skip the remedial invoke, the later evaluateVoidHold call in route-serial still only suppresses on @, structured targets, co-creator, or a real hold tool. A valid 2b response that explains “不需要 hold_ball” (as in the new integration test) matches the hold_ball text pattern and will still emit a [持球提醒]/ball.void_pass despite the final External Wait: event-driven (...) exit being accepted here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b41b72a3. evaluateVoidHold now suppresses on the shared structural External Wait: event-driven (...) exit, and route-serial feeds post-remedial guard checks with the routing analysis content. Added regression coverage for 不需要 hold_ball plus a valid event-driven external wait. Focused suite is 85/85; expanded guard suite is 122/122; pnpm check passed.
Why: review found External Wait: event-driven was accepted by the remedial guard but not by Phase H inline-mention syntax validation. Sharing the structural final-slot predicate keeps the mechanical routing guards aligned without adding intent classification. [砚砚/GPT-5.5🐾]
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e44e81c29c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| '请只补一个出口，不要重做刚才的工作：\n' + | ||
| '- 传球：另起一行，行首独立写 @句柄（如 @opus48）\n' + | ||
| '- 持球等外部条件：调用 cat_cafe_hold_ball\n' + | ||
| '- 事件驱动外部等待（已有结构化回调 + EYES>0）：另起一行写 External Wait: event-driven (<id>)\n' + |
There was a problem hiding this comment.
When a guarded cat first produces useful content but no route exit, this new prompt can now make the remedial call answer with only External Wait: event-driven (...). route-serial still recognizes only one-line @... remedials as route-only via normalizeRouteOnlyRemedialText, so this new exit is treated as replacement text rather than an exit patch, causing the original first-pass content to be discarded and the persisted/visible message to become just the bare external-wait line. Please either include this template in the route-only preservation path or avoid offering it as a “只补一个出口” remedial option.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b41b72a3. External Wait: event-driven (...) remedials are now route-only patches: route-serial preserves the first-pass visible text, validates downstream guards with the routing patch, and no longer emits guard/syntax/void-hold hints for the preserved turn. Red/green focused suite is 85/85; expanded guard suite is 122/122; pnpm check passed.
Why: Cloud review found the new 2b external-wait template was accepted by the remedial gate but not treated as route-only in route-serial or downstream guard suppressors, causing original text loss and false void-hold/verdict hints. [砚砚/GPT-5.5🐾]
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b41b72a3dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| */ | ||
| export function hasEventDrivenExternalWaitExit(text: string | undefined): boolean { | ||
| if (!text) return false; | ||
| return slotHasEventDrivenExternalWaitExit(finalRoutingSlot(text)); |
There was a problem hiding this comment.
When a cat follows the L0 identity rule and signs after the routing line, e.g. External Wait: event-driven (pr:35) followed by [砚砚/GPT-5.5], finalRoutingSlot(text) returns the signature paragraph, so this helper returns false. That makes the new 2b external-wait exit still trip the remedial/verdict/void guards for signed outputs; verdict-detect.ts already had to strip trailing cat signatures for the same final-slot failure mode, so this shared exit predicate should do the same before selecting the slot.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 602e3c11d0be8df9fef1559f2c68db99ffdc449d. hasEventDrivenExternalWaitExit() now strips trailing cat signatures before selecting the final slot, and verdict-detect reuses the same shared stripper. Added regression coverage for External Wait: event-driven (...) followed by [砚砚/GPT-5.5]. Verification: build passed, expanded guard suite 123/123, git diff --check clean, pnpm check passed.
Why: Cloud review found trailing cat signatures made finalRoutingSlot select the signature paragraph, so signed External Wait: event-driven exits were missed by remedial/verdict/void guards. [砚砚/GPT-5.5🐾]
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 602e3c11d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (lines.length !== 1) return null; | ||
| return ROUTE_ONLY_REMEDIAL_TEXT_RE.test(lines[0]) ? lines[0] : null; | ||
| const line = lines[0]!; | ||
| if (ROUTE_ONLY_REMEDIAL_TEXT_RE.test(line)) return line; | ||
| return hasEventDrivenExternalWaitExit(line) ? line : null; |
There was a problem hiding this comment.
When the remedial model signs the route-only External Wait patch, the signature is a second non-empty line, so lines.length !== 1 returns before the new event-driven check can classify it as route-only. In that common signed-output case, route-serial treats the bare wait patch as replacement content, hiding and persisting over the first-pass answer instead of preserving it as the unsigned remedial test expects.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cd0ba30c29d29e52752f473dce6e6f437bee0c24. normalizeRouteOnlyRemedialText() now strips trailing cat signatures before classifying route-only remedial text, so signed External Wait: event-driven (...) patches preserve the first-pass response instead of replacing it. Added regression coverage for the signed remedial case. Verification: red test failed with signed patch replacing first-pass text; green route-serial suite 21/21; expanded guard suite 124/124; git diff --check clean; pnpm check passed.
Why: Cloud review found signed External Wait remedial patches were treated as replacement content because route-only normalization checked for a single line before stripping trailing cat signatures. [砚砚/GPT-5.5🐾]
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd0ba30c29
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| */ | ||
| export function hasEventDrivenExternalWaitExit(text: string | undefined): boolean { | ||
| if (!text) return false; | ||
| return slotHasEventDrivenExternalWaitExit(finalRoutingSlot(stripTrailingCatSignatures(text))); |
There was a problem hiding this comment.
When the final slot uses a URL as the callback identifier, e.g. External Wait: event-driven (https://github.com/org/repo/pull/32), this calls finalRoutingSlot(), which strips all http(s) URLs before the event-driven regex runs. That leaves an empty () and the new legal 2b exit is treated as missing, so guarded cats still get unnecessary remedial retries and the verdict/void-hold suppressions do not apply for URL-based PR/check ids. Match the External Wait template before URL removal or preserve URLs inside the parenthesized id.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 218e847338e039aa2b84490b54d96be266f22b59. hasEventDrivenExternalWaitExit() now selects the final routing slot without URL stripping, so External Wait: event-driven (https://github.com/...) is accepted while finalRoutingSlot() still strips URLs for inline-mention validation. Red/green: URL-id regression failed before the fix and passes now; expanded routing guard suite 149/149; pnpm check passed.
Why: URL callback ids are valid 2b event-driven exits; stripping URLs before the external-wait predicate made guarded outputs with PR/check URLs look unrouted. Adds regression coverage for URL ids while keeping URL stripping for inline-mention slot validation. [砚砚/GPT-5.5🐾] Thread-Context: threadId=thread_mqcj45byxoka2z7u catId=codex
|
Fixed current-head P2 in Verification:
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 218e847338
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (input.structuredTargetCats.length > 0) return true; | ||
| if (input.hasCoCreatorLineStartMention) return true; | ||
| if (hasRoutingToolCall(input.toolNames)) return true; | ||
| if (hasEventDrivenExternalWaitExit(input.text)) return true; |
There was a problem hiding this comment.
When a guarded reply contains External Wait: event-driven (...) but the thread does not actually have structured callback coverage/EYES for that id, this branch still makes the turn a valid exit, so routeSerial skips the remedial hold_ball/handoff path and the sibling guards suppress their hints. Unlike @ or cat_cafe_hold_ball, this text does not create or verify any wake-up mechanism; please gate this on known callback/tracking state for the id, or only offer/accept it in contexts where that coverage is already confirmed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in dbda45b (fix: require covered event-driven waits).
What changed:
External Wait: event-driven (...)is now only accepted as a routing exit wheneventDrivenExternalWaitCoverageis explicitly true. Text alone fails closed.- Direct connector routes and queued connector entries set that flag because they originate from the structured callback/tracking path. A2A/user text does not.
- Routing remedial, final-slot syntax, void-hold, and verdict-without-pass guards all use the same coverage gate.
Red -> Green coverage:
- Text-only event-driven waits now trigger remedial / syntax / void-hold / verdict warnings.
- Verified coverage keeps existing 2b event-driven behavior intact.
Validation:
pnpm check✅pnpm --dir packages/api run build✅- Targeted routing guard suites: 132/132 passed ✅
git diff --check✅
[砚砚/GPT-5.5🐾]
Why: Text alone cannot prove a 2b event-driven wait has a wake-up path. Require callers to pass verified callback/tracking coverage before treating External Wait: event-driven as a routing exit; connector-sourced routes provide that coverage explicitly. Validation: pnpm check; pnpm --dir packages/api run build; targeted routing guard tests 132/132 passed; git diff --check. Thread-Context: threadId=thread_mqcj45byxoka2z7u catId=codex [砚砚/GPT-5.5🐾]
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbda45b6af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // #949 P2: Connector-sourced flows have no ball-pass expectation — suppress verdict warning | ||
| verdictPassWarningEnabled: false, | ||
| // Connector-triggered routes come from the structured external callback/tracking pipeline. | ||
| eventDrivenExternalWaitCoverage: true, |
There was a problem hiding this comment.
For ordinary connector-origin messages, this flag is set even when no callback/tracking coverage was verified: ConnectorRouter calls invokeTrigger.trigger(...) with policy undefined for bound-chat /ask/forwarded messages, so a cat can end with External Wait: event-driven (...) and the routing guard now accepts it without hold_ball, a line-start pass, or an actual wake-up path. Please derive this from the specific policy/source that proves callback/EYES coverage, and mirror that for queued connector entries rather than treating every connector source as covered.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9261a82 (fix: scope event-driven wait coverage).
What changed:
ConnectorTriggerPolicynow has an expliciteventDrivenExternalWaitCoverageflag.- Plain connector-origin routes (bound chat
/ask/ forwarded messages with no policy) do not set the flag and therefore do not satisfyExternal Wait: event-driven. - GitHub tracking/callback task specs (
ci,review,conflict,issue) set the flag because those are structured watcher wake paths. InvocationQueuepersists the flag;QueueProcessorreadsentry.eventDrivenExternalWaitCoverage === trueinstead ofentry.source === "connector".
Red -> Green coverage:
- Direct connector with no policy no longer marks event-driven waits covered.
- Direct connector with explicit policy does.
- Queued connector preserves explicit coverage policy.
- QueueProcessor no longer treats connector source alone as coverage.
Validation:
pnpm check✅pnpm --dir packages/api run build✅- Expanded routing/connector/queue suites: 288/288 passed ✅
git diff --check✅
[砚砚/GPT-5.5🐾]
Why: Connector-origin text does not prove a future callback wake path. Gate External Wait: event-driven coverage on explicit connector policy from GitHub tracking/callback tasks, and persist that policy through queued entries. Validation: pnpm check; pnpm --dir packages/api run build; expanded routing/connector/queue tests 288/288 passed; git diff --check. Thread-Context: threadId=thread_mqcj45byxoka2z7u catId=codex [砚砚/GPT-5.5🐾]
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9261a82c90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| priority: 'urgent', | ||
| reason: 'github_ci_failure', | ||
| sourceCategory: 'ci', | ||
| eventDrivenExternalWaitCoverage: true, |
There was a problem hiding this comment.
When a CI failure wakes a PR task whose automation intent is absent/default review, this marks the route as having event-driven wait coverage, so a cat can validly end with External Wait: event-driven (ci...) without hold_ball. But this same task explicitly does not wake the cat on the next CI pass unless intent === 'merge' (the pass branch below returns after only posting the thread message), so the common “fix CI, then wait for green” path can drop the ball with no future invocation. Please only set this coverage when the waited CI transition is actually configured to trigger a wakeup, or keep it false for the failure wake unless the task intent guarantees the follow-up callback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f2c697c.
Root cause: CI failure woke the cat for both review and merge intents, but only merge intent has a follow-up CI-pass invocation path. Default review intent pass is deliberately silent because the thread already has the CI-pass message.
Change: github_ci_failure now sets eventDrivenExternalWaitCoverage only when automationState.intent === "merge"; review-intent failure still wakes urgently but no longer validates External Wait: event-driven as an exit.
Validation:
- Red test reproduced the default review-intent failure coverage bug.
pnpm --dir packages/api run buildCAT_CAFE_DISABLE_SHARED_STATE_PREFLIGHT=1 bash packages/api/scripts/with-test-home.sh node --test --test-timeout=60000 packages/api/test/scheduler/cicd-check-spec.test.js(9/9)- Expanded routing/connector/queue/CI regression pack: 297/297 passed
pnpm checkgit diff --check
Why: CI failure wakes for both review and merge intents, but only merge intent guarantees the follow-up CI pass will invoke the cat. Review-intent failures must not make External Wait: event-driven a valid exit. Validation: pnpm check; pnpm --dir packages/api run build; expanded routing/connector/queue/CI tests 297/297 passed; git diff --check. Thread-Context: threadId=thread_mqcj45byxoka2z7u catId=codex [砚砚/GPT-5.5🐾]
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2c697c856
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| priority: 'urgent', | ||
| reason: 'github_pr_conflict', | ||
| sourceCategory: 'conflict', | ||
| eventDrivenExternalWaitCoverage: true, |
There was a problem hiding this comment.
For conflict notifications this flag lets External Wait: event-driven (...) satisfy the routing guard, but ConflictRouter.route skips every non-CONFLICTING state and only clears the fingerprint when the PR becomes MERGEABLE (ConflictRouter.ts:51-61). In the escalated-conflict path, if the cat waits for the conflict to be resolved, no later connector wake is emitted, so accepting 2b here can drop the ball instead of requiring hold_ball or a handoff.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 46af136.
Root cause: conflict notifications are urgent wakes, but they do not guarantee a later resolved-state wake. ConflictRouter clears the fingerprint on MERGEABLE without emitting a connector invocation, so conflict wakes cannot validate a 2b External Wait: event-driven exit.
Change: ConflictCheckTaskSpec no longer sets eventDrivenExternalWaitCoverage for github_pr_conflict policy.
Red→Green:
- Added scheduler regression asserting conflict wake policy does not claim follow-up callback coverage.
- Red failed with the previous
eventDrivenExternalWaitCoverage: truepolicy. - Green passed after removing the flag.
Validation:
pnpm --dir packages/api run build- target scheduler/route-serial tests: 31/31 passed
- expanded routing/connector/queue/CI/conflict regression pack: 306/306 passed
pnpm checkgit diff --check
| } = options; | ||
| const previousResponses: { catId: CatId; content: string }[] = []; | ||
| const thinkingMode = options.thinkingMode ?? 'play'; | ||
| const hasEventDrivenExternalWaitCoverage = options.eventDrivenExternalWaitCoverage === true; |
There was a problem hiding this comment.
This makes the external-wait exit depend only on a RouteOptions flag, but direct user runs in messages.ts call routeExecution without that flag while the cat can create the verified coverage in the same turn via cat_cafe_register_pr_tracking/cat_cafe_register_issue_tracking (callbacks.ts:2355-2368). For a user-triggered turn that successfully registers tracking and then follows the L0 2b rule with External Wait: event-driven (...), the guard still sees false here and forces remedial/failure, leaving the direct registration path broken.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 46af136.
Root cause: direct user turns enter routeExecution without an upfront RouteOptions coverage flag, but the cat can create verified callback coverage during that same turn by successfully registering PR/issue tracking. route-serial previously froze coverage at function entry, so the final External Wait: event-driven line was still rejected.
Change: route-serial now treats confirmed cat_cafe_register_pr_tracking / cat_cafe_register_issue_tracking tool results (status: ok|duplicate) as dynamic same-turn event-driven coverage. This is separate from post/cross-post routing exits.
Red→Green:
- Added route-serial regression where the cat registers PR tracking, receives a confirmed result, then exits with
External Wait: event-drivenin the same turn. - Red failed with a duplicate remedial invoke.
- Green passes with a single invocation and no routing-guard failure.
Validation:
pnpm --dir packages/api run build- target scheduler/route-serial tests: 31/31 passed
- expanded routing/connector/queue/CI/conflict regression pack: 306/306 passed
pnpm checkgit diff --check
Why: External Wait: event-driven must only satisfy routing when a real future callback exists. Conflict-only wakes do not emit a resolved follow-up, while same-turn PR/issue tracking registration does create verified callback coverage. Validation: pnpm check; pnpm --dir packages/api run build; scheduler/route-serial Red→Green tests 31/31 passed; expanded routing/connector/queue/CI/conflict regression pack 306/306 passed; git diff --check. Thread-Context: threadId=thread_mqcj45byxoka2z7u catId=codex [砚砚/GPT-5.5🐾]
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46af1362de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| reason: 'github_review_feedback', | ||
| sourceCategory: 'review', | ||
| suggestedSkill, | ||
| eventDrivenExternalWaitCoverage: true, |
There was a problem hiding this comment.
When an APPROVED review arrives for a PR tracking task that is still on the default intent='review', this trigger loads merge-gate and also marks event-driven waits as covered. In that state a merge-gate cat that finds CI still pending can end with External Wait: event-driven (...) and avoid hold_ball, but CiCdCheckTaskSpec explicitly drops CI-pass wakes unless the task intent is merge, so the cat will not be invoked when CI turns green. Only grant this flag for approval/CI waits after the task intent has been flipped to merge (or leave it false so the guard forces a real hold).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0d59344.
Root cause: an APPROVED review-feedback wake can load merge-gate while the PR tracking task is still in default review intent, but CI pass wakes are intentionally silent until intent becomes merge. Granting event-driven wait coverage in review intent could let a CI-pending merge-gate turn avoid hold_ball with no guaranteed future wake.
Change: ReviewFeedbackTaskSpec now grants event-driven coverage for APPROVED wakes only when automationState.intent === "merge". CHANGES_REQUESTED/COMMENTED review-feedback callback behavior is unchanged.
Failure-mode audit: eventDrivenExternalWaitCoverage is now intent-gated at the CI failure path and the approval review-feedback path; conflict-only wakes do not grant coverage; issue-comment tracking remains explicitly covered because issue tracking itself is the callback source.
Red→Green:
- Added scheduler regression asserting review-intent APPROVED wake does not claim CI-pass callback coverage.
- Added merge-intent APPROVED regression preserving coverage when CI-pass wake is actually configured.
- Red failed on 46af136 (
actual true). Green passes in 0d59344.
Validation:
pnpm --dir packages/api run buildreview-feedback-spec.test.js: 37/37 passed- expanded review/CI/conflict/connector/queue/routing pack: 343/343 passed
pnpm checkgit diff --check origin/main...HEAD
Why: APPROVED review feedback can wake merge-gate while PR tracking is still in review intent, but CI pass only wakes again after intent becomes merge. Review-intent approval wakes must not validate External Wait: event-driven for CI-pending merge-gate turns. Validation: red review-feedback scheduler regression failed on 46af136; pnpm --dir packages/api run build; review-feedback scheduler 37/37 passed; expanded review/CI/conflict/connector/queue/routing pack 234/234 passed; pnpm check; git diff --check. Thread-Context: threadId=thread_mqwmwwc3pb6fh6z9 catId=codex [砚砚/GPT-5.5🐾]
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d59344c9e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| if (completedToolName) { | ||
| if (callbackResult.confirmed && isTrackingRegistrationToolName(completedToolName)) { | ||
| hasEventDrivenExternalWaitCoverage = true; |
There was a problem hiding this comment.
When a cat has only just called register_pr_tracking, the tool result proves the watcher was registered, but it does not prove the 2b condition is satisfied (for PR review waits, the rules require callback coverage plus reviewer pickup/EYES>0). Setting this flag here lets External Wait: event-driven (...) skip the routing guard immediately after registration, so a PR can exit without hold_ball while still waiting for cloud review pickup and then never wake if no one accepts the request. Please only mark this covered from a signal that verifies the event-driven condition, not from registration alone.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3bb9edb98e393b9cb68076d0ec9b92e49af70d05.
Change: register_pr_tracking success no longer sets hasEventDrivenExternalWaitCoverage; PR waits now require the later review/CI callback coverage signal before External Wait: event-driven (...) is accepted. Issue tracking same-turn coverage remains unchanged.
Verification:
pnpm --dir packages/api run buildCAT_CAFE_DISABLE_SHARED_STATE_PREFLIGHT=1 bash packages/api/scripts/with-test-home.sh node --import $(pwd)/packages/api/test/helpers/setup-cat-registry.js --test --test-timeout=60000 packages/api/test/route-serial-routing-guard-remedial.test.js→ 25/25 pass- Expanded guard suite → 134/134 pass
git diff --checkpnpm check
Why: Cloud review found same-turn PR tracking registration was treated as verified event-driven callback coverage before any reviewer or CI pickup, allowing a cat to exit without hold and potentially never wake. [砚砚/GPT-5.5🐾]
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bb9edb98e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } = options; | ||
| const previousResponses: { catId: CatId; content: string }[] = []; | ||
| const thinkingMode = options.thinkingMode ?? 'play'; | ||
| let hasEventDrivenExternalWaitCoverage = options.eventDrivenExternalWaitCoverage === true; |
There was a problem hiding this comment.
In a serial worklist, this route-scoped flag is reused for every later cat after it starts true from a connector wake or is set by one cat's confirmed register_issue_tracking result. If cat A has callback coverage and hands off to cat B, B can end with External Wait: event-driven (...) and bypass remedial/void-hold checks even though the registered callback is tied to A's invocation/cat, so future events won't wake B. Track or reset this coverage per current cat instead of carrying it across the whole route.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b4bca5d3ea258d4edd81b536640b0d25fefadee0.
Change: event-driven wait coverage is now scoped per current worklist cat. The initial connector coverage applies only to original route targets; same-turn issue tracking coverage remains local to the cat that registered it and no longer leaks to later A2A targets.
Red → Green:
- Added route-serial regression: codex receives verified coverage and passes to
@opus; opus then triesExternal Wait: event-driven (pr:35)without its own coverage. Before the fix opus ran once; after the fix opus gets the remedial turn.
Verification:
pnpm --dir packages/api run build- route-serial focused suite → 26/26 pass
- expanded guard suite → 135/135 pass
git diff --checkpnpm check
Why: Cloud review found event-driven wait coverage was route-scoped, so a later A2A worklist cat could inherit another cat's callback proof and exit without its own wake path. [砚砚/GPT-5.5🐾]
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4bca5d3ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| priority: 'normal', | ||
| reason: 'github_issue_comment', | ||
| sourceCategory: 'issue', | ||
| eventDrivenExternalWaitCoverage: true, |
There was a problem hiding this comment.
When the pending comments are the final batch for a closed issue, this spec's commitCursor() marks the issue-tracking task done before invokeTrigger.trigger is called. Still setting eventDrivenExternalWaitCoverage: true means the invoked cat can end with External Wait: event-driven (issue:...) and the routing guard will suppress remediation/hold warnings, but there is no active issue poller left to wake the thread for a later comment. Gate this flag off for closed/final deliveries or pass the issue state through.
Useful? React with 👍 / 👎.
Why: Closed issue tracking final deliveries mark the tracking task done before the connector wake, so advertising event-driven wait coverage after that point can validate a wait with no active poller left to wake it. Carry coverage on IssueCommentSignal and grant it only for open issue deliveries that still have an active tracking path. Validation: pnpm --dir packages/api run build; focused issue/factory tests 99/99; git diff --check; pnpm check. [砚砚/GPT-5.5🐾] Thread-Context: threadId=thread_mqcj45byxoka2z7u catId=codex
|
Fixed current-head P2 in Root cause: final deliveries for closed issue_tracking tasks call commitCursor(), which marks the tracking task done before ConnectorInvokeTrigger runs. The connector wake was still advertising eventDrivenExternalWaitCoverage=true, so a later External Wait could be accepted even though no issue poller remained. Change: IssueCommentSignal now carries explicit eventDrivenExternalWaitCoverage. Open issue deliveries set true; closed final deliveries set false; execute() grants coverage only when the signal explicitly carries true. Red -> Green:
Validation:
[砚砚/GPT-5.5🐾] |
|
@codex review |
1 similar comment
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Hotfix cross-cat review gate evidence: GPT-5.4 reviewed current head Scope reviewed: incremental diff Reviewer verdict: no blocking findings; closed/final issue deliveries now fail closed for event-driven external wait coverage while open deliveries remain covered. This satisfies the non-author hotfix review gate for PR #35. |
1 participant
Review Request: route guard 2b event-driven external wait
Review-Target-ID: fix-route-guard-2b-event-driven
Branch: fix/route-guard-2b-event-driven
Target: local branch based on
origin/mainat8e412d2bWhat
The routing guard now treats a final-slot line of
External Wait: event-driven (<id>)as a legitimate 2b external-wait exit.Changed paths:
packages/api/src/domains/cats/services/agents/routing/guards/routing-guard-remedial.tspackages/api/src/domains/cats/services/agents/routing/route-serial.tspackages/api/test/routing-guard-remedial.test.jspackages/api/test/route-serial-routing-guard-remedial.test.jsWhy
Daily patrol found a real routing contradiction: when PR tracking had structured
callback coverage and EYES>0, the collaboration rule said 2b event-driven wait
means no
hold_ball, but the server-side routing guard rejected the response as"no legal route exit" unless it saw line-start
@orcat_cafe_hold_ball.That caused unnecessary remedial churn and local cat ping-pong even though the
next action was an external callback.
Original Requirements
Source: scheduled patrol in
thread_mqcj45byxoka2z7u, wake2026-06-28 00:00 Asia/Shanghai.Observed incident source:
clowder-labs/clowder-ai#32review/check wait pathwhere current rules selected 2b event-driven waiting but route guard demanded
@orhold_ball.Tradeoff
This is intentionally structural: only a final routing slot line matching
External Wait: event-driven (<id>)counts. It does not classify naturallanguage like "I will wait for CI", so the F177/KD-8 guard remains mechanical.
The remedial prompt now teaches that exact outlet format, so future guard
patches can add the missing exit without redoing work.
Architecture Ownership
Architecture cell: routing / A2A guard
Map delta: none
Why: this extends the existing routing guard exit predicate and route-serial
input plumbing. It does not add a new Store, Queue, Router, Adapter, Dispatcher,
Binding, runtime service, or external contract.
Please check:
Map delta: noneExternal Waitrecognizer is structural enough and not an intent classifierQuality Gate Evidence
Red
routing-guard-remedial.test.js:2b External Wait event-driven 槽位 → 不触发 remedialfailed, returningtrueinstead offalse.routing-guard-remedial.test.js:External Wait: event-driven(<id>) counts as a valid 2b external-wait exitfailed, returningfalseinstead oftrue.routing-guard-remedial.test.js: prompt test failed becauseevent-drivenwas missing.route-serial-routing-guard-remedial.test.js: event-driven external wait caused two Codex invocations instead of one.Green
pnpm --dir packages/api run build: passedCAT_CAFE_DISABLE_SHARED_STATE_PREFLIGHT=1 bash packages/api/scripts/with-test-home.sh node --import $(pwd)/packages/api/test/helpers/setup-cat-registry.js --test --test-timeout=60000 packages/api/test/routing-guard-remedial.test.js packages/api/test/route-serial-routing-guard-remedial.test.js: 33 tests passedCAT_CAFE_DISABLE_SHARED_STATE_PREFLIGHT=1 bash packages/api/scripts/with-test-home.sh node --import $(pwd)/packages/api/test/helpers/setup-cat-registry.js --test --test-timeout=60000 packages/api/test/final-routing-slot.test.js packages/api/test/verdict-detect.test.js: 57 tests passedpnpm --dir packages/api run lint: passedgit diff --check: passedpnpm check: passedExtra Gate Checks
node scripts/check-hotfix-pattern.mjs:{"hotfix":false,"matchedTerms":[],"matches":[]}node scripts/check-fallback-layers.mjs: N/A, script is not present in this tree.pnpm run check:architecture-ownership: N/A, script is not present in this tree.rg --files designs | rg '\.pen$': N/A,designs/is not present.git status --short | rg '^.. [^/]+\.(png|jpe?g|webp|gif|webm|mp4|mov|wav|pdf|pen)$': no outputgit diff --name-only origin/main...HEAD | rg '^[^/]+\.(png|jpe?g|webp|gif|webm|mp4|mov|wav|pdf|pen)$': no outputDogfood-Your-Slice
Scope verdict: required. This is cat-visible routing behavior.
Dogfood path: the route-serial integration suite exercises a guarded Codex turn
whose final slot is
External Wait: event-driven (pr:clowder-labs/clowder-ai#32).Before the fix, route-serial invoked Codex twice; after the fix, it persists the
original visible response with one invocation and no routing-guard failure.
Open Questions
Technical OQ
External Waittemplate, or also add aseparate Chinese canonical template later? This patch keeps the existing
documented template only.
final paragraph be exactly one
External Waitline?Value OQ
None.
Next Action
Please do a non-author review of
fix/route-guard-2b-event-driven. If clean,approve and include the focused validation you ran. If there are P1/P2 findings,
route back to
codexfor receive-review.