Add document approval system

[SachaProbo]

Introduce a quorum-based approval workflow that decouples approval from publication. Each approval request creates a quorum with individual decisions, enabling approval history and policy-driven access control.

• Add DocumentVersionApprovalQuorum and ApprovalDecision entities with PENDING/APPROVED/REJECTED lifecycle
• Simplify DocumentVersionStatus to DRAFT and PUBLISHED
• Enforce publish/approval rules via IAM deny policies on resource.last_quorum_status attribute
• Rename document approvers to default approvers
• Add approval history tab and approve page in frontend

---

Summary by cubic

Adds a quorum-based approval workflow for document versions with per-approver decisions, auto‑publish on full approval, and email notifications. Publishing can still bypass approvals; we show a warning if a request is pending.

• New Features

  • Quorums and decisions (PENDING/APPROVED/REJECTED); one pending quorum per version (DB‑enforced); all approvals required; any rejection fails; auto‑publish when approved; publish can bypass with a warning.
  • Console: Approvals tab (/documents/:documentId/approvals), Approve page (/documents/:documentId/versions/:versionId/approve), combined Publish/Request approval dialog, header actions (Request approval, Publish), add/remove approvers while pending, approval progress in lists, PDF preview/export.
  • API/Policy: New GraphQL mutations and types — requestDocumentVersionApproval, approveDocumentVersion, rejectDocumentVersion, addDocumentVersionApprover, removeDocumentVersionApprover; bulkPublishDocumentVersions now returns documentVersions and documents; last_quorum_status exposed on documents/versions for IAM; deny requesting approval if a quorum is pending or the version is published; approval actions restricted to approvers.
  • Emails: New approval invite (document-approval) with review link; e‑sign certificate emails use document name; added e‑sign document type variants.

• Refactors

  • Renamed approvers to defaultApprovers (defaultApproverIds) across GraphQL, MCP, console, forms, tests; table is document_default_approvers.
  • Document version status simplified to DRAFT and PUBLISHED; routes, MCP spec, and e2e updated; minor UI cleanups (e.g., removed avatars in people pickers and signature lists).

^{Written for commit 4e1ec67. Summary will update on new commits.}

[cubic-dev-ai]

17 issues found across 60 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="pkg/probo/policies.go">

<violation number="1" location="pkg/probo/policies.go:165">
P1: ViewerPolicy grants `Approve` and `Reject` but omits the `documentWriteActiveOnly` deny rule, so viewers can approve/reject document versions on archived documents — bypassing the protection that Owner and Admin have. Add `documentWriteActiveOnly` to ViewerPolicy.</violation>
</file>

<file name="apps/console/src/pages/organizations/documents/approvals/_components/DocumentApprovalListItem.tsx">

<violation number="1" location="apps/console/src/pages/organizations/documents/approvals/_components/DocumentApprovalListItem.tsx:223">
P1: PDF export is triggered on component mount instead of when the approval dialog is opened, causing unnecessary export requests for each list item.</violation>

<violation number="2" location="apps/console/src/pages/organizations/documents/approvals/_components/DocumentApprovalListItem.tsx:273">
P2: The reject dialog is closed in `onCompleted`, which runs even when GraphQL errors are returned; failed rejections still dismiss the dialog.</violation>
</file>

<file name="apps/console/src/pages/organizations/documents/approvals/DocumentApprovalsPageLoader.tsx">

<violation number="1" location="apps/console/src/pages/organizations/documents/approvals/DocumentApprovalsPageLoader.tsx:31">
P2: Add a dependency array to the initial load effect to avoid running it on every render and risking duplicate network loads.</violation>
</file>

<file name="pkg/server/api/mcp/v1/schema.resolvers.go">

<violation number="1" location="pkg/server/api/mcp/v1/schema.resolvers.go:3352">
P2: Return persisted version approvers instead of raw request approver IDs in `RequestDocumentVersionApprovalTool` to avoid inconsistent `DocumentVersion` approver data.</violation>
</file>

<file name="apps/console/src/pages/organizations/documents/_components/DocumentList.tsx">

<violation number="1" location="apps/console/src/pages/organizations/documents/_components/DocumentList.tsx:317">
P2: `colspan` is hardcoded to 11 even when the optional action column is hidden, causing a column-span mismatch in the selection row.</violation>
</file>

<file name="pkg/coredata/document_version_approval_decision_filter.go">

<violation number="1" location="pkg/coredata/document_version_approval_decision_filter.go:29">
P2: An empty `states` slice is treated as an active filter and makes the query return zero rows. Normalize empty input to `nil` so empty means “no state filter.”</violation>
</file>

<file name="apps/console/src/pages/organizations/documents/approve/DocumentApprovePage.tsx">

<violation number="1" location="apps/console/src/pages/organizations/documents/approve/DocumentApprovePage.tsx:199">
P2: This view renders all decisions but uses first-person status text, so users can incorrectly see "You have approved/rejected" for other approvers’ decisions.</violation>
</file>

<file name="pkg/server/api/console/v1/v1_resolver.go">

<violation number="1" location="pkg/server/api/console/v1/v1_resolver.go:5005">
P2: Missing specific error handling in `BulkPublishDocumentVersions`. The single `PublishDocumentVersion` resolver handles `ErrDocumentArchived` → Conflict and `ErrDocumentVersionNotDraft` → Invalid, but the bulk variant returns a generic internal error for all failures. Add matching error type checks for consistency.</violation>
</file>

<file name="pkg/probo/document_service.go">

<violation number="1" location="pkg/probo/document_service.go:1913">
P2: PDF export now fetches approved decisions with a single fixed-size page (100), which can omit approvers when a version has more approvals than that page size.</violation>
</file>

<file name="pkg/coredata/migrations/20260319T150000Z.sql">

<violation number="1" location="pkg/coredata/migrations/20260319T150000Z.sql:9">
P2: Missing indexes on `document_version_id` foreign key columns. The partial unique index on the quorums table only covers `PENDING` rows, so general lookups and `ON DELETE CASCADE` from `document_versions` will require sequential scans on both tables. Add regular indexes for these FK columns.</violation>
</file>

<file name="pkg/coredata/membership_profile.go">

<violation number="1" location="pkg/coredata/membership_profile.go:830">
P2: This join can overcount version approvers because approval decisions are historical per quorum; the same approver may have multiple decision rows for one document version.</violation>
</file>

<file name="pkg/probo/document_approval_service.go">

<violation number="1" location="pkg/probo/document_approval_service.go:130">
P2: DB errors from `LoadPendingByDocumentVersionID` are silently swallowed. If the query fails for a reason other than 'not found' (e.g., connection error), the existing pending quorum won't be rejected and a second pending quorum will be created. Distinguish 'not found' from other errors, e.g. using `errors.Is` or `errors.As` with the appropriate sentinel.</violation>

<violation number="2" location="pkg/probo/document_approval_service.go:256">
P1: PDF generation (chromedp) and S3 upload run inside the database transaction, holding the connection for potentially seconds. Split the transaction: gather the data needed for PDF in the transaction, commit, then generate the PDF and upload outside the transaction. A second short transaction can record the file and update the decision.

(Based on your team's feedback about splitting data fetching and PDF generation outside the transaction.) [FEEDBACK_USED]</violation>

<violation number="3" location="pkg/probo/document_approval_service.go:551">
P2: `LoadLastByDocumentVersionID` returns the last quorum regardless of status. If the last quorum was REJECTED, the new approver decision gets added to it but the quorum stays REJECTED. Either use `LoadPendingByDocumentVersionID` (like `Approve`/`Reject` do) to only allow adding approvers to pending quorums, or also handle the REJECTED case explicitly.</violation>
</file>

<file name="pkg/server/api/mcp/v1/specification.yaml">

<violation number="1" location="pkg/server/api/mcp/v1/specification.yaml:5096">
P1: `PENDING_APPROVAL` was added to the MCP enum, but the mapped Go `DocumentVersionStatus` type does not support that value, causing invalid-status parse/serialization failures.</violation>

<violation number="2" location="pkg/server/api/mcp/v1/specification.yaml:7464">
P1: Replacing `publishDocumentVersion` with `requestDocumentVersionApproval` removes MCP’s publish operation, leaving no tool to transition versions to `PUBLISHED`.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[SachaProbo]

@cubic-dev-ai please review

[cubic-dev-ai]

@cubic-dev-ai please review

@SachaProbo I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

10 issues found across 58 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="pkg/probo/document_service.go">

<violation number="1" location="pkg/probo/document_service.go:1918">
P2: PDF export only fetches the first 100 approved decisions, so approver names are truncated for larger quorums.</violation>
</file>

<file name="pkg/probo/policies.go">

<violation number="1" location="pkg/probo/policies.go:81">
P1: `documentApproverRequiresQuorum` only blocks add/remove when no quorum exists (`""`), but permits it when the quorum is `APPROVED` or `REJECTED`. Since the intent is to allow approver changes only while a quorum is `PENDING`, use `NotEquals(…, "PENDING")` instead — this covers all three non-pending states in one condition.</violation>
</file>

<file name="pkg/coredata/migrations/20260319T160000Z.sql">

<violation number="1" location="pkg/coredata/migrations/20260319T160000Z.sql:16">
P2: Missing non-partial index on `document_version_approval_quorums.version_id`. The partial unique index only covers `PENDING` rows, so fetching all quorums for a version (e.g., the approval history tab) cannot use it and will seq-scan the table.</violation>
</file>

<file name="pkg/coredata/membership_profile.go">

<violation number="1" location="pkg/coredata/membership_profile.go:753">
P2: `DocumentVersion.approvers` will include approvers from all historical quorums because the query only filters by version_id. This should be restricted to the last (or active) quorum so the list reflects the current approvers.</violation>

<violation number="2" location="pkg/coredata/membership_profile.go:835">
P2: CountVersionApprovers will overcount after multiple approval quorums because it includes approvers from all quorums; constrain the count to the latest (or active) quorum.</violation>
</file>

<file name="apps/console/src/pages/organizations/documents/_components/DocumentList.tsx">

<violation number="1" location="apps/console/src/pages/organizations/documents/_components/DocumentList.tsx:124">
P3: Adding documentTypeFilter to the effect deps causes a duplicate refetch whenever the filter changes because the handler already refetches. This doubles the network request for each filter change.</violation>
</file>

<file name="pkg/probo/document_approval_service.go">

<violation number="1" location="pkg/probo/document_approval_service.go:184">
P1: PDF generation and S3 upload run inside the database transaction, holding the connection for the duration of chromedp rendering and the S3 PUT. This risks connection pool exhaustion under load. Split the workflow: gather the data and update decision/quorum state within the transaction, then perform PDF generation and file upload outside it (or in a follow-up step).

(Based on your team's feedback about splitting data fetching and PDF generation out of transactions.) [FEEDBACK_USED]</violation>

<violation number="2" location="pkg/probo/document_approval_service.go:199">
P0: `RemoveApprover` does not check whether the quorum is still `PENDING` before calling `maybeApproveQuorum`. Removing a rejected decision from a `REJECTED` quorum can flip it to `APPROVED` and auto-publish the document version, bypassing the intended approval workflow.</violation>

<violation number="3" location="pkg/probo/document_approval_service.go:286">
P2: `AddApprover` does not verify that the loaded quorum is still in `PENDING` state. An approver can be added to an already-approved or rejected quorum, creating a dangling pending decision and sending a misleading approval-request email.</violation>
</file>

<file name="pkg/coredata/document_version_approval_decision.go">

<violation number="1" location="pkg/coredata/document_version_approval_decision.go:106">
P2: Use `pgx.CollectExactlyOneRow` for single-row loads so duplicate matches are surfaced instead of silently taking the first row.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}