Design: a task closes on merge or cancel; reject with feedback requeues - #62
Merged
Merged
Conversation
L1 listed "rejected" as a way a task closes, which contradicted step 8 (Reject with feedback creates the next revision and requeues the task). The F1 contract already followed step 8; drop its interim note now that the design agrees. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The lifecycle rules are consistently reconciled with no unresolved issues.
Review effort: Lite
Findings: None
What changed in this PR
This documentation-only PR reconciles task lifecycle semantics: merge or cancellation closes a task, while rejection requeues it and preserves feedback.
Changes:
- Updates L1 closure and feedback rules.
- Aligns the F1 runner contract.
| File | Description |
|---|---|
docs/implementation/runner-lifecycle.md |
Aligns F1 with the reconciled design. |
docs/designs/codeboost-plan-indexed-review.md |
Clarifies closure and rejection-feedback retention. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
mchwang
added a commit
that referenced
this pull request
Sep 30, 2026
* F1d: startup recovery and the single-runner lock acquireRunnerLock: owner-only parent directory, no-follow identify or create, refuse hard links and symlinks, then an OS lock on ~/.codeboost/locks/<dev>-<ino>.runner-lock (SQLite exclusive locking) that the OS releases on exit or crash; verify() after the Store opens. The CLI takes the lock before starting and releases it after shutdown. recoverStartup: stop leftover preparation groups, D recovery (injected until transaction (settlement precedence, cancel and time-limit guards, requeue claim), storage removal, attempt-directory sweep, merged-task repair, and a fail-closed stop for unrecorded preparation. Store: runner owner token, preparation and allocation markers, requeue claim at admission, and releasePreparation with an open-files check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep held runner locks referenced so GC cannot release them A caller that dropped the returned RunnerLock let its SQLite connection be garbage-collected, which released the OS lock while the runner was still alive. Found by a CI failure of the cross-process lock test on #60. The new test drops the lock and forces GC in a child, and fails without the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Start the merge-repair recovery test from a task in review main (#62, #53) lets only a task in review begin a merge attempt, so the test puts its queued task in review before it records the merge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep recovery fail-closed: unowned directories, unreadable start times, symlinked roots, unstopped exports - The attempt-directory sweep skips every unowned preparation, including one an earlier startup already finalized. Before, the second startup deleted its directory while step 7 still blocked on it. - ps runs with LC_ALL=C, and a start time Date.parse cannot read counts as ours, so a live preparation group is still stopped. - hostOpenFiles compares against the resolved directory. On Linux a symlink in the runner root made an in-use directory look unused. - A timed-out export is awaited, for up to the grace period, before its task storage is removed. An export that does not stop blocks startup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Follow-up from #52 and the F1 contract (#49).
Problem
The design's learning section L1 said "A task closes: it is merged, cancelled, or rejected." Step 8 of the same design says "Reject with feedback" creates the next plan revision and puts the task back in the queue. These contradict each other about when a task closes, and so about when lane J may distill its feedback into lessons.
The F1 runner contract (
docs/implementation/runner-lifecycle.md) followed step 8, and carried an interim note saying it governed until the design was reconciled.Change
No behaviour changes. This matches what F1 implements: the closed statuses are
mergedandcancelled, andtask-closedis written only for a confirmed merge or a cancel task.Validation
Docs only;
git diff --checkis clean. I searched the design for any other place that ties "rejected" to task closure or lessons, and found none.Readiness report
49603b6testpassed on49603b649603b6): approval recommended, no findings🤖 Generated with Claude Code