ADR-314: Remove todo-log/TodoWrite-mirroring plumbing - #85
Conversation
jodavis-claude
left a comment
jodavis-claude
left a comment
There was a problem hiding this comment.
Review summary — ADR-314
Reviewed the sole commit unique to this PR (34bac71) against the task brief's exit criteria and CONTRIBUTING.md.
Exit criteria — all met:
- Todo-log-tailing /
TodoWrite-mirroring plumbing fully removed fromworkflow-orchestrate/SKILL.md(steps renumbered 1→3 cleanly,--todo-logdropped from thespawn_agenttemplate,TaskStopmentions removed) andworkflow-worker/SKILL.md(old step 1 removed,--todo-logdropped fromargument-hint/Arguments, steps renumbered). get_todo_log_path.pyandappend_todo_log.pydeleted; confirmed viagit show <parent>:<dir>that neither had a dedicated test file before deletion, so nothing orphaned.## Task trackingsection removed identically fromdeveloper.md,researcher.md,reviewer.md; confirmeddebugger.md,script-runner.md, and thetdd-*.mdfiles never had it, so correctly left untouched.TodoWritetool grant intentionally left in each frontmattertools:list per the brief's recommendation — reasonable, since the exit criteria scope only the instruction text.watch-pr/SKILL.mdupdated in the same commit to drop itsget_todo_log_path.pyreference and both--todo-logspawn arguments — correctly treated as in-scope despite not being literally named in the exit-criteria checklist; the file itself had explicitly deferred this cleanup to ADR-314, and leaving it unedited would have left a live reference to a deleted script.- Repo-wide grep (
todo_log|--todo-log|TaskStop|get_todo_log_path|append_todo_log|## Task tracking) confirms zero remaining references anywhere underplugins/.
No Priority 1-4 issues found. This is a pure documentation/plumbing removal with no new source behavior, so correctness/security/performance dimensions mostly don't apply; the one dimension that does apply (documentation) is satisfied — the diff matches _spec_ConcurrentDevelopment.md's design decision exactly.
Regression check: ran test_dev_team.py (111 tests) — 1 test (test_uses_state_dir_env) timed out on the full-suite run but passed cleanly in isolation (26.77s); this looks like sandbox/CI resource flakiness unrelated to this diff (the test only exercises --print-context-path, a code path this PR never touches), not a regression.
Process note (non-blocking): this PR's declared base (dev/claude/ADR-313) is currently behind the actual ancestry of this branch by four already-committed ADR-313 commits (58f47a5, 9958a69, 8337d60, 0e7cb28) that haven't been pushed to origin/dev/claude/ADR-313 yet (PR #84 is still at ee1bc99). That makes gh pr diff/the GitHub Files-changed view show those four unrelated commits' diffs mixed in with this PR's own single commit. It's a base-branch sync artifact, not a defect in this diff — the actual ADR-314 commit (34bac71) is clean, isolated, and reviewed above. It should resolve itself once PR #84's branch is pushed/merged.
No inline comments needed — approving.
Removes the todo-log-tailing / TodoWrite-mirroring mechanism from workflow-orchestrate and workflow-worker: it never surfaced well even for a single task's pipeline, and had no clean extension to the concurrent multi-task orchestrator this spec introduces. - workflow-orchestrate/SKILL.md: drop the todo-log-path step and the Monitor/TaskStop-based tailing step, renumber remaining steps, and drop --todo-log from the spawn_agent prompt template. - Delete workflow-orchestrate/scripts/get_todo_log_path.py (no dedicated test file existed). - workflow-worker/SKILL.md: drop the log-redirection step and --todo-log argument, renumber remaining steps. - Delete workflow-worker/scripts/append_todo_log.py (no dedicated test file existed). - developer.md, researcher.md, reviewer.md: remove the "Task tracking" instruction to mirror updates via TodoWrite through the log file. The TodoWrite tool grant in each file's frontmatter is left in place since the exit criteria scope only the instruction text. - watch-pr/SKILL.md: this ADR-313-added skill still depended on the exact plumbing being removed here (its own text explicitly deferred this cleanup to ADR-314); updated to drop get_todo_log_path.py from its script list, remove its own todo-log-path computation step, and drop --todo-log from both nested workflow-worker spawn prompts. Verified via repo-wide grep that no todo_log/TaskStop/--todo-log/ get_todo_log_path/append_todo_log references remain, and the existing workflow-orchestrate test suite (test_dev_team.py, 111 tests) still passes unchanged.
34bac71 to
8e48157
Compare
build-and-test: Python test resultsStatus: ✅ Passed Test log |
3 participants
Work item
ADR-314— remove the todo-log-tailing /TodoWrite-mirroring plumbing fromworkflow-orchestrateandworkflow-worker(and the per-agent "maintain a to-do list" instruction from the three agent role files), because it doesn't surface well even for a single task's pipeline today and has no clean extension to the concurrent multi-task orchestrator introduced by_spec_ConcurrentDevelopment.md.Changes
plugins/dev-team/skills/workflow-orchestrate/SKILL.md— removed the todo-log-path computation half of old step 1, removed old step 2 ("Start tailing the todo log" — theMonitor/tail -F/TodoWrite-mirroring block) in full, renumbered old steps 3/3a/3b/3c/4 to 2/2a/2b/2c/3, dropped--todo-log <todo_log>from thespawn_agentprompt template, and dropped the threeTaskStopmentions tied to the removed monitor.plugins/dev-team/skills/workflow-orchestrate/scripts/get_todo_log_path.py— deleted (no dedicated test file existed for it).plugins/dev-team/skills/workflow-worker/SKILL.md— removed old step 1 ("Redirect todo list tracking to the log file") in full, dropped--todo-log <path>fromargument-hintand theArgumentslist, renumbered old steps 2/3/4 to 1/2/3, and reworded the new step 1 to drop the todo-log-redirection phrase.plugins/dev-team/skills/workflow-worker/scripts/append_todo_log.py— deleted (no dedicated test file existed for it).plugins/dev-team/agents/developer.md,plugins/dev-team/agents/researcher.md,plugins/dev-team/agents/reviewer.md— removed the identical## Task trackingsection (the "useTodoWrite... via the log file" instruction) from all three.plugins/dev-team/skills/watch-pr/SKILL.md— updated to keep it consistent with the removal: droppedget_todo_log_path.pyfrom the borrowed-scripts list, removed its own "compute this session's own todo log path" step, dropped--todo-log <todo_log>from both nestedworkflow-workerspawn prompts, and updated the## Skillssection'sworkflow-workerargument description accordingly.Design decisions
TodoWritetool grant in each agent file's frontmattertools:list in place — the exit criteria only call for removing the instruction to use it for log-mirroring, not the tool grant itself.watch-pr/SKILL.mdas in scope despite not being literally named in the spec's exit-criteria checklist: the file's own text explicitly noted its--todo-logplumbing removal "is tracked separately as ADR-314 and is not pre-empted here," and ADR-313's commit message said the same. Leaving it unedited would have left a live reference to a deleted script and an argumentworkflow-workerno longer accepts.dev/claude/ADR-313(PR ADR-313: Add dev-team:watch-pr post-hand-off PR monitor and /watch-pr command #84, still open) since this branch was created on top of that in-flight work, per the dependency-aware base-branch convention.Testing completed
plugins/dev-team/skills/workflow-orchestrate/scripts/test_dev_team.pysuite (111 tests) as a regression check — all pass unchanged.plugins/tree fortodo_log/--todo-log/TaskStop/get_todo_log_path/append_todo_log/## Task trackingto confirm zero remaining references outside this diff.