fix(isms): management review deploy-review fixes - #3486
Merged
Conversation
Post-merge review round on the production deploy PR:
- ensure-setup now heals an empty Programme (9.2) / Procedure (9.3)
paragraph on EXISTING documents, not only ones created in the same
call — a document left blank (e.g. a crash between provision and
seed) previously stayed blank forever; the per-doc updateMany guard
still protects populated drafts
- attendee parsing is now an exact client/server mirror: all-or-nothing
on malformed entries and deduped by member on read, so the Submit UI
and the server gate always agree and duplicate legacy entries cannot
render duplicate chips or duplicate keys
- the chair select gained an explicit "No chair yet" option so a stale
chair can be cleared, not only replaced
- the outputs editor closes when the review becomes signed or edit
rights are lost, matching the other locked-state affordances
- action-owner display stays neutral ("Unknown member") when the people
roster is unavailable instead of mislabeling active owners as former
members
- deterministic ordering tie-breakers on review lists (position,
createdAt, id) and role assignments (position, id) so carry-forward
attribution and participant defaults are reproducible when positions
collide
- seeded inputs (c) and (d.4) point at the canonical ISMS document
titles so the agenda references are followable
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
tofikwest
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
cubic analysis
All reported issues were addressed across 13 files
Confidence score: 5/5
- Safe to merge after the addressed issues were fixed.
Linked issue analysis
Linked issue: CS-726: [Feature] Area- Management Review
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Procedure (Programme/Procedure) default is applied when a Management Review document's narrative is empty (heals pre-existing blank documents). | Service-level seeding was moved into ensureSetup with guarded updateMany; a regression test was added that asserts an empty management_review draftNarrative is healed. Tests and code changes demonstrate the behavior. |
| ✅ | Attendee parsing mirrors server contract: all-or-nothing for malformed entries and dedupe by member on read (client/server agree). | Server parseReviewAttendees was changed to dedupe and return [] on invalid input; client-side parseAttendees was updated to mirror all-or-nothing + dedupe. Tests on both sides were added/updated. |
| Chair selection can be cleared (explicit 'No chair yet' option, sentinel-backed). | UI change was implemented to add a NO_CHAIR option and map it to an empty stored value, but no specific unit test was added for this behavior in the diff. The code shows the intended behavior, but verification coverage for this exact UI interaction is not shown. | |
| Outputs editor closes / stops editing when the review becomes signed or caller loses edit permission (honours lock). | The component now listens for canEdit changes and closes editing when lost; code change present, but no explicit tests for this interaction shown in the PR. | |
| Action owner display is neutral when people roster unavailable (renders 'Unknown member' instead of 'Former member'). | Component changes introduce the 'Unknown member' fallback when memberOptions is empty; implementation present but no unit test or end-to-end evidence shown in the diff. | |
| Deterministic ordering for reviews and role assignments (orderBy includes tie-breakers so carried-forward attribution is stable). | Server and export include ordering were updated to use deterministic tie-breakers ([position, createdAt, id]) and role assignments ordering adds id as a second key. The diffs show the change; explicit tests verifying downstream carry-forward behavior are not included in the PR. | |
| ✅ | Seeded Inputs table uses followable/canonical document titles for 'Where to find' cells (improves agenda followability). | Seed data for the inputs was updated to canonical document titles; the change is small and deterministic and present in the seed file. |
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
The server zod schema trims names on parse, so a legacy " Jane " entry rendered with spaces in the UI but trimmed in the generated minutes. The client parse now trims too, keeping both representations identical.
claudfuen
pushed a commit
that referenced
this pull request
Jul 22, 2026
# [3.107.0](v3.106.0...v3.107.0) (2026-07-22) ### Bug Fixes * **api:** resolve comments left by Cubic-dev-ai from Production-Deploy PR ([#3483](#3483)) ([e489af6](e489af6)) * **app:** show the actual error reason when comment deletion fails ([#3485](#3485)) ([25ac1d9](25ac1d9)) * **isms:** management review deploy-review fixes ([#3486](#3486)) ([49fcd6a](49fcd6a)) ### Features * isms management review (clause 9.3) ([#3481](#3481)) ([b201b7e](b201b7e))
Contributor
|
🎉 This PR is included in version 3.107.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the valid CS-726 (ISMS Management Review, clause 9.3) findings cubic raised on the production deploy PR #3479. Ten of the twenty-one ISMS findings were real; the rest are answered on their threads (per-clause conventions, a premise that doesn't hold, and one renderer-wide pre-existing behavior).
Fixes
updateMany(NULL/{}only) still guarantees a populated draft is never overwritten.parseAttendeesnow matches the API'sparseReviewAttendeesexactly: all-or-nothing on malformed entries and deduped by member on read. The Submit button and the server gate can no longer disagree, and duplicate legacy entries can't render duplicate chips / React keys.evidence:update, matching the details form and input rows.[position, createdAt, id](export include + getDocument; carry-forward attribution depends on it) and role assignments by[position, id](participant defaults).Verification
bun run --filter '@trycompai/app' buildexit 0.Not fixed here (answered on the #3479 threads): the non-ASCII PDF font limitation is renderer-wide and pre-existing across all ten ISMS document types; the "stale text after save" finding's premise doesn't hold (
updateRowawaits the SWR revalidation before edit mode exits); the remaining items are the per-clause duplication conventions already settled on #3481.