Repository navigation
fix: per-node environment + pending clientThreadId resolution - #14
Conversation
Lisbon dogfood v4 blocked with N2A–N2C stuck in pending_setup after create_thread returned only clientThreadId. Generation applied worktree to every node because one writer existed; skill text had allowed a graph-level reading. Research workers need local; writers alone need worktree. Also: pending match must not require projectId while setup loads, may correlate by clientThreadId, and worktree needs a longer provisioning-scale resolve bound. Refs: frozen-lisbon-v4 terminal ee3f60e0… / CRITIQUE-v4.
📝 WalkthroughWalkthroughThe PR documents per-node task environments, strengthens pending task setup correlation, updates skill validation requirements, and adds an Unreleased changelog entry. ChangesTask lifecycle guidance
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
Handle.environment stores the spawn target object ({ type }), not the
string "worktree", so the pending-setup ternary never selected
provisioning-scale polling. Document the field on the handle, copy it
from the node in startRequiredNode, and key bounds on environment.type.
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/validate_skill.py`:
- Around line 178-179: Extend the validation checks in validate_skill.py for
each environment-contract mapping to require its complete clause, not just
generic phrases such as “per node” or “provisioning-scale”. Require read-only
nodes to specify local execution, repository-writing nodes to specify worktrees,
and local starts to specify chat-scale polling, covering the related checks near
the existing mapping phrase lists.
🪄 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: 47730d7c-85e0-4055-90e6-da3dfd2ca9d1
📒 Files selected for processing (5)
CHANGELOG.mdskills/codex-graph/SKILL.mdskills/codex-graph/references/code-mode-script-patterns.mdskills/codex-graph/references/task-lifecycle.mdskills/codex-graph/scripts/validate_skill.py
| "per node", | ||
| "provisioning-scale", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require the validator to check the complete environment contract.
The new checks only require generic phrases such as per node and provisioning-scale. They do not require local execution for read-only nodes, worktrees for repository-writing nodes, or chat-scale polling for local starts. A later edit can remove these mappings and still pass validation. Add checks for each required mapping clause.
Also applies to: 211-215, 256-258
🤖 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/validate_skill.py` around lines 178 - 179, Extend
the validation checks in validate_skill.py for each environment-contract mapping
to require its complete clause, not just generic phrases such as “per node” or
“provisioning-scale”. Require read-only nodes to specify local execution,
repository-writing nodes to specify worktrees, and local starts to specify
chat-scale polling, covering the related checks near the existing mapping phrase
lists.
…es (#16) * feat: dynamic-workflow golden test cases from real Grok/Claude orchestration shapes Turn curated Grok Rhai workflows (destination/hotel discovery funnels, fixed-lane research DAGs) and Claude multi-agent patterns (disjoint worktree implementers, review-scorer panels) into a maintainable codex-graph test-case pack: - docs/dynamic-workflow-testcase-catalog.md: source survey, shape analysis, tier/contract mapping, prioritized shortlist - skills/codex-graph/testcases/: catalog.json + six case bundles (GOAL.md, EXPECTATIONS.md with fenced machine contract, topology.hint.mmd passing graph_coherence.py) - testcases/harness/: offline expectations loader + static workflow.js checker (per-node env #14, read-first collection #13, node-local prompt scoping #12, result normalization #11) with CLI and 15 tests - task_collection_harness.test.js: three pattern-derived fixtures (atomic screen fan-out terminal-at-first-read, screen-then-POV staged cardinality with root-owned fail-open, adversarial dual validators failing closed on malformed verdicts) - both regression runners execute the new suite and lint the hints Goals are synthetic and anonymized; no live Desktop run required. * fix: harden expectations checker per PR #16 review - collection:read-first / read-after-wait judge await-adjacent call sites only; parameter lists, bindings, and comments can no longer flip the verdict (v3 param-swap now fails; wrapper-based correct collectors are no longer false-failed) - setup:pending-resolution requires bounded-resolution evidence (attempts constant or task-list poll) alongside the clientThreadId identifier; mentions-only scripts fail - checkWorkflowText refuses vacuous ok on an empty contract - atomic-screen-fanout goal genericized (composition/origin/month) - disjoint-writers-worktree contract declares its drawn repair path (single_repair, R1, V2) - four new negative/positive controls in testcases.test.js; README semantics updated to match the strengthened checks * fix: fail collection checks when read/wait tools are absent Await-adjacent matching correctly skips wrapper collectors, but treated scripts with no read/wait bindings the same way and green-lit #13. Require tool bindings when there are no await-adjacent call sites. --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
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.
Summary
Lisbon family dogfood v4 (skill 0.4.4 / #13) blocked before collection: all three research workers stayed
pending_setupwithclientThreadIdonly. Frozen graph appliedenvironment: worktreeto every create_thread, including read-only research nodes. v3 usedlocalfor research and got readythreadIds.Root cause was skill guidance that could be read as graph-level (“worktree when the graph can write”) rather than per-node.
Changes
local; writers →worktree; never global worktree because one writer existsclientThreadIdwhen listed; allow run-tag match whileprojectIdabsent during loading; chat-scale vs provisioning-scale boundsEvidence
mac-vm-harness--codexgraph-testcommit2f51adf/candidates/frozen-lisbon-v4/blocked,Required research starts failed: N2A, N2B, N2C, handles preservedTest plan
python3 skills/codex-graph/scripts/validate_skill.pynode --test skills/codex-graph/scripts/task_collection_harness.test.js(14/14)local, fix: collect handoffs when workers finish before wait sees them #13 collection exercised, realLISBON-FAMILY-DAY.mdNote
Low Risk
Changes are limited to skill documentation, reference patterns, and validator string checks—no runtime orchestration or application code.
Overview
Fixes Lisbon dogfood v4 guidance where read-only research workers inherited a graph-wide
worktreeenvironment and stayedpending_setupwith onlyclientThreadId.Per-node environments: read-only nodes use
local; nodes that write repo files useworktreewith disjoint scopes—never worktree on every create because one writer exists. Final project-root artifacts should be written from the root orchestrator (or a single integration owner), not only inside a disposable worker worktree.Pending setup resolution is tightened: poll with chat-scale bounds for local starts and provisioning-scale bounds for worktree; match run tags in
title/summarywithout requiringprojectIdon early list rows; correlate byclientThreadIdwhen listed. Example handle shape andfindExactThreadlogic intask-lifecycle.mdreflect this.validate_skill.pygains required phrases so this guidance cannot regress silently; CHANGELOG records the unreleased fix.Reviewed by Cursor Bugbot for commit 345a22a. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Documentation