Add multiple card assignments and explicit import mapping - #2977
Conversation
…40/card-assignments
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Chris0Jeky
added this to the
v0.4 — Hosted Open Beta + Work Model + Fabric Foundation milestone
|
ROUND 1 independent review disposition at 08cdf99 against 86c6f1b: both distinct lenses return SHIP. No confirmed CRITICAL/HIGH blockers and no remaining MEDIUM/LOW findings. Correctness/integration lens traced CAS, proposal restrictions and readable diffs, transaction/reload boundaries, rollback/audit/notifications, import mappings/legacy parsing and UI drafts/stale responses. A tentative archive-after-assignment conflict was ruled out: archive obtains a fresh preview timestamp before confirmation. Authority/privacy lens traced claims-derived caller attribution, owner-or-access eligibility, Viewer assignment without grants, revoke/erasure including archived cards, single/batch Apply, MCP Read/Propose classification, explicit importer/null mapping, preview rollback and scoped exports. Existing sole-owner erasure ambiguity is unchanged in the baseline. Both reviewers used guarded read-only inventories on the clean exact head; fingerprint comparisons passed and state was cleaned. They inspected source and test receipts but did not rerun tests or call GitHub. The earlier broad backend invocation remains nonzero with the documented superseding green checks; review does not relabel it green. Exact-head hosted CI is still required. No fix round has been used. |
![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: 08cdf99d9c
ℹ️ 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".
|
CI triage: Required run 34544027968 frontend job 103092733907 reports a Vitest worker SIGSEGV while running the unchanged lazyLocales.spec.ts, not a failed assertion or coverage threshold. It completed 421 files with 6,527 passed and three skipped; the crashed file accounts for the missing three local tests. The failure remains a merge blocker. Log preserved as taskdeck-alpha-evidence/2977-frontend-ci.log. The next candidate must pass this hosted gate; no flaky classification or waiver is being applied. Automatic review's three comments are being causally triaged together before the bounded fix round. #2951 has now merged, so the next assignment candidate must also integrate main 02abedf and re-prove the affected WIP/assignment seam. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29ead8fdf1
ℹ️ 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".
| } finally { if (request === generation) saving.value = false } | ||
| } | ||
| const unavailable = computed(() => props.readOnly ? [] : selected.value.filter(id => !participants.value.some(p => p.userId === id))) | ||
| onBeforeUnmount(() => { generation++ }) |
There was a problem hiding this comment.
When a user clicks Save assignments and then confirms “Discard changes” before a slow PUT completes, unmounting only increments generation; it suppresses the eventual response handling but cannot cancel or roll back the already-sent server mutation. The assignment can therefore commit after the editor has promised that the changes were discarded, so propagate the pending-save state to the modal and disable closing/discarding until it settles.
AGENTS.md reference: frontend/AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed source path; coordinator classifies this HIGH at the save/discard trust boundary. The assignment field does not expose saving state to CardModal, which offers Discard while the request may commit. Runtime delayed-request reproduction is still outstanding. Parked under the user's session wrap-up request with exact restart contract #2981. This thread stays unresolved and the head must not merge. No repair/fix round has been started.
|
[Codex lane release v2] CLAUDE_SYNC_PACKET Evidence exported to C:/Users/Public/codex-shell-home/taskdeck-alpha-evidence/2240/ (logs/HANDOFF.log, coverage, browser results and synthetic database). Local final proof: 238 Application + 53 API, full frontend coverage 6530 passed/3 skipped; prior two browser journeys pass. Those checks missed #2981 and do not waive it. All worktrees removed; main's unrelated bash.exe.stackdump preserved. |
|
CLAUDE_SYNC_PACKET This small documentation correction belongs to the existing owner; no competing UPGRADING edit was made. The broader reconciliation describes archive/type/parent/assignment portability and typed-link Down obligations separately. |
Legacy and Paper, delayed success and delayed failure, across Escape (the editor binding and the shared escape stack), the backdrop, the header close and the discard confirmation - including a save that starts behind an already open confirmation. Settlement is asserted too: the notice is withdrawn, the committed assignees or the failure and the kept draft are shown, and the close paths work normally again. The field spec pins the saving-change contract, and the Paper board spec pins its own refusals: card switch, route leave, an open discard dialog withdrawn when a save starts behind it, and the unload warning. Restores the discard-dialog withdrawal in handleCardEditorSavingChange, which was lost while verifying the new specs against the pre-fix source.
The assignment contract now states what the editor does while a set replacement is in flight, and lists the new proof entry points. UPGRADING gains the card-archive migration note from the PR #2977 sync packet, placed after the work-item-type paragraph so the Unreleased sequence stays in newest-first order - 20260910165817 predates the type, parent and assignment migrations. Verified present on main as backend/src/Taskdeck.Infrastructure/Migrations/20260910165817_AddCardArchiveLifecycle.cs.
…2981) The editor now refuses to close while an assignment PUT is unanswered, and the shared axios instance sets no default timeout - a socket that never answers would hold the card editor open indefinitely. This one call gets a 30s bound. On expiry the caller sees the existing uncertain-save outcome (draft kept, refresh before retrying), which is exactly what a client timeout means: it does not cancel a request the server may still be processing. The call already opts out of retries, so nothing is resubmitted.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11883e3ce8
ℹ️ 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".
| watch(assignmentSaving, (saving) => { | ||
| emit('saving-change', saving) |
There was a problem hiding this comment.
When using the Legacy board, navigating through the shell while an assignment PUT is pending unmounts CardAssignmentField, discards the eventual response, and may leave a committed mutation with no success or failure feedback. The fresh code consumes this newly emitted saving-change event only in PaperBoardView; Legacy's ColumnLane does not handle it, and BoardView has no corresponding route-leave or unload guard. Keep Legacy navigation disabled or guarded until the request settles.
AGENTS.md reference: frontend/AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Triaged once: real, non-blocking (MEDIUM). No discard promise is made on this path and the mutation commits server-side; the gap is settlement feedback on Legacy. Tracked as #2999 together with the product-doc sentence that overstates Legacy.
| public async Task<IReadOnlyList<User>> ReadParticipantsAsync(Guid boardId, CancellationToken ct) | ||
| => await context.Users.AsNoTracking().Where(u => u.IsActive && | ||
| (context.Boards.Any(b => b.Id == boardId && b.OwnerId == u.Id) || | ||
| context.BoardAccesses.Any(a => a.BoardId == boardId && a.UserId == u.Id))) |
There was a problem hiding this comment.
When an assignee calls POST /api/users/{id}/deactivate, UserService.DeactivateUserAsync only marks the user inactive, while this new eligibility query excludes inactive users without removing their existing assignment rows. Those stale responsibilities therefore remain displayed and exported on active and archived cards, with no cleanup audit or realtime event, even though the user is no longer an eligible participant; integrate deactivation with the same transactional detach path used for access revocation and account erasure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Triaged once: real, non-blocking (MEDIUM). No authorization bypass or data loss; stale rows stay visible until detached. Tracked as #3000, to reuse the revocation/erasure detach path.
|
#2981 repaired on this branch. Exact head: What the fix does
The generation/identity guard is untouched: a delayed receipt for a card or session the editor has left is still dropped, and drafts stay independent. No backend file changed — server-side permission handling was deliberately left alone (issue point 5). Commits
Reproduction first, as the issue required
after confirming "Discard changes" with the PUT still unanswered — the editor emitted Checks run at this head
New coverage: NOT verified
Docs
|
|
Round 2 (fix round) review at exact head Round count: round 1 was the Codex lane's original review (parked on HIGH #2981); this is round 2. Per law 2d the pipeline ships or parks here; only a new CRITICAL introduced by the fixes would reopen it, and none was found. Fresh-context independent review of the fix batch verified from source:
Triage of everything raised since the parked head (law 2c):
Author-run proof at this head: full frontend vitest 6,627 passed; chromium Playwright Disposition: parked status lifted. Merging with a merge commit once the last hosted check at this head completes and the aging floor is met. |
…nter CardAssignmentField shared one generation counter between load() and save(). The readOnly watcher is reachable while a PUT is unanswered - a background board refetch that re-reports write permission mid-save runs load() - and that bumped the shared counter, so the save's finally no-opped. saving stayed true, locked stayed true, and the saving-change the card editor reads to refuse every close affordance (#2977/#2981) stayed latched at true until navigation remounted the field, trapping the user in the editor. Split the counter per request kind. Each request still rejects its own stale body - a superseded save, a read left behind by a newer read - and the card-identity watcher and unmount bump both, so a receipt for a card the editor has left is still dropped. A read cannot invalidate a write it does not contend with: a non-refreshing load() only replaces the participant list, and the refreshing one is unreachable while saving holds. The #2982 permission lock, its release rules and load()'s stale-response rejection are unchanged. Closes #3017
1 participant
Parked status lifted 2026-09-11 (round 2 complete). The parked head
29ead8fdfwas never merged. Blocker #2981 was reproduced with a deferred request and repaired on this branch; the fix batch received its fresh-context independent review at head11883e3ce(no CRITICAL/HIGH; MEDIUM follow-ups tracked as #2997, #2999, #3000). The PR now ships under the ordinary T3 gate: exact-headci-requiredgreen plus the aging floor.Historical banner: Parked at the user's clean-checkpoint request. Late review identified pending-save/discard blocker #2981. No fix round had been used at that point.
#2981 repair pushed (2026-09-11). The defect was reproduced with a deferred request and repaired on this branch; the parked status is left for the reviewing agent to lift. See the "#2981 — in-flight assignment save" section below.
Summary
Cards now support multiple assignees drawn from active board owners and participants. Assignment grants no access. The shared Paper/Legacy editor supports explicit save, clear and cancel while preserving other card drafts; stale saves require a refresh and deliberate retry.
Versioned direct edits and proposal preview/apply share eligibility and state checks. Assignment rows and the detailed assignment audit use the authenticated applying user, including when a different person authored the proposal. Access revocation and account erasure detach affected assignments atomically. Assignment changes and their audit rows commit together; notifications follow commit.
Board import now parses and validates the full file before showing a preview. Every source assignee must explicitly map to the importer or Unassigned before creating the new board. Names never match automatically and memberships are not imported. Assignment-bearing exports use version 4; older assignment-free files remain supported.
A submitted assignment PUT cannot be recalled, so while one is in flight the editor never offers a discard: closing, Escape, the backdrop, the header close, the thinking-deck link, switching card and leaving the board all answer that the change was already sent, and an open discard confirmation is withdrawn. Settlement restores every control and shows the committed assignees or the failure with the kept draft.
Verification
Original local evidence at source head
08cdf99d9ce8865d1aaa024e06f4feea3f12c63a, based on86c6f1bdf:dotnet test backend/Taskdeck.sln -c Release -m:1: Domain 1,676, CLI 243 and Integration 36 passed. The broad run was not fully green: Application had an obsolete transaction assertion, API had an obsolete MCP tool-count assertion (3,169 passed, one failed, four skipped), and Architecture had three controller-scanner failures.npx vitest --run --maxWorkers=2: 6,530 passed, three skipped across 422 files; final focused frontend suite 138 passed. Typecheck, build and lint pass, with nine existing lint warnings.npx playwright test tests/e2e/card-assignments.spec.ts --project=chromium --workers=1 --reporter=line: two real-API Paper/Legacy journeys pass. Includes independent card drafts, multi-assign/clear, conflict refresh and explicit import mapping.Full API was not repeated after the small final parser/declaration and test-only changes; affected checks above supersede them. Exact-head hosted CI remains required. No live-provider or release validation is claimed. Evidence exported to
C:/Users/Public/codex-shell-home/taskdeck-alpha-evidence/2240/; final receipt islogs/HANDOFF.log. The clean pushed worktree was removed.Current-base qualification at
29ead8fdf15d6761f2b1d7c14ad147e309d205b5integrates main02abedfe9: 238 Application and 53 API tests pass, and the bounded independent interaction review is SHIP. Full Windowsnpm run test:coveragewith two workers passes 6,530 tests, three skipped, and all coverage thresholds. The earlier Linux hosted run crashed a Vitest worker in unchangedlazyLocales.spec.ts; its cause is unconfirmed. Local coverage does not waive the fresh hosted gate.#2981 — in-flight assignment save
Frontend-only; no backend file changed, and server-side permission handling was deliberately not touched.
64e01502badds the deferred-PUT component test and is deliberately red on both surfaces (AssertionError: expected [ [] ] to be undefinedat the post-confirmationcloseassertion) — the editor emittedclose, the UI's "discarded" outcome, while the submitted mutation was still unanswered and still committed.CardAssignmentFieldnow emitssaving-change;CardModalrefuses every close path while it is true and answers with a notice ("already sent to the server … cannot be discarded or cancelled"), withdrawing it on settlement.PaperBoardViewrefuses its own card-switch, route-leave and pending-discard confirmations the same way, andbeforeunloadstill warns.cardsApi.replaceAssignmentsnow carries a 30s bound. On expiry the existing uncertain-save outcome applies (draft kept, refresh before retrying); the call already opts out of retries.5c3a81e6e: fullnpx vitest --run --maxWorkers=2— 432 files, 6,627 passed, three skipped;npm run typecheck;npm run build; ESLint clean on every changed file;node scripts/check-doc-links.mjsandcheck-docs-governance.mjspass.npx playwright test tests/e2e/card-assignments.spec.ts --project=chromium --workers=1: 4 passed against a real stack (scripts/dev-up.ps1), including the two new held-PUT journeys that drive the real editor for Paper and Legacy.CardModalAssignmentSave.spec.tscases and all 4 newPaperBoardView.spec.tscases fail.Documentation
docs/product/CARD_ASSIGNMENTS.md— now also states the in-flight save behaviour and the new proof entry points.UPGRADING.mdrecords migration defaults, version-4 import mapping and rollback data loss, and now carries the20260910165817_AddCardArchiveLifecyclenote from the Codex sync packet (verified present on main).Tracking
Closes #2240. Closes #2981. Refs #2093.
Priority II, v0.4. Estimates, roll-ups, new identity tables and invitations remain outside this assignment slice.
Risk Notes
The schema, proposal executor, account cleanup and import paths change together. Assignments never grant authority; MCP changes only provide participant reads and proposal creation. One assignment replacement per card per proposal avoids conflicting writes to the same reviewed version. Both original independent reviews and the current-base interaction review passed. Exact-head hosted CI remains a merge gate. Non-blocking review follow-ups are tracked separately: secondary execution-history attribution #2978, revocation webhook card IDs #2979, and conflicting import source labels #2980, and permission-loss recovery #2982.
The #2981 repair narrows what the editor will do: it can no longer be closed while an assignment PUT is unanswered. The window is bounded by the request itself, every refusal is announced, and a failed save returns the editor to the ordinary discard path.