Conversation
Plugin Version StatusVersions are auto-bumped in PRs. Manual bumps to higher versions are preserved.
|
…ization — consolidated into task-utils
- Wire active-task-guard.sh into PreToolUse hook array in hooks.json - Add .release-it.js following existing plugin pattern (common-sense) - Update sync-task-create.sh comment to mark payload schema verification as a follow-up task instead of an inline NOTE
…rebase - Remove self-contradicting migration note telling users to keep todo-sync installed (this PR deletes it) - Add "not yet implemented" callouts for config knobs in README and spec (no plugin_get_config calls exist in v0.1.0) - Mark 4 stub features as "Stub (no-op, v1.1)" in MVP scope table - Remove PreToolUse no-op from hooks.json (per-call overhead for zero benefit); mark deferred in spec - Fix session-start.sh comment claiming "restores in-progress tasks" when it only emits a static awareness message - Update spec frontmatter status from draft to in-progress
f3358a0 to
f348856
Compare
There was a problem hiding this comment.
👍 Round 3 — all round-2 blockers addressed; ready to merge
✅ Self-contradicting "keep todo-sync installed" note removed from README
✅ Inert config knobs now explicitly flagged "Not yet implemented" in README and spec
✅ MVP scope table marks 4 stub features as Stub (no-op, v1.1) instead of "In scope"
✅ PreToolUse no-op un-wired from hooks.json and marked deferred in spec
✅ session-start.sh header corrected — no longer claims "restores in-progress tasks"
✅ Spec frontmatter status: in-progress (was draft)
✅ Branch is MERGEABLE (was DIRTY)
🟢 Spec source attribution for Stop hook — still says "todo-plus-plus Stop hook" but todo-plus-plus had no Stop hook; non-blocking doc nit
🟢 stop-guard.sh header comment says "Commented out initially" but hook is wired; non-blocking doc nit
🖱️ Click to expand for full details
✅ Inert config knobs now explicitly flagged "Not yet implemented" in README and spec
✅ MVP scope table marks 4 stub features as
Stub (no-op, v1.1) instead of "In scope"✅
PreToolUse no-op un-wired from hooks.json and marked deferred in spec✅
session-start.sh header corrected — no longer claims "restores in-progress tasks"✅ Spec frontmatter
status: in-progress (was draft)✅ Branch is
MERGEABLE (was DIRTY)🟢 Spec source attribution for Stop hook — still says "todo-plus-plus Stop hook" but todo-plus-plus had no Stop hook; non-blocking doc nit
🟢
stop-guard.sh header comment says "Commented out initially" but hook is wired; non-blocking doc nitContext
f348856 resolved every blocking item from my round 2 review. This re-review verified the fixes against the shipped code at 6a31371 and confirmed merge-readiness. Most prior threads have been resolved; two minor doc-accuracy nits remain as non-blocking inline comments.
What was verified this round
git show main:plugins/todo-plus-plus/hooks/hooks.json— confirmed the deletedtodo-plus-plushook config registered onlyTaskCompletedandSessionStart(noStop), so the spec source attribution atspec.md:117is still inaccurate. Fix is a one-line wording change; flagged inline.git show HEAD:.claude/settings.json—todo-sync@ai-mktplremoved fromenabledPlugins.todo-plus-plus@ai-mktplandtask-parallelization@ai-mktplwere never in this file's enabled list (no removal needed in this repo's local settings)..claude-plugin/marketplace.json— three deleted plugins removed,task-utils@0.1.0added with correctsourceand merged keyword set.plugins/task-utils/lib/log.sh— symlink to../../../shared/lib/log.sh, matching the convention of the other 17 plugins per.claude/rules/shared-libs.md.plugins/task-utils/.release-it.js— present, mirrors the canonical pattern (extends: "../../.release-it.base.json"+@release-it/bumperforplugin.json).plugins/task-utils/.claude-plugin/plugin.json—"version": "0.1.0", appropriate for scaffold release.plugins/task-utils/hooks/hooks.json—PreToolUseremoved; remaining 5 entries (TaskCompleted, SessionStart, Stop, PostToolUse:TaskCreate, PostToolUse:TaskUpdate) are intentional per the spec's Hook table.plugins/task-utils/README.md—Configurationsection now leads with "Not yet implemented" disclaimer; "keep todo-sync installed" advice removed; feature list marks 3 hooks explicitly as(scaffold — no-op in this release).plugins/task-utils/docs/spec.md— frontmatterstatus: in-progress; MVP scope table marks 4 rows asStub (no-op, v1.1);PreToolUserow notes "script exists but is not wired."plugins/task-utils/hooks/scripts/session-start.sh— header comment no longer claims restore behavior; only emits the staticsystemMessageheredoc.grep -rn "plugin_get_config\|plugin_is_enabled" plugins/task-utils/— confirmed zero hits in scripts (the only mention is a self-disclosure inspec.md:161).
Remaining non-blocking items
- Spec source attribution (
spec.md:117) — still misattributes the Stop hook to todo-plus-plus. Inline comment with a one-linesuggestionblock. Doc accuracy only. stop-guard.sh:6stale comment — says "Commented out initially" but the hook is registered and runs as a no-op. Inline comment with asuggestionblock aligning the comment withspec.md:120's wording.
Both are pure documentation; the shipped behavior is correct.
Scoring rationale
- Code Quality (92%) — Up from 80% in round 2. All structural and accuracy concerns resolved. Migrated code (commit guard, session awareness, parallelization skill) is faithful and clean. Docked 8% for two stale-comment / source-attribution nits and the lingering
Stopno-op stub firing on session end (per-event cost is negligible, so this isn't worth blocking on). - Security (N/A) — Plugin scaffold; no new auth or trust surface.
- Simplicity (92%) — Up from 85%. README/spec/code now agree on what's implemented vs. deferred, which removes the confusion that drove the previous score. Six hook entry points where 3 are placeholders is still a structural choice the v1.1 work will validate.
- Confidence (93%) — Up from 88%. Verified each fix against the actual file in
HEAD, rangit show main:for deleted-file verification, confirmed symlink target viareadlink, and confirmed the auto-bumpgithub-appline inmarketplace.jsonis just a stale auto-bump artifact (no actual code change inplugins/github-app/). Cannot verify.tool_responsevs.tool_resultagainst a live Claude Code hook payload, but that uncertainty is now appropriately tracked in the script's TODO.
Auto-bump artifact note
plugins/github-app/.claude-plugin/plugin.json ships a 0.3.1 → 0.3.2 bump in this PR with no corresponding code changes. This is a stale auto-bump artifact from earlier in the branch's life, not author-introduced. Won't affect anything functionally; flagging for awareness in case future review tooling queries "why did github-app bump in a task-utils PR?"
What I couldn't verify
- Live Claude Code hook payload schema for
PostToolUse:TaskCreate/TaskUpdate(whether.tool_responseor.tool_resultis the canonical field). The script TODO comment atsync-task-create.sh:22already calls this out as a follow-up; acceptable for a stub. - That
TaskCompleted,TaskCreate,TaskUpdatematchers continue to fire in the current Claude Code version. They were active in todo-plus-plus historically.
Recommended follow-ups (non-blocking, for v1.1):
- File a tracked issue to verify hook payload schema (
.tool_responsevs.tool_result) against a livePostToolUseinvocation, and link it fromsync-task-create.sh:22andsync-task-update.sh:22. The two stubs use different shapes (.tool_result.taskIdvs.tool_input.id) so they will need a single source of truth before either provider can sync correctly. - Consider a smoke-test mise task (
mise run test:task-utils) that pipes minimal stdin JSON into each hook script and asserts exit codes — would catch shell regressions and protect against schema drift at provider-implementation time. - The "ALWAYS use TaskCreate on EVERY action request even for simple one-offs" framing in
task-management/SKILL.md:12is heavy — consider softening once the skill has had real-world usage. Carrying forward as a v1.1 judgment call rather than a blocker.
Footnotes
-
Workflow Run: https://github.com/nsheaps/ai-mktpl/actions/runs/24958799575/attempts/1 ↩
-
Shared libs convention:
.claude/rules/shared-libs.md↩ -
Hook output patterns:
.claude/rules/hook-output-patterns.md↩ -
Versioning rules:
.claude/rules/versioning.md↩
|
|
||
| ### 4. `Stop` (advisory) — In-Progress Task Warning | ||
|
|
||
| **Source**: todo-plus-plus `Stop` hook — **migrated from agent repo to plugin** |
There was a problem hiding this comment.
🟢 Source attribution still inaccurate (carry-forward from round 1)
todo-plus-plus never had a Stop hook. Verified against the deleted plugins/todo-plus-plus/hooks/hooks.json — only TaskCompleted and SessionStart were registered there.
The hook script header at plugins/task-utils/hooks/scripts/stop-guard.sh:5 already attributes correctly:
# Source: migrated from agent repo (was in .claude/rules as a hook, not a plugin hook).
Recommend matching the spec to that:
| **Source**: todo-plus-plus `Stop` hook — **migrated from agent repo to plugin** | |
| **Source**: Migrated from agent repo `.claude/rules`/hook configuration (not part of the deleted todo-plus-plus plugin) |
Non-blocking for this PR — pure documentation accuracy. Flagging because the same incorrect attribution would otherwise propagate into anyone reading this spec as the canonical history of the consolidation.
| # Triggered by Stop hook (advisory only — does not block) | ||
| # | ||
| # Source: migrated from agent repo (was in .claude/rules as a hook, not a plugin hook). | ||
| # Commented out initially pending validation in plugin context — see spec. |
There was a problem hiding this comment.
🟢 Stale header comment — script is wired in hooks.json, not commented out
The header still says "Commented out initially pending validation in plugin context" but hooks.json:28-39 registers this script on Stop with matcher * and a 10s timeout. The script is firing on every session-stop, just no-op'ing.
Recommend aligning the comment with the spec's wording (spec.md:120 says "Registered as a no-op stub, full logic deferred to v1.1"):
| # Commented out initially pending validation in plugin context — see spec. | |
| # Source: migrated from agent repo (was in .claude/rules as a hook, not a plugin hook). | |
| # Registered as a no-op stub — full logic deferred to v1.1 pending validation in plugin context. |
Non-blocking, doc accuracy only. The Stop hook firing once per session is negligible overhead so the per-call concern that drove un-wiring PreToolUse doesn't apply here.
Summary
Consolidates
todo-plus-plus,todo-sync, andtask-parallelizationinto a singletask-utilsplugin with provider-based task sync architecture.Discord thread: https://discord.com/channels/1490863845252665415/1497254984696205445
Milestone: 999 - plugin consolidation
Changes
plugins/task-utils/docs/spec.md— provider architecture, 7 hooks, MVP scopeplugins/todo-plus-plus/,plugins/todo-sync/,plugins/task-parallelization/plugins/task-utils/with full scaffold: hooks, skills, lib, READMERelated issues
Closes #319 Closes #320
Related: #330 #370 #138 #64 #65
Test plan
Co-Authored-By: Jack Oat <jack-nsheaps[bot]@users.noreply.github.com>