This repository was archived by the owner on Sep 28, 2026. It is now read-only.
Repository navigation
fix: thread-list matcher robustness + worktree preflight gate - #17
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe change adds Git-aware worktree preflight, exact per-node task-thread matching, claimed-thread exclusion, expanded lifecycle guidance, and validation tests. ChangesTask setup and resolution
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TaskSetupWorkflow as Task setup workflow
participant preflightWorktreeTarget
participant findExactThread
participant TaskRows as Task rows
TaskSetupWorkflow->>preflightWorktreeTarget: validate environment and Git status
preflightWorktreeTarget-->>TaskSetupWorkflow: allow worktree, root fallback, or unresolved_risk
TaskSetupWorkflow->>findExactThread: resolve run tag, node ID, client thread, and exclusions
findExactThread->>TaskRows: inspect name, title, summary, and preview
TaskRows-->>findExactThread: return matching metadata
findExactThread-->>TaskSetupWorkflow: return resolved thread ID
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+85
to
+96
| Match the run tag only in the bracketed exact form `[<runTag>]`, against the | ||
| explicit key list `name`/`title`/`summary`/`preview`. Open-source thread rows | ||
| carry `name` (optional user-facing title) and `preview` (first user message), | ||
| not `title`/`summary` (`thread_data.rs`); a matcher that reads only | ||
| `title`/`summary` misses a materialized, ready thread. Desktop rows have | ||
| shown `title`, and while a project task is loading, Codex can temporarily put the | ||
| requested title in `summary` — keep both keys too. Prefer `name`/`title` | ||
| (and `summary`) over `preview`: a parent thread's preview can embed worker | ||
| titles, so accept a preview-only hit just as a second-pass fallback, and | ||
| never match a row whose thread id is the orchestrator's own (or parent) | ||
| thread id, when that id is known. Keep the key list explicit; | ||
| never add a bare `id` fallback to list matching. |
Contributor
There was a problem hiding this comment.
🟡 Recommended task naming makes newly created worker tasks unfindable under the new stricter matching rules
The reference spawn pattern names a new task with only the lower-cased node id (args.task_name = node.id.toLowerCase() at skills/codex-graph/references/code-mode-script-patterns.md:125), which no longer satisfies the new matching rule that requires the bracketed run tag plus the exact-case node id in the same field, so a freshly created worker task can never be recognised in the task list.
Impact: Workflows generated from this guidance can stall waiting for a worker task that in fact exists but is never matched.
How the naming pattern and the new matcher contradict each other
findExactThread (skills/codex-graph/scripts/task_collection_harness.js:48-104, mirrored at skills/codex-graph/references/task-lifecycle.md:126-182) now requires, for every candidate field (name/title/summary/preview), both s.includes("[" + runTag + "]") and a case-sensitive token match on the node id.
The list row's name is populated by the create call's task_name. The reference spawn helper still sets task_name = node.id.toLowerCase() — no brackets, no run tag, and lower-cased so even \bN2A\b would not match n2a. A script that follows both references therefore never gets a name hit and depends entirely on the preview second-pass fallback (which the same docs say is only a fallback and can false-positive on the parent thread's preview).
skills/codex-graph/references/task-lifecycle.md:58-61 says every node's title must be [<runTag>] <label>; the spawn-args example should be updated to build task_name/title from that exact form.
Prompt for agents
The new matcher in findExactThread (skills/codex-graph/scripts/task_collection_harness.js and the mirrored snippet in references/task-lifecycle.md) requires the bracketed run tag `[<runTag>]` plus a case-sensitive node-id token in the same list-row field (name/title/summary/preview). However, the spawn-argument example in references/code-mode-script-patterns.md still sets `args.task_name = node.id.toLowerCase()`, which produces a row `name` with neither the bracketed tag nor an exact-case node id. Scripts following both references would never match on `name` and would rely solely on the preview fallback. Update the spawn-args pattern (and any related guidance) so the created task's name/title uses the documented `[<runTag>] <label>` form with the per-node run tag `<graphTag>-<nodeId>`, or relax the matcher to be case-insensitive on the node-id token.
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/codex-graph/scripts/task_collection_harness.js`:
- Around line 59-82: Require a resolved thread identifier in both resolver
patterns: in skills/codex-graph/scripts/task_collection_harness.js lines 59-82,
return false immediately when rowThreadId is absent, before clientThreadId or
metadata matching; in skills/codex-graph/references/task-lifecycle.md lines
137-160, document the same guard and add a fixture where an ID-less matching
metadata object precedes a valid thread row.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0af44466-6e88-4dd6-859b-4ff57178692d
📒 Files selected for processing (7)
CHANGELOG.mdskills/codex-graph/SKILL.mdskills/codex-graph/references/code-mode-script-patterns.mdskills/codex-graph/references/task-lifecycle.mdskills/codex-graph/scripts/task_collection_harness.jsskills/codex-graph/scripts/task_collection_harness.test.jsskills/codex-graph/scripts/validate_skill.py
Comment on lines
+59
to
+82
| const rowThreadId = row.threadId ?? row.thread_id ?? row.id ?? null; | ||
| if (rowThreadId != null && excluded.has(String(rowThreadId))) return false; | ||
| const listedClient = row.clientThreadId ?? row.client_thread_id ?? null; | ||
| if ( | ||
| clientThreadId && | ||
| listedClient && | ||
| String(listedClient) === String(clientThreadId) | ||
| ) { | ||
| return true; | ||
| } | ||
| // Explicit key list: OSS rows carry name/preview, Desktop rows have | ||
| // shown title/summary. Never add a bare `id` fallback here. | ||
| const strongHit = | ||
| fieldHit(text(row, "name")) || | ||
| fieldHit(text(row, "title")) || | ||
| fieldHit(text(row, "summary")); | ||
| const previewHit = allowPreview && fieldHit(text(row, "preview")); | ||
| if (!strongHit && !previewHit) return false; | ||
| // Projectless handles have no projectId; match the unique run tag alone. | ||
| // When a project is bound, prefer projectId match but do not require it | ||
| // while setup is still loading (projectId may be absent on early rows). | ||
| const projectRequired = projectId != null && projectId !== ""; | ||
| const listedProject = row.projectId ?? row.project_id; | ||
| return !projectRequired || listedProject == null || listedProject === projectId; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require a thread identifier for every resolved match. Both resolvers can return an ID-less nested metadata object before they reach the actual thread row. The pending handle then remains unresolved.
skills/codex-graph/scripts/task_collection_harness.js#L59-L82: returnfalsewhenrowThreadIdis absent before checkingclientThreadIdor metadata fields.skills/codex-graph/references/task-lifecycle.md#L137-L160: apply the same guard to the documented resolver pattern and add a fixture with an ID-less matching object before a valid thread row.
📍 Affects 2 files
skills/codex-graph/scripts/task_collection_harness.js#L59-L82(this comment)skills/codex-graph/references/task-lifecycle.md#L137-L160
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/codex-graph/scripts/task_collection_harness.js` around lines 59 - 82,
Require a resolved thread identifier in both resolver patterns: in
skills/codex-graph/scripts/task_collection_harness.js lines 59-82, return false
immediately when rowThreadId is absent, before clientThreadId or metadata
matching; in skills/codex-graph/references/task-lifecycle.md lines 137-160,
document the same guard and add a fixture where an ID-less matching metadata
object precedes a valid thread row.
jnyross
added a commit
that referenced
this pull request
Aug 9, 2026
- resume-path handle re-validation now mirrors the matcher contract: name/title/summary/preview + bracketed [<runTag>] + node id (finding 1) - bracket fixture keys the row on title so it discriminates the bracket rule against the pre-PR17 matcher (finding 4) - document the extraction-vs-matching id distinction instead of dropping id: open-source rows carry the thread id under id, so a matched row must stay extractable; matching still never falls back to bare id (finding 2, alternative branch per review) - adopt per-node resolveBounds(handle) over module-level constants; CLAIMED_THREAD_IDS stays one shared set — bounds per node, claims global (finding 3)
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_42cde175-6058-456d-a9c0-4f2c25ebf01b) |
- findExactThread reads name AND preview besides title/summary (OSS thread rows carry name?/preview, not title/summary — thread_data.rs); key list stays explicit, no bare id fallback - matcher requires the bracketed exact form [<runTag>] plus the handle's node id in the same field (per-node unique form), prefers name/title/ summary over preview, excludes the orchestrator's own/parent thread id, and excludes already-claimed thread ids from later resolutions - worktree targets are gated on isGitRepository === true from the project lookup; single writer degrades to a root-orchestrator write, otherwise fail fast with a named unresolved_risk (openai/codex#28204) - mirrored in task-lifecycle.md, code-mode-script-patterns.md, SKILL.md; enforced by validate_skill.py phrases and 9 new harness fixtures
Plain includes() let N1 bind N10 and N2 bind N2A when titles share a run tag. Use a word-boundary regex so concurrent pending workers stay distinct, and cover the prefix-collision cases in tests.
- resume-path handle re-validation now mirrors the matcher contract: name/title/summary/preview + bracketed [<runTag>] + node id (finding 1) - bracket fixture keys the row on title so it discriminates the bracket rule against the pre-PR17 matcher (finding 4) - document the extraction-vs-matching id distinction instead of dropping id: open-source rows carry the thread id under id, so a matched row must stay extractable; matching still never falls back to bare id (finding 2, alternative branch per review) - adopt per-node resolveBounds(handle) over module-level constants; CLAIMED_THREAD_IDS stays one shared set — bounds per node, claims global (finding 3)
jnyross
force-pushed
the
feat/matcher-keys-preflight
branch
from
August 9, 2026 20:46
8004d93 to
6b4f2d1
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_af05cd39-423e-4de9-8244-8c2e474a7d9d) |
jnyross
added a commit
that referenced
this pull request
Aug 9, 2026
PRs #14, #16, #17, #18, and #20 added manual Unreleased bullets; the release workflow prepended [0.4.5] through [0.5.4] for the same changes without folding them in. The changelog is generated from commit subjects at release time, so the manual section is pure duplication. Also document the convention in the README versioning section so it stops recurring.
2 of 3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements three ranked findings from the pending-clientThreadId research memo (
/tmp/cg-researcher-pending-v4.md, sections 2, 3, 6) plus finding 4 of the frozen Lisbon v5 static review (/tmp/orch/lean-v5-review.md).Changes
P0 — matcher reads
nameandpreview(memo §2.3, §6.1)Open-source thread list rows carry
name?andpreview, nottitle/summary(openai/codexcodex-rs/app-server-protocol/src/protocol/v2/thread_data.rs:id, sessionId, preview, name?, cwd, status, gitInfo…). The documentedfindExactThreadread onlytitle/summary, so a materialized, ready thread could go unmatched forever. The matcher now reads the explicit key listname/title/summary/preview. The key list stays explicit; there is deliberately still no bareidfallback (memo §6.7).P1 — git-toplevel preflight for worktree targets (memo §3, §6.2)
openai/codex#28204 diagnosis: worktree provisioning runs
git rev-parse --show-toplevelon the saved project root; on failure it aborts beforegit worktree add, writes no thread row, and the pending id never appears inlist_threads— the orchestrating agent gets no error signal at all. Generated scripts already fetchisGitRepositoryinresolveProjectand then ignore it. New contract rule:environment: { type: "worktree" }REQUIRESisGitRepository === true; otherwise degrade a single repository writer to a root-orchestrator write on the real checkout, or fail fast with a namedunresolved_riskbefore any create. This closes the silent #28204 class entirely; openai/codex#26861 confirms there is no resolve API to recover after the fact.P2 — matcher false-positive and cross-binding guards (memo §2.6, §6.5; lean-v5-review finding 4)
[<runTag>], never a bare substring.name/title/summaryare preferred overpreview; a preview-only hit is a second-pass fallback (a parent thread's preview can embed worker titles).Enforcement surfaces
references/task-lifecycle.md: contract prose,findExactThreadreference pattern, resolution loop with sharedCLAIMED_THREAD_IDS, §8 lifecycle checklist cases.references/code-mode-script-patterns.md+SKILL.md: mirrored rules.scripts/validate_skill.py: phrase checks for all three surfaces (name/title/summary/previewkey list, bracketed exact form, per-node unique, claimed thread id,isGitRepository === true, namedunresolved_risk, no bareidfallback).scripts/task_collection_harness.{js,test.js}:findExactThreadandpreflightWorktreeTargetwith 9 new fixtures — OSSname/previewrows resolve,clientThreadIdcorrelation regression, preview-only second-pass fallback, bracketed-form guard, parent-preview false positive rejected, two concurrent shared-tag workers resolve to their own threads (no cross-binding, no double-claim), worktree-without-git degrade/fail-closed/proceed.Verification
sh scripts/run_regressions.shgreen: validator PASS, graph_coherence selfcheck PASS, harness 23/23 (14 existing + 9 new), auto-release tests 4/4.No version bump;
testcases/untouched (PR #16 owns it).Evidence: memo §2/§3/§6 · openai/codex#26861 · openai/codex#28204 ·
thread_data.rsrow shape · frozen-lisbon-v4 terminal (workflow.js:133-181,188) · lean-v5-review finding 4.Note
Low Risk
Documentation, reference patterns, and test harness only—no production runtime path changes; risk is low aside from stricter generated-script contracts that may surface previously silent orchestration failures.
Overview
Tightens pending
clientThreadIdresolution and worktree targeting for generated Code Mode orchestration scripts.Thread-list matching now scans
name/title/summary/preview(not only Desktoptitle/summary), requires the bracketed tag[<runTag>]plus the node id in the same field, prefers strong keys over preview-only (second pass), excludes the orchestrator/parent and already-claimed thread ids, and keeps per-node resolve bounds with a sharedCLAIMED_THREAD_IDSset.Worktree preflight requires
isGitRepository === truebefore worktree creates; non-git roots either degrade a single writer to a root-orchestrator write or fail fast with a namedunresolved_riskinstead of silent provisioning failure.Docs (
SKILL.md,task-lifecycle.md,code-mode-script-patterns.md) mirror the contract;validate_skill.pyenforces the new phrases;task_collection_harness.jsexportsfindExactThreadandpreflightWorktreeTargetwith nine new tests. Changelog Unreleased line added.Reviewed by Cursor Bugbot for commit 6b4f2d1. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Documentation
Tests