Safely release completed Hermes task worktrees - #79
Conversation
![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: ba98404d9f
ℹ️ 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".
| if (requiresHermesWorkspaceRelease(currentTask.workflow_meta_json)) { | ||
| appendTaskLog(taskId, "system", "Review hold: waiting for Hermes result storage and workspace release receipt"); | ||
| return; |
There was a problem hiding this comment.
Once a Hermes task has successfully posted /release-worktree, the endpoint only appends a receipt log and leaves the Hermes marker unchanged. This unconditional check therefore returns on every later finishReview invocation as well, and the release endpoint does not invoke finishReview itself, so a successfully released task remains in review indefinitely unless an external caller bypasses the normal finalization workflow. Check for the matching persisted receipt before holding, and trigger or permit finalization after it is written.
Useful? React with 👍 / 👎.
| const output = requireGit(worktreePath, ["status", "--porcelain"]); | ||
| if (!output) return []; | ||
| return output | ||
| .split("\n") | ||
| .map((line) => line.trimEnd()) | ||
| .filter(Boolean) | ||
| .filter((line) => { | ||
| const candidate = line.slice(3).trim().split(" -> ").pop() ?? ""; | ||
| return candidate !== ".claude/skills" && !candidate.startsWith(".claude/skills/"); |
There was a problem hiding this comment.
When installed skills are propagated by lifecycle.ts as an untracked .claude/skills symlink and the project does not ignore or track .claude, default git status --porcelain reports the collapsed entry ?? .claude/, not .claude/skills. The filter below consequently retains it, and every otherwise-clean Hermes release is rejected as workspace_dirty. Use an untracked-files mode that emits individual paths or explicitly recognize the generated directory without hiding unrelated .claude changes.
Useful? React with 👍 / 👎.
| `verify ${proof.savedRef} ${proof.savedHead}`, | ||
| `delete refs/heads/${proof.branchName} ${proof.worktreeHead}`, |
There was a problem hiding this comment.
If another task advances savedRef after this function removes the worktree but before this transaction executes, the exact-head verification fails after the destructive removal. Retrying cannot recover: reconciliation rejects the changed saved head, while submitting a refreshed proof has no matching prepared marker and attempts to open the now-missing worktree. This leaves the Hermes task permanently unable to obtain a receipt even when the new saved ref still contains worktreeHead; reconciliation should revalidate ancestry against the current saved head or otherwise support a refreshed proof for the already-removed worktree.
Useful? React with 👍 / 👎.
…oster TICKET-47: show Notion-linked development roster
…oster TICKET-47: preserve linked roster across restarts
…ktree-recovery TICKET-49: Notion準拠のAI組織へ統一
…epartments TICKET-50: Notion部門カタログ7件を表示
…nt-restart-order fix: preserve Notion department order on restart
1 participant
Summary\n- add a task-scoped, non-force worktree release endpoint for Hermes-managed review tasks\n- require exact execution identity, clean Git state, persistent saved-ref reachability, CSRF/auth, and durable task-log receipts\n- atomically verify the saved ref and delete only the disposable task branch\n- keep native review auto-finalization on hold until Hermes completes the release/read-back contract\n\n## Safety\n- validation failures delete nothing\n- retries converge through prepared/receipt logs\n- prior execution receipts cannot finalize a new execution\n- explicit discard cannot be reconciled into a false receipt\n\n## Verification\n- API: 217 tests passed\n- Web: 76 tests passed\n- TypeScript/build passed\n- OpenAPI check passed\n- ESLint passed with pre-existing warnings only\n- independent review: no Critical/Important findings