fix: separate dependency waits from needs-spec in pickup - #27
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: QUIET Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe issue instructions now require a blocked-by link when work depends on another open issue. Orca worker guidance distinguishes dependency waits from missing user decisions. Triage and idle pickup guidance address linked open dependencies and prose-only prerequisites. Validation scenarios cover issue filing, worker handling, and readiness after linking a dependency. Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to No demonstrated issue blocks merging. The partial-work handoff has not been verified against Orca’s worktree behavior. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 2 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 4 | ❓ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The reviewed skill guidance implements the main Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f739b75890
ℹ️ 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".
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
skills/origin89-orca/references/unattended-run.md-240-241 (1)
240-241: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPreserve partial work during dependency waits.
The dependency-wait path records the partial-work branch, then unlinks the preserved worktree. The next pickup uses
new-top-level, but its spec does not include that branch. A later worker can therefore start without the partial changes.Record the branch in the pickup handoff and require the next worker to reuse the preserved worktree or use that branch before unlinking it.
Suggested fix
- authority in the spec. A parent with independent sub-issues coordinates them + authority in the spec. If the issue records a branch holding partial work, + include that branch in the pickup handoff and reuse it or the preserved + worktree. A parent with independent sub-issues coordinates them
🧹 Nitpick comments (1)
docs/skill-validation.md (1)
74-74: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a combined blocker scenario.
unattended-run.mdrequires both blocker paths when an open dependency and an unresolved decision occur together. The current scenarios cover these paths separately, so a worker can handle only one blocker without the validation scenarios detecting it.Suggested scenario
+| Orca | A pickup worker finds an open dependency and an unresolved decision | Link the blocker, comment the evidence, ask the decision questions, remove agent-working and agent-ready, and add needs-spec |
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: QUIET
Plan: Advanced
Run ID: 9e737b0a-25e9-4bfa-b701-48f15dd49116
📒 Files selected for processing (3)
docs/skill-validation.mdskills/origin89-orca/references/unattended-run.mdskills/origin89-working/SKILL.md
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
|
Decision (lemarier, Roger Ask |
Change
Idle pickup had one blocker path: comment questions, add
needs-spec, and dropagent-ready. A worker waiting on another open issue took that path too, so the issue needed a human to relabel it after the blocker closed, and a dependency written only in prose let the dispatcher claim an issue it could not finish (origin89hq/firmware#6 waiting on #12).Workers now separate the two cases. A dependency on an open issue gets a blocked-by link, an evidence comment, and
agent-workingremoved whileagent-readystays, and the coordinator unlinks the issue from the preserved worktree, so pickup takes the issue again once the blocker closes. A missing user decision keeps theneeds-specpath, and both apply when both are true. The coordinator reads issue comments before claiming and skips, and reports, issues naming an open prerequisite without a link. The precheck excludes issues with an open blocker (issue_dependencies_summary.blocked_by), so a keptagent-readydoes not start an idle run every tick. The final report lists issues sent toneeds-spec, linked to a blocker, or skipped, and the daily hygiene check flags prose-only dependencies.Filing gets the same rule, so new issues do not repeat the gap:
origin89-workingnow requires a blocked-by link when an issue cannot start until another open issue lands, and whenever a later comment reveals one. Needs-spec triage links a prose dependency and may addagent-readyto an issue whose only remaining blocker is that link, since pickup skips it until the blocker closes; previously triage withheldagent-ready, which left the same manual relabel step.The issue's point about pickup workers having no coordinator was already addressed by #16; this change only states that the issue comment is the record that outlives the Run.
Closes #15
Validation
just checkpasses (32 tests). Confirmed that the REST issues list returnsissue_dependencies_summary.blocked_byper issue (firmware#11 reports 1). Open Code Review excludes Markdown, so it reviewed no files; the prose was reviewed by hand. Two scenarios were added todocs/skill-validation.md; no live pickup run has exercised the new rules yet.