fix(goals): Start Cycle stayed disabled with every task committed #15

Merged
forgejo-admin merged 1 commit from fix/start-cycle-disabled into main 2026-07-31 01:05:54 +00:00

Reported on production: every task on /goals shows committed, but Start Cycle stays greyed out.

Root cause

readyToStartCycle compared list lengths — totalTasksCount === committedTasks.length. A single duplicate entry in committedTasks inflates the length, so the equality never holds again, while every card still renders as committed (a card's committed state is a find-by-id, which a duplicate satisfies). All green, button dead.

Duplicates were easy to produce. handleToggleCommitted decided add-vs-remove from the render-time committedTasks but wrote through a functional updater, so two toggles inside one render both saw "not committed yet" and both appended — a double-click on Commit all, or Commit all over a template task that getStatistics lists under two categories (possible once a task's category changes between months). handleCommitAll compounded it by toggling task-by-task, which also un-committed anything already committed.

Changes

  • readiness is a coverage check — every task id present in the committed set — instead of a count match
  • the commit toggle decides against prev, not a stale render-time copy
  • Commit all sets the full deduped list, idempotently
  • getTaskIds returns record keys, which is what the cards actually toggle with
  • bootstrap.spec.ts clicks Commit all twice and still expects Start Cycle enabled — that second click is the regression guard

Verified locally: tsc -b --noEmit clean, 52 unit tests pass. Patch changeset included.

🤖 Generated with Claude Code

Reported on production: every task on /goals shows committed, but **Start Cycle** stays greyed out. ## Root cause `readyToStartCycle` compared list lengths — `totalTasksCount === committedTasks.length`. A single duplicate entry in `committedTasks` inflates the length, so the equality never holds again, while every card still *renders* as committed (a card's committed state is a `find`-by-id, which a duplicate satisfies). All green, button dead. Duplicates were easy to produce. `handleToggleCommitted` decided add-vs-remove from the render-time `committedTasks` but wrote through a functional updater, so two toggles inside one render both saw "not committed yet" and both appended — a double-click on **Commit all**, or **Commit all** over a template task that `getStatistics` lists under two categories (possible once a task's category changes between months). `handleCommitAll` compounded it by toggling task-by-task, which also *un*-committed anything already committed. ## Changes - readiness is a coverage check — every task id present in the committed set — instead of a count match - the commit toggle decides against `prev`, not a stale render-time copy - **Commit all** sets the full deduped list, idempotently - `getTaskIds` returns record keys, which is what the cards actually toggle with - `bootstrap.spec.ts` clicks **Commit all** twice and still expects Start Cycle enabled — that second click is the regression guard Verified locally: `tsc -b --noEmit` clean, 52 unit tests pass. Patch changeset included. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(goals): Start Cycle stayed disabled with every task committed
All checks were successful
pr-validate / validate (pull_request) Successful in 20s
release / version (pull_request) Successful in 8s
release / e2e (pull_request) Successful in 2m33s
release / publish (pull_request) Has been skipped
b61b1b47e1
The readiness check compared list lengths (totalTasksCount === committedTasks.length),
so a single duplicate entry in committedTasks wedged the button off — while every card
still rendered committed, since a card's committed look is a find-by-id that a duplicate
satisfies.

Duplicates were easy to create: handleToggleCommitted decided add-vs-remove from the
render-time committedTasks but wrote through a functional updater, so two toggles within
one render (a double-click on "Commit all", or "Commit all" over a template task listed
under two categories) both saw "not committed" and both appended. handleCommitAll made it
worse by toggling task-by-task, un-committing anything already committed.

- readiness is now a coverage check: every task id present in the committed set
- the toggle decides against prev, not a stale copy
- "Commit all" sets the full deduped list, idempotently
- getTaskIds returns record keys, which is what the cards actually toggle with

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign in to join this conversation.
No description provided.