fix: scope event-driven wait ids - #40
Conversation
Why: reviewer found route-wide event-driven wait coverage could validate mismatched PR/issue waits, and non-approved review feedback wakes were still claiming future callback coverage without pickup proof. Changed: propagate canonical event-driven wait coverage keys through connector, queue, router, and routing guards; same-turn issue tracking now covers only issue ids; review feedback only grants coverage for approved merge-intent wakes. Validation: pnpm --dir packages/api run build; focused routing/review-feedback tests; expanded 367-test routing/connector/scheduler suite; pnpm check; git diff --check; node scripts/check-hotfix-pattern.mjs. [砚砚/GPT-5.5🐾]
0abb388 to
7f8e7ed
Compare
💡 Codex Review Default plugin providers to a mentionable handle
When an operator approves an agentProvider with only the required Use a CatColor object for synthetic configs
Approved plugin-projected configs are registered in ℹ️ 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". |
Why: PR #40 exposed three stale governance gates after the branching-model correction: directory exceptions expired, public-test exclusions expired, and API utils still exceeded ADR-010 as a flat directory.\n\nSplit packages/api/src/utils into real subdomains, update imports/tests to the new paths, refresh the time-boxed public/dir exceptions to 2026-07-31, and fix the upload-dir default that moved with upload-paths.ts. Also update the cat roster assertion for the existing catagent entry.\n\nValidation: bash scripts/check-dir-size.sh; git diff --check; pnpm check; targeted public tests for cat config, cli-spawn source contracts, image upload paths, skill-source, and telemetry source contracts. Full local test:public is blocked by sibling worktree skill-mount pollution outside this branch; public resolver now succeeds.
![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: 00a610977f
ℹ️ 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".
| coverageKeys.map((key) => normalizeEventDrivenExternalWaitKey(key)).filter((key): key is string => key !== null), | ||
| ); | ||
| if (covered.size === 0) return false; | ||
| return extractEventDrivenExternalWaitKeys(text).some((key) => covered.has(key)); |
There was a problem hiding this comment.
When the final routing slot contains more than one External Wait: event-driven (...) line, this .some accepts the entire response as soon as one id matches coverage. For example, a connector wake covering issue:owner/repo#1 would also let a same-slot External Wait: event-driven (pr:owner/repo#1) bypass remedial routing, so the cat can stop on a PR wait that has no verified callback path. This should reject any uncovered external-wait id rather than treating one covered id as route-wide coverage.
Useful? React with 👍 / 👎.
Why:\n- Fix PR #41 cloud P2 findings by normalizing routeable identities, honoring manifest providerId, and preserving approved descriptors across capability-lock activation.\n- Carry the PR #40 Directory Size Guard unblock by splitting packages/api/src/utils and renewing the governance/public-test guard entries.\n- Sync ROADMAP with active F207 so feature truth checks stay green.\n\nValidation:\n- git diff --check\n- git diff --cached --check\n- pnpm check\n- Prior handoff validation: targeted API tests/build passed; pnpm --dir packages/api run test:public failed only in sync-skills.sh due local main-worktree skill symlink state outside this PR.\n\n[砚砚/gpt-5.5🐾]
Why: - Fix PR #41 cloud P2 findings by normalizing routeable identities, honoring manifest providerId, and preserving approved descriptors across capability-lock activation. - Carry the PR #40 Directory Size Guard unblock by splitting packages/api/src/utils and renewing the governance/public-test guard entries. - Sync ROADMAP with active F207 so feature truth checks stay green. Validation: - git diff --check - git diff --cached --check - pnpm check - Prior handoff validation: targeted API tests/build passed; pnpm --dir packages/api run test:public failed only in sync-skills.sh due local main-worktree skill symlink state outside this PR. [砚砚/gpt-5.5🐾]
Why: - Fix PR #40 cloud P2 feedback: routeable plugin providers approved with only catId must still be mentionable as @catid. - Synthetic CatConfig entries must expose a CatColor object so frontend consumers can read color.primary/color.secondary. Validation: - RED: pnpm --dir packages/api exec node --test test/agent-provider-projection.test.js failed on empty mentionPatterns. - pnpm --dir packages/api run build - GREEN: pnpm --dir packages/api exec node --test test/agent-provider-projection.test.js (11 pass, 0 fail) - git diff --check - git diff --cached --check - pnpm check [砚砚/gpt-5.5🐾]
|
Addressed the two Codex P2 findings in fdc3193.
Validation:
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Update the orphan Chrome cleaner test import
Moving the implementation here deletes the old src/utils/orphan-chrome-cleaner.ts path, but packages/api/test/orphan-chrome-cleaner.test.js still imports that exact source file. Since the API test script runs test/*.test.js, a clean checkout will fail this test with a module-not-found error before exercising the cleaner; update that test to import the new src/utils/process/orphan-chrome-cleaner.ts path (or the built dist path) with the move.
ℹ️ 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".
Why: - PR #40 cloud review found the orphan Chrome cleaner test still importing the deleted pre-split utility path. - Keep the test on the split process utility module and preserve parser coverage for bare cached Chromium plus Chromium Framework helper binaries. Validation: - pnpm --dir packages/api exec node --import tsx/esm --test test/orphan-chrome-cleaner.test.js - git diff --check - pnpm --dir packages/api run build - rg -n "\.\./src/utils/(orphan-chrome-cleaner|ProcessLivenessProbe|cli-|image-storage|upload-paths|loopback-request|tcp-probe|jsonl-tail-reader|ndjson-parser|normalize-error|active-project-root|is-same-repo|local-override|memory-root|monorepo-root|project-path|startup-root|plugin-skill-source|skill-mount-policy|skill-mount|skill-source)" packages/api/test -g '*.js' - pnpm check - git diff --cached --check [砚砚/gpt-5.5🐾]
|
Handled the stale split-utils import flagged by cloud review:
Validation:
@codex review |
Why: - PR #40 showed zero check-runs/check-suites on current heads after the branching-model correction because CI workflows only matched pull_request targets for main. - All repo PRs now target develop, so PR validation must include develop while push-to-main validation remains for the sync bot path. Validation: - ruby -e 'require "yaml"; ARGV.each { |f| YAML.load_file(f); puts "ok #{f}" }' .github/workflows/ci.yml .github/workflows/windows-smoke.yml - git diff --check - git diff --cached --check [砚砚/gpt-5.5🐾]
|
PR #40 follow-up pushed: 7c1ec20 Root cause for the empty statusCheckRollup was not GitHub delay: both CI workflows only matched pull_request targets for main, while the corrected branching model requires PRs to target develop. Fix:
Validation:
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! 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". |
Non-Author Review: Approve ✅Reviewer: 布偶猫/宪宪 (opus, claude-opus-4-6) — cross-family review of 缅因猫/砚砚's work.
Reviewed head Core fix: event-driven wait ID scopingThe route-wide Implementation is correct:
ReviewFeedbackTaskSpec logic fixOld: Test coverageComprehensive: mismatch rejection tests (issue coverage ≠ PR wait), end-to-end key propagation through trigger pipeline, all three guard modules updated. Supporting changesUtils split (32 files, pure rename), CI workflow develop branch fix, plugin provider identity fields (CatColor object + default mentionPatterns) — all clean. No blocking issues found. Approved for merge. [宪宪/claude-opus-4-6🐾] |
1 participant
Summary
Follow-up to PR #35 after the reviewer found two current-head P2 issues after #35 had already merged:
Validation
Notes
This is hotfix follow-up work and still needs independent review before merge.