Maniacs_Mansion/MealQuest
11
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Feature 14 integration fix: revalidate the draft pool, refuse duplicate week dinners, refresh the Quest log #122

Merged
approver_agent merged 1 commits from task/feature14-integration-fix_857e4c33-e250-46ec-b5ea-7dd4f3b719d9 into feature/ca2a118d-0beb-49f9-b2ef-d5ed0ac1fa2a-weekly-plan-builder 2026-10-07 16:30:04 -04:00
Conversation 1 Commits 1 Files Changed 8
+168 -22
orchestrator-agent commented 2026-10-07 16:19:08 -04:00
Member
Copy Link
Copy Source

Task

Maestro task 857e4c33-e250-46ec-b5ea-7dd4f3b719d9 — Feature 14 integration fix, from the independent feature-level review of Feature 14 (feature PR #121).

Findings fixed

  • F14-01 (HIGH), internal/plan/draft.go storeDraft: the candidate pool was read before the AI call and the save transaction checked only the week plan's revision, so a dinner archived or moved quest_log -> tried while the AI ran could still be saved, and two workers drafting different weeks could each select the same Quest log dinner before either plan was saved. Now the save transaction re-runs the same ListDraftPool eligibility rule the AI's pool came from; a dinner that left the pool, or changed kind (which would make is_new stale), fails the job with the new typed reason pool_changed and the transaction rolls back, writing no partial plan.
  • F14-02 (MEDIUM), internal/plan/plan.go Swap: a swap now reads the week's days inside the same transaction and refuses a dinner already on another day of that week with the new 409 dinner_already_planned. Swapping a day to the dinner it already has still works. No migration or unique constraint was added.
  • F14-03 (MEDIUM), web/src/plan/PlanPage.tsx: a finished draft now invalidates the Quest log query, as the swap path already did, so newly planned Quest log dinners stop being listed as available in the mounted tablet strip and desktop panel.

Tests (each verified to fail against the feature head 9556139)

  • TestDraftFailsWhenPoolDinnerLeavesWhileDrafting (archived and tried cases) — typed retryable failure, no plan written.
  • TestDraftsOfTwoWeeksDoNotShareAQuestLogDinner — two real workers drafting different weeks; the second is refused.
  • TestSwapRefusals / TestPutPlanDay — ErrDinnerAlreadyPlanned and HTTP 409, plan unchanged.
  • Web: "reads the Quest log again once a draft finishes", run at 768, 1180 and 1440 px.

The developer verified all new tests fail against the pre-fix logic by disabling each new check in turn.

Reviewer attention

Three existing Go tests (TestStartByAcrossDaylightSavingTimeEnd, TestDraftFailsWhenPlanSwappedWhileDrafting, TestSwapStatusTransitions) swapped in a dinner that was already on the week, so the new duplicate rule broke them; they now swap in a newly created dinner and are claimed to still test the same behaviour. Please confirm they were not weakened.

Evidence (orchestrator-run gate at b3b342ef55)

  • gofmt -l . — empty; go vet ./... — clean
  • go test -race -count=1 ./... — all packages ok (server 97.1s, suggest 24.7s)
  • CGO_ENABLED=0 go build ./... — ok
  • make sqlc-check SQLC=$(go env GOPATH)/bin/sqlc — no drift
  • cd web && npm run typecheck && npm test && npm run build — PASS, 45 files / 764 tests

Docs: docs/ARCHITECTURE.md records the new pool_changed reason and the 409 dinner_already_planned.

## Task Maestro task `857e4c33-e250-46ec-b5ea-7dd4f3b719d9` — Feature 14 integration fix, from the independent feature-level review of Feature 14 (feature PR #121). ## Findings fixed - **F14-01 (HIGH), `internal/plan/draft.go` `storeDraft`:** the candidate pool was read before the AI call and the save transaction checked only the week plan's revision, so a dinner archived or moved `quest_log -> tried` while the AI ran could still be saved, and two workers drafting different weeks could each select the same Quest log dinner before either plan was saved. Now the save transaction re-runs the same `ListDraftPool` eligibility rule the AI's pool came from; a dinner that left the pool, or changed kind (which would make `is_new` stale), fails the job with the new typed reason `pool_changed` and the transaction rolls back, writing no partial plan. - **F14-02 (MEDIUM), `internal/plan/plan.go` `Swap`:** a swap now reads the week's days inside the same transaction and refuses a dinner already on another day of that week with the new 409 `dinner_already_planned`. Swapping a day to the dinner it already has still works. No migration or unique constraint was added. - **F14-03 (MEDIUM), `web/src/plan/PlanPage.tsx`:** a finished draft now invalidates the Quest log query, as the swap path already did, so newly planned Quest log dinners stop being listed as available in the mounted tablet strip and desktop panel. ## Tests (each verified to fail against the feature head `9556139`) - `TestDraftFailsWhenPoolDinnerLeavesWhileDrafting` (archived and tried cases) — typed retryable failure, no plan written. - `TestDraftsOfTwoWeeksDoNotShareAQuestLogDinner` — two real workers drafting different weeks; the second is refused. - `TestSwapRefusals` / `TestPutPlanDay` — `ErrDinnerAlreadyPlanned` and HTTP 409, plan unchanged. - Web: "reads the Quest log again once a draft finishes", run at 768, 1180 and 1440 px. The developer verified all new tests fail against the pre-fix logic by disabling each new check in turn. ## Reviewer attention Three existing Go tests (`TestStartByAcrossDaylightSavingTimeEnd`, `TestDraftFailsWhenPlanSwappedWhileDrafting`, `TestSwapStatusTransitions`) swapped in a dinner that was already on the week, so the new duplicate rule broke them; they now swap in a newly created dinner and are claimed to still test the same behaviour. Please confirm they were not weakened. ## Evidence (orchestrator-run gate at b3b342ef55a77c937f859be36a28af0ec388bc56) - `gofmt -l .` — empty; `go vet ./...` — clean - `go test -race -count=1 ./...` — all packages ok (server 97.1s, suggest 24.7s) - `CGO_ENABLED=0 go build ./...` — ok - `make sqlc-check SQLC=$(go env GOPATH)/bin/sqlc` — no drift - `cd web && npm run typecheck && npm test && npm run build` — PASS, 45 files / 764 tests Docs: `docs/ARCHITECTURE.md` records the new `pool_changed` reason and the 409 `dinner_already_planned`.
orchestrator-agent added 1 commit 2026-10-07 16:19:08 -04:00
Revalidate the draft pool on save, refuse duplicate swaps, refresh the Quest log after a draft
CI / ci (push) Successful in 3m18s
Details
CI / ci (pull_request) Successful in 3m19s
Details
b3b342ef55 
Feature 14 integration fix (Maestro 857e4c33-e250-46ec-b5ea-7dd4f3b719d9).

- F14-01: storeDraft re-reads the draft pool (ListDraftPool, the same
  eligibility rule draft_week was given) inside the save transaction. A
  day whose dinner was archived, moved to tried, or planned on another
  current or later week while draft_week ran fails the job with the new
  retryable reason pool_changed and stores nothing.
- F14-02: Swap refuses a dinner already on another day of the week inside
  the write transaction; the API answers 409 dinner_already_planned.
- F14-03: a finished draft invalidates the Quest log query, as the swap
  path does, so the tablet strip and desktop panel drop dinners it planned.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
review_agent left a comment
Member
Copy Link
Copy Source

Independent Codex gpt-6-sol review: APPROVED for this exact commit.

Independent Codex gpt-6-sol review: APPROVED for this exact commit.
approver_agent merged commit 76a2b611c8 into feature/ca2a118d-0beb-49f9-b2ef-d5ed0ac1fa2a-weekly-plan-builder 2026-10-07 16:30:04 -04:00
Sign in to join this conversation.
Reviewers
No Reviewers
review_agent
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
approver_agent (The Approver) chatgpt claude-ai Claude-Code codex (Codex) dev_agent grok opencode orchestrator-agent overseer (Joshua) review_agent (The Reviewer)
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Maniacs_Mansion/MealQuest#122