[comp] Production Deploy by github-actions[bot] · Pull Request #3364 · trycompai/comp

github-actions · 2026-07-07T17:28:42Z

This is an automated pull request to release the candidate branch into production, which will trigger a deployment.
It was created by the [Production PR] action.

Summary by cubic

Adds ISMS document versioning and history (CS-701) and a read‑only GitHub App integration using an installation-based OAuth flow (CS-710). Ensures exports are audit-safe and approvals are serialized to prevent double‑publish.

New Features
- ISMS API: GET /v1/isms/documents/:id/versions for history. Exports accept optional versionId and default to the current published artifact (draft only when never published).
- Data model: IsmsDocument adds currentVersionId, draftNarrative, and draftSnapshot. Approval freezes an immutable version with a contentSnapshot. Edits write to the draft and invalidate approval.
- Export integrity: Per-version downloads serve the stored file or re-render from the immutable snapshot; never from live data. Default export serves the current published version when one exists.
- Concurrency safety: Approve/decline take a per-document advisory lock and atomically claim status to prevent double‑publish. All edits acquire the same lock via centralized approval invalidation. Control link add/remove now locks before writing so those mutations serialize with approve.
- UI: Version history with per-version PDF/DOCX download, accessible labels, and a clear “Published vN / editing creates a new draft” state. Fixes: legacy approvals respected without published versions; Scope form re-seeds when draftNarrative changes; minor banner formatting.
- Integrations: New github-app manifest adds a fine‑grained, read‑only GitHub App connection. OAuth supports appInstallFlow (start redirects to the App install URL with state only; GitHub ignores redirect_uri). The legacy github OAuth integration is unchanged. The install callback no longer persists installation_id (unverified).
Migration
- Schema: updates IsmsDocument/IsmsDocumentVersion; adds Member.publishedIsmsVersions. Legacy pre-version rows are dropped; existing approvals remain, and the next approval creates the first real published version.
- No manual steps. Exports for never‑published docs render the current draft until the first approval. S3 uploads for retained artifacts are handled via AttachmentsModule.

^{Written for commit 9c6f6fa. Summary will update on new commits.}

* feat(isms): document versioning and history (CS-701) Bring ISMS documents onto the Policies versioning model. Approving a document freezes an immutable published version (v1). Editing afterwards creates a draft without touching the published version, which stays live and exportable. Re-approving publishes the next version and moves the old one into history. Rendered DOCX/PDF are retained per published version so an auditor sampling v1 later downloads exactly what was approved. - schema: IsmsDocument gains currentVersion/draftNarrative/draftSnapshot; IsmsDocumentVersion gains contentSnapshot/changelog/publishedBy. Backfill migration (pre-GA, flag-gated) moves the old single-version model over. - api: new IsmsVersionService freezes a version at approval and renders + uploads PDF/DOCX to S3 after the txn; lists history; serves a version's export (stored file -> snapshot re-render -> live fallback). Editing an approved document reverts status to draft but preserves the published version. Drift baseline + draft narrative moved onto the document. New GET /v1/isms/documents/:id/versions; optional versionId on export. - app: version-history section on every document detail page with per-version PDF/DOCX download, and a "Published vN / editing creates a new draft" signal. - tests: 312 api + 119 app tests passing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp * fix(isms): address CS-701 PR review — export integrity, download UX, org scoping Fixes the real issues from the automated PR review: - Versioned export never serves live/draft data (audit integrity): getVersionExport now falls back to the immutable snapshot if a stored S3 object is missing, and throws for a version with neither a file nor a snapshot instead of rendering current live data. - Default export of a clean approved document serves its retained published artifact (byte-identical to approval); only in-progress drafts export the draft. - Regenerating ("Generate from platform data") now invalidates approval, matching every other edit path, so an approved doc can't silently diverge from its published version. - Version-history download buttons are gated on real availability (file or snapshot); unavailable formats are hidden with a clear note. - Version-history fetch passes the route org (X-Organization-Id), matching the per-version download path. - Approval success is separated from the best-effort post-approval refresh so a revalidation hiccup no longer reports "Failed to approve". - Version history retains the approver via the frozen snapshot metadata if the approving member is later deleted. Not changed (deliberate): approval stays committed if export rendering fails — the immutable contentSnapshot makes every version reproducible and the AWS SDK retries transient errors; throwing would strand a doc "approved but unretryable". Tests: 316 api + 122 app ISMS tests passing; typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp * fix(isms): second review pass — default export, download errors, tx isolation, a11y Addresses the follow-up automated review on the fix commit: - Default export (no versionId) now serves the current PUBLISHED version whenever one exists — so v1 stays the live/exportable default while v2 is being drafted (CS-701). Only a never-published document renders the live draft. (P1) - Per-version downloads now catch export failures and surface a toast instead of failing silently / as an unhandled rejection. (P2) - The approve snapshot reads the org profile through the same transaction client (threaded through resolveOrgProfile/loadOrgProfile) so profile + register rows share one point-in-time. (P2) - Version-history download buttons get version-specific accessible labels ("Download v2 as PDF") for screen-reader / voice-control users. (P3) Tests: 318 api + 123 app ISMS tests passing; typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp * fix(isms): third review pass — no snapshot-less published versions, download races - Migration no longer promotes legacy approved docs to a published version. The pre-CS-701 version row had no contentSnapshot, so serving it as currentVersion would 404 on export (regression from the default-export + no-live-fallback fixes). Legacy approved docs revert to draft and re-approve once to establish a real v1; all legacy version rows are dropped. currentVersionId is now only ever set by the approve flow, which always writes a contentSnapshot — so it can never point at a snapshot-less version. (P1) - Per-version download indicator clears only if the finishing request still owns it, so starting a second download no longer clears the first's spinner early. (P3) Not changed (deliberate, re-confirmed): approval stays committed if publishRenders fails — createPublishedVersion always persists an immutable contentSnapshot, so the version re-renders on demand (no lost export); forcing a throw would strand a doc "approved but unretryable". Tests: 318 api + 123 app ISMS tests passing; typecheck clean; migration SQL validated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp * fix(isms): migration — preserve approvals, add FKs after legacy cleanup Addresses the latest review on the CS-701 migration: - Non-destructive: existing approved documents keep their approved status. The migration no longer resets them to draft — it only drops the legacy (pre-CS-701, snapshot-less) version rows and leaves currentVersionId null, so an approved doc exports its current content on demand and captures a real versioned artifact on its next approval. currentVersionId is only ever set by the approve flow, which always writes a contentSnapshot. (P2) - Defensive ordering: the legacy-row DELETE now runs BEFORE the foreign keys are added, so FK validation can never trip on a stale publishedById. (In practice publishedById was a dead/always-null column pre-migration, so it could not have failed — but the ordering makes it robust regardless.) (P2) Verified: migration applies cleanly from scratch (fresh DB, all migrations deploy); 318 api + 123 app ISMS tests passing; typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

vercel · 2026-07-07T17:28:48Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
comp-framework-editor (staging)	Ready	Preview, Comment	Jul 7, 2026 9:17pm

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
app (staging)	Skipped		Jul 7, 2026 9:17pm
portal (staging)	Skipped		Jul 7, 2026 9:17pm

cubic-dev-ai

4 issues found across 49 files

Confidence score: 3/5

The highest risk is in packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql: approved documents can keep status='approved' while losing version linkage (IsmsDocumentVersion rows removed and no currentVersionId backfill), which can make “approved version exists” checks fail after deploy — update the migration to preserve/backfill the active approved version before merging.
In apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.tsx, draft content refreshes may not appear because ScopeForm consumes defaultValues only at mount and the remount key tracks published currentVersionId, not draft changes; users may see stale scope text after generate/reload — trigger a form reset or key update when draft content changes before merging.
apps/api/src/isms/dto/export-isms-document.dto.ts now documents that omitting versionId exports the working draft, but runtime behavior prefers currentVersionId when published content exists, which can mislead API consumers into exporting the wrong revision — align the OpenAPI description with actual behavior (or vice versa) before release.
apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx has a minor display regression where draft version renders as v 2 instead of v2; low impact, but it can look unpolished in approval UI — keep the label in one JSX expression to remove the extra space.

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="packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql">

<violation number="1" location="packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql:41">
P2: Existing approved ISMS documents can lose their “approved version exists” signal after this migration: the migration preserves `status = 'approved'` but deletes all `IsmsDocumentVersion` rows and never sets `currentVersionId`, while the API now derives `hasApprovedVersion` and version history from those fields. It would be safer to either backfill a real published version/current pointer for approved documents or explicitly transition legacy approved documents into a state that matches the new versioning model.</violation>
</file>

_{Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic}

cubic-dev-ai · 2026-07-07T17:37:38Z

+--    always writes a contentSnapshot — so currentVersionId never points at a
+--    snapshot-less version. This DELETE also clears the legacy (always-null)
+--    `publishedById` values before the FK below is added.
+DELETE FROM "IsmsDocumentVersion";


P2: Existing approved ISMS documents can lose their “approved version exists” signal after this migration: the migration preserves status = 'approved' but deletes all IsmsDocumentVersion rows and never sets currentVersionId, while the API now derives hasApprovedVersion and version history from those fields. It would be safer to either backfill a real published version/current pointer for approved documents or explicitly transition legacy approved documents into a state that matches the new versioning model.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql, line 41: <comment>Existing approved ISMS documents can lose their “approved version exists” signal after this migration: the migration preserves `status = 'approved'` but deletes all `IsmsDocumentVersion` rows and never sets `currentVersionId`, while the API now derives `hasApprovedVersion` and version history from those fields. It would be safer to either backfill a real published version/current pointer for approved documents or explicitly transition legacy approved documents into a state that matches the new versioning model.</comment> <file context> @@ -0,0 +1,53 @@ +-- always writes a contentSnapshot — so currentVersionId never points at a +-- snapshot-less version. This DELETE also clears the legacy (always-null) +-- `publishedById` values before the FK below is added. +DELETE FROM "IsmsDocumentVersion"; + +-- CreateIndex </file context>

) * feat(integrations): add read-only GitHub App integration (CS-710) Add a new, additive "GitHub App" integration (slug `github-app`) that connects via a GitHub App installation flow with fine-grained, read-only permissions, instead of the legacy `github` OAuth App which can only reach private repos through the broad `repo` scope (read + write). The existing `github` OAuth integration is left completely untouched so current connections keep working — customers opt into the new one. - New `github-app` manifest reuses the five existing GitHub checks verbatim (no logic duplication; check results are keyed per connection so ids can be shared across the two manifests). - Add an `appInstallFlow` flag to the OAuth config. When set, `/oauth/start` redirects to the App install URL (github.com/apps/{slug}/installations/new) with only `state` appended — client_id/response_type/scope do not apply. - The OAuth callback still exchanges the returned `code` for a user-to-server token (unchanged) and now persists `installation_id` on the connection so a future server-to-server (installation token) upgrade needs no re-connect. - App slug is admin-configurable via the existing additionalOAuthSettings mechanism ({APP_SLUG} substitution), like Rippling's app name. - Tests: new manifest test (registration, read-only flow, check reuse, legacy untouched) + OAuth controller tests for the install URL and installation_id capture. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XyttkYNbkdYRp7DBKNuFVH * fix(integrations): route GitHub App install callback per environment Pass `redirect_uri` on the GitHub App install URL (not just `state`). A GitHub App can register multiple callback URLs (prod/staging/localhost); without an explicit redirect_uri, GitHub falls back to the first-registered one, so a staging/localhost install could bounce back to production. Send the environment-specific callback so each environment routes to its own callback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XyttkYNbkdYRp7DBKNuFVH * fix(integrations): fail loudly when GitHub App install URL is unresolved cubic flagged that the org-scoped OAuth-app save path (oauth-apps.controller) does not persist customSettings, so org-level `github-app` credentials would leave {APP_SLUG} unresolved and `/oauth/start` would redirect to a broken GitHub install URL. github-app is a single Comp AI-owned platform App — bring-your-own-app at the org level isn't a supported flow for it — so instead of building org-level slug persistence, guard the install flow: if any install-URL placeholder token (e.g. {APP_SLUG}) is still unresolved after credential substitution, throw PRECONDITION_FAILED with a clear setup message instead of redirecting to a broken URL. Integration-agnostic (applies to any appInstallFlow provider). The platform-credential path (our deployment) persists customSettings via the admin UI, so the happy path is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XyttkYNbkdYRp7DBKNuFVH * fix(integrations): don't set redirect_uri on GitHub App install URL cubic (and GitHub's documented behavior) confirmed that GitHub ignores redirect_uri on github.com/apps/{slug}/installations/new and always redirects to the App's first registered callback URL after installation. The previous commit's redirect_uri on the install URL was therefore non-functional and its comment was inaccurate. Set only `state` on the install URL and document that per-environment routing must be handled with a separate GitHub App per environment (each with its own single callback URL). redirect_uri remains on the standard OAuth authorize flow, where GitHub does honor it. Update the test to assert redirect_uri is NOT on the install URL. Refs: https://github.com/orgs/community/discussions/54273 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XyttkYNbkdYRp7DBKNuFVH --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

tofikwest · 2026-07-07T19:41:37Z

The 4 ISMS issues cubic flagged here are addressed in #3365 (base main), which will land in release on the next deploy:

P2 hasApprovedVersion is now status-aware, so a doc approved before versioning existed (legacy version row dropped by the migration) no longer reports false.
P3 Scope form re-seeds on draft-content change (remount key now tracks draftNarrative, not just the published version id).
P3 export DTO description aligned with actual behavior (omitting versionId serves the current published version, not the draft).
P3 approval banner renders (v2) instead of v 2.

The migration is intentionally non-destructive (approvals preserved); the hasApprovedVersion fix makes that state self-consistent without a backfill. 319 API + 123 app ISMS tests passing.

Addresses the ISMS issues cubic flagged on the main->release deploy PR (#3364): - hasApprovedVersion is now status-aware (currentVersionId != null OR status === 'approved'), so a document approved before versioning existed — whose legacy version row the migration drops — still reports an approved version instead of flipping to false. (P2) - ScopeForm re-seeds when the persisted draft content changes: its remount key now reflects draftNarrative, not just the published currentVersionId, so scope text refreshes after "Generate from platform data" / reload. (P3) - export DTO description now matches runtime: omitting versionId exports the current published version (or the draft if never published), not always the draft. (P3) - Approval banner renders "(v2)" instead of "v 2" (single JSX expression). (P3) Migration is intentionally left non-destructive (approvals preserved); the hasApprovedVersion fix makes that state self-consistent without a heavy backfill for pre-GA data. Tests: 319 api + 123 app ISMS tests passing; typecheck clean. Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

tofikwest · 2026-07-07T19:55:29Z

@cubic-dev-ai review it

cubic-dev-ai · 2026-07-07T19:55:39Z

@cubic-dev-ai review it

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

cubic-dev-ai

1 issue found across 55 files

Confidence score: 2/5

apps/api/src/isms/isms.service.ts approval flow has a race condition: assertPendingApprovalBy runs outside the transaction, so concurrent approve requests can both pass the guard, create duplicate published versions, and leave currentVersionId pointing to the wrong record, causing version history/data integrity issues after merge — move the pending-approval check inside a single transaction (or enforce a DB-level uniqueness/locking strategy) before merging.

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="packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql">

<violation number="1" location="packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql:41">
P2: Existing approved ISMS documents can lose their “approved version exists” signal after this migration: the migration preserves `status = 'approved'` but deletes all `IsmsDocumentVersion` rows and never sets `currentVersionId`, while the API now derives `hasApprovedVersion` and version history from those fields. It would be safer to either backfill a real published version/current pointer for approved documents or explicitly transition legacy approved documents into a state that matches the new versioning model.</violation>
</file>

_{Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic}

tofikwest · 2026-07-07T20:15:37Z

The concurrent-approve race flagged here is fixed in #3366 (base main), landing in release on the next deploy.

Root cause: assertPendingApprovalBy ran before the transaction; under READ COMMITTED an in-tx status re-read doesn't serialize, and @@unique([documentId, version]) only catches the perfectly-simultaneous case (the staggered case silently double-publishes).

Fix: approve() takes a per-document advisory lock and atomically claims the approval (updateMany where status='needs_review', abort on count !== 1) inside the transaction; decline() gets the same guard so approve/decline can't both win. Pre-tx guard kept for error messages.

I also adversarially audited the full approve/publish/version/export flow — getVersionExport org-scoping, S3 keys (no IDOR), snapshot parsing, and currentVersionId integrity all verified safe. 321 API ISMS tests passing.

…lish (CS-701) (#3366) Flagged on the production deploy PR (#3364): assertPendingApprovalBy runs before the transaction, so two racing approve() calls could both pass the guard. Root cause (verified): under READ COMMITTED a plain in-transaction status re-read does NOT serialize — the loser reads the still-committed `needs_review`. The @@unique([documentId, version]) constraint only turns the perfectly-simultaneous case into an error; the staggered case silently creates a second published version and overwrites currentVersionId. Fix: - approve() now takes a per-document advisory lock and then claims the approval atomically via updateMany({ where: status='needs_review' + approverId }) inside the transaction, aborting when count !== 1. Exactly one caller freezes a version; losers abort before any derivation/version creation. The pre-transaction guard is kept for the normal-case specific error messages. - decline() gets the same atomic claim, so an approve/decline race can no longer leave a "declined" document that still owns a live currentVersionId. - Renamed the shared advisory-lock helper lockDocumentForPositions -> lockDocument (it now serializes approval too, not just register-row position allocation); updated the four register-create callers. Adversarially audited the whole approve/publish/version/export flow for related concurrency + IDOR issues; getVersionExport org-scoping, S3 keys, snapshot parsing, currentVersionId integrity and the migration paths verified safe. The narrow edit-during-approve window for register update/delete is low-severity/self-healing and left as a follow-up. Tests: 321 api ISMS tests passing (incl. new approve + decline race tests); typecheck clean. Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

tofikwest · 2026-07-07T20:22:02Z

@cubic-dev-ai review it

cubic-dev-ai · 2026-07-07T20:22:19Z

@cubic-dev-ai review it

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

cubic-dev-ai

1 issue found across 60 files

Confidence score: 3/5

In apps/api/src/isms/isms.service.ts, approval flow locking is incomplete because lockDocument only protects paths that also take the advisory lock, so concurrent register update/delete operations can still race with approval and produce inconsistent or invalid approved state — extend the same serialization/locking to register edit/remove transactions (or enforce it centrally) and add a concurrency test before merging.

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="apps/api/src/isms/isms.service.ts">

<violation number="1" location="apps/api/src/isms/isms.service.ts:230">
P1: Approval is still not serialized against register edits that update or delete existing rows. The new `lockDocument` call only blocks code paths that also acquire the advisory lock, but the register update/remove transactions do not, so an edit can interleave while approval is loading `EXPORT_DOCUMENT_INCLUDE` and freezing the version. Consider taking the same document lock in all register mutations that can change approved content, not only create paths, before publishing relies on this lock for immutability.</violation>
</file>

_{Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic}

cubic-dev-ai · 2026-07-07T20:29:08Z

+    const published = await db.$transaction(async (tx) => {
+      // Serialize concurrent approvals (and register-row creates, which take the
+      // same lock) on this document so they can't interleave and double-publish.
+      await lockDocument(tx, documentId);


P1: Approval is still not serialized against register edits that update or delete existing rows. The new lockDocument call only blocks code paths that also acquire the advisory lock, but the register update/remove transactions do not, so an edit can interleave while approval is loading EXPORT_DOCUMENT_INCLUDE and freezing the version. Consider taking the same document lock in all register mutations that can change approved content, not only create paths, before publishing relies on this lock for immutability.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/isms/isms.service.ts, line 230: <comment>Approval is still not serialized against register edits that update or delete existing rows. The new `lockDocument` call only blocks code paths that also acquire the advisory lock, but the register update/remove transactions do not, so an edit can interleave while approval is loading `EXPORT_DOCUMENT_INCLUDE` and freezing the version. Consider taking the same document lock in all register mutations that can change approved content, not only create paths, before publishing relies on this lock for immutability.</comment> <file context> @@ -204,10 +219,37 @@ export class IsmsService { + const published = await db.$transaction(async (tx) => { + // Serialize concurrent approvals (and register-row creates, which take the + // same lock) on this document so they can't interleave and double-publish. + await lockDocument(tx, documentId); + + // Atomically claim the approval: the check-then-act guard above runs before </file context>

tofikwest · 2026-07-07T20:36:26Z

The P1 residual flagged here (approval not serialized against register update/delete and other edits) is fixed in #3367 (base main), completing #3366.

Enforced centrally as suggested: invalidateApprovalIfNeeded now takes the per-document advisory lock as its first action. Every content-mutation path already calls it (register create/update/delete, narrative save, control add/remove, regenerate), so all edits now serialize against approve(). Either an edit is frozen into the version or it reverts the doc to draft — no interleaving does neither. The lock is transaction-scoped + re-entrant, so create paths that already lock for position allocation are unaffected.

324 API ISMS tests passing (incl. a new approval.spec.ts asserting lock-before-status ordering); typecheck clean.

…ral lock) (#3367) Follow-up to the approve-race fix: cubic correctly noted the advisory lock only protected paths that also take it, so a register UPDATE/DELETE (or narrative / control / regenerate) could still interleave while approve() loads and freezes the version, leaving an approved version that omits the edit — or an edit that never invalidated the approval. Enforced centrally instead of per-call-site: invalidateApprovalIfNeeded now takes the per-document advisory lock as its first action. Every content-mutation path already calls it (register create/update/delete, narrative save, control add/remove, regenerate), so all edits now serialize against approve() (which takes the same lock). The lock is transaction-scoped and re-entrant, so create paths that already acquire it for position allocation are unaffected. Adds utils/approval.spec.ts (lock-before-status ordering + revert/no-op behavior) and $executeRaw to the two tx mocks that exercise the real invalidate. Tests: 324 api ISMS tests passing; typecheck clean. Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

tofikwest · 2026-07-07T20:40:52Z

@cubic-dev-ai review it

cubic-dev-ai · 2026-07-07T20:45:52Z

@cubic-dev-ai review it

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

cubic-dev-ai

2 issues found across 63 files

Confidence score: 2/5

In apps/api/src/integration-platform/controllers/oauth.controller.ts, query.installation_id is trusted after state/session checks, so a tampered callback can bind an arbitrary githubInstallationId to a connection; merging as-is risks mislinked GitHub installations and downstream automation acting on the wrong tenant/account — validate the installation ID against the authenticated GitHub app/install context before persisting.
In apps/api/src/isms/utils/approval.ts (invalidateApprovalIfNeeded flow), the lock is acquired too late on some control-link mutation paths, so edits can race with approval and leave approval state inconsistent with the final links; merging now could allow stale or invalid approvals to stand — move locking/invalidating ahead of link writes (or make the mutation+invalidation atomic) on all mutation paths before merge.

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="packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql">

<violation number="1" location="packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql:41">
P2: Existing approved ISMS documents can lose their “approved version exists” signal after this migration: the migration preserves `status = 'approved'` but deletes all `IsmsDocumentVersion` rows and never sets `currentVersionId`, while the API now derives `hasApprovedVersion` and version history from those fields. It would be safer to either backfill a real published version/current pointer for approved documents or explicitly transition legacy approved documents into a state that matches the new versioning model.</violation>
</file>

<file name="apps/api/src/isms/utils/approval.ts">

<violation number="1" location="apps/api/src/isms/utils/approval.ts:27">
P2: Control-link edits can still race with approval because this lock is taken inside `invalidateApprovalIfNeeded`, but some mutation paths call that helper only after writing the link rows. In those paths, a concurrent approval can acquire the same lock and freeze a version before the edit owns the document lock, so the new central lock does not fully serialize “all edits against approve()”. Consider moving the document lock before the control-link `createMany`/`deleteMany` writes (while still invalidating only when `count > 0`).</violation>
</file>

<file name="apps/api/src/isms/isms.service.ts">

<violation number="1" location="apps/api/src/isms/isms.service.ts:230">
P1: Approval is still not serialized against register edits that update or delete existing rows. The new `lockDocument` call only blocks code paths that also acquire the advisory lock, but the register update/remove transactions do not, so an edit can interleave while approval is loading `EXPORT_DOCUMENT_INCLUDE` and freezing the version. Consider taking the same document lock in all register mutations that can change approved content, not only create paths, before publishing relies on this lock for immutability.</violation>
</file>

<file name="apps/api/src/integration-platform/controllers/oauth.controller.ts">

<violation number="1" location="apps/api/src/integration-platform/controllers/oauth.controller.ts:442">
P1: A tampered callback URL can save an arbitrary `githubInstallationId` on the connection because the value is copied directly from `query.installation_id` after only OAuth state/session validation. Since this metadata is intended to drive a future GitHub App installation-token flow, it would be safer to persist it only for `appInstallFlow` providers after validating that the installation id is one of the installations accessible to the exchanged GitHub user token/current tenant.</violation>
</file>

_{Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic}

cubic-dev-ai · 2026-07-07T20:50:50Z

  tx: Prisma.TransactionClient;
  documentId: string;
 }): Promise<void> {
+  await lockDocument(tx, documentId);


P2: Control-link edits can still race with approval because this lock is taken inside invalidateApprovalIfNeeded, but some mutation paths call that helper only after writing the link rows. In those paths, a concurrent approval can acquire the same lock and freeze a version before the edit owns the document lock, so the new central lock does not fully serialize “all edits against approve()”. Consider moving the document lock before the control-link createMany/deleteMany writes (while still invalidating only when count > 0).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/isms/utils/approval.ts, line 27: <comment>Control-link edits can still race with approval because this lock is taken inside `invalidateApprovalIfNeeded`, but some mutation paths call that helper only after writing the link rows. In those paths, a concurrent approval can acquire the same lock and freeze a version before the edit owns the document lock, so the new central lock does not fully serialize “all edits against approve()”. Consider moving the document lock before the control-link `createMany`/`deleteMany` writes (while still invalidating only when `count > 0`).</comment> <file context> @@ -14,6 +24,8 @@ export async function invalidateApprovalIfNeeded({ tx: Prisma.TransactionClient; documentId: string; }): Promise<void> { + await lockDocument(tx, documentId); + const document = await tx.ismsDocument.findUnique({ </file context>

tofikwest · 2026-07-07T20:55:04Z

ISMS control-link ordering (P2): fixed in #3369 (base main). addControls/removeControl now take the per-document lock before writing the link rows (previously the write ran before invalidateApprovalIfNeeded acquired the lock). Impact was limited since control links aren't part of the frozen version snapshot, but the mutation is now serialized against approve() like every other content edit. 325 API ISMS tests passing, incl. a lock-before-write ordering test.

GitHub-App OAuth installation_id (P1): this one is in the GitHub App integration (integration-platform/controllers/oauth.controller.ts), a separate feature from the ISMS work — not addressed here. It looks like a genuine issue (the callback's installation_id is persisted without validating it belongs to the exchanged install), and should be fixed in its own PR by validating the installation against the authenticated GitHub context. Flagging for the owner rather than bundling it into an ISMS change.

…approve (#3369) Follow-up flagged on #3364: addControls/removeControl wrote the control-link rows and only then called invalidateApprovalIfNeeded (which holds the per-document lock), so the write itself ran outside the lock — it could interleave with approve() before the lock was taken. (Impact was limited because control links are not part of the frozen version snapshot, but the mutation should still be serialized like every other content edit.) Both transactions now take lockDocument() as their first statement, before the createMany/deleteMany. The conditional invalidate is unchanged (real-change-only), and the lock is re-entrant so re-taking it there is a no-op. Adds an ordering test. Tests: 325 api ISMS tests passing; typecheck clean. Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…3370) The GitHub App install callback returns an `installation_id`, which GitHub's own docs document as not trustworthy (it can be spoofed on the callback URL). The previous code stored it on the connection metadata, but nothing reads it anywhere, so it was unused data that a future installation-token flow could later trust unsafely. Remove the storage (and its now-unused query fields + test). If GitHub App installation-token auth is added later, resolve the installation via `GET /user/installations` with the user token — which also proves the user owns it — instead of trusting the callback value. No behavior change: the integration authenticates with the OAuth user token, which is unaffected. Claude-Session: https://claude.ai/code/session_01XyttkYNbkdYRp7DBKNuFVH Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

tofikwest · 2026-07-07T21:17:03Z

@cubic-dev-ai review it

cubic-dev-ai · 2026-07-07T21:17:13Z

@cubic-dev-ai review it

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

cubic-dev-ai

No issues found across 64 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

Re-trigger cubic}

# [3.99.0](v3.98.1...v3.99.0) (2026-07-07) ### Bug Fixes * **integrations:** don't persist unverified GitHub installation_id ([#3370](#3370)) ([9c6f6fa](9c6f6fa)) * **isms:** lock before writing control links so edits serialize with approve ([#3369](#3369)) ([a21f866](a21f866)) * **isms:** production-deploy review follow-ups (CS-701) ([#3365](#3365)) ([d756d4a](d756d4a)), closes [#3364](#3364) * **isms:** serialize approve against ALL register/content edits (central lock) ([#3367](#3367)) ([5c88a3d](5c88a3d)) * **isms:** serialize approve/decline to prevent concurrent double-publish (CS-701) ([#3366](#3366)) ([b158bc9](b158bc9)), closes [#3364](#3364) ### Features * **integrations:** add read-only GitHub App integration (CS-710) ([#3363](#3363)) ([a5a9243](a5a9243)) * **isms:** document versioning and history (CS-701) ([#3361](#3361)) ([d95dc2e](d95dc2e))

claudfuen · 2026-07-07T21:27:10Z

🎉 This PR is included in version 3.99.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

github-actions Bot and others added 2 commits July 6, 2026 21:34

chore: merge release v3.98.1 back to main [skip ci]

fe4394b

github-actions Bot added prod-deploy automated-pr labels Jul 7, 2026

vercel Bot deployed to staging – app July 7, 2026 17:31 View deployment

vercel Bot deployed to staging – portal July 7, 2026 17:31 View deployment

vercel Bot deployed to staging – comp-framework-editor July 7, 2026 17:33 View deployment

cubic-dev-ai Bot reviewed Jul 7, 2026

View reviewed changes

vercel Bot temporarily deployed to staging – portal July 7, 2026 19:06 Inactive

vercel Bot deployed to staging – comp-framework-editor July 7, 2026 19:08 View deployment

vercel Bot deployed to staging – app July 7, 2026 19:09 View deployment

tofikwest mentioned this pull request Jul 7, 2026

fix(isms): production-deploy review follow-ups (CS-701) #3365

Merged

vercel Bot temporarily deployed to staging – portal July 7, 2026 19:55 Inactive

vercel Bot deployed to staging – comp-framework-editor July 7, 2026 19:56 View deployment

vercel Bot deployed to staging – app July 7, 2026 19:59 View deployment

cubic-dev-ai Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread apps/api/src/isms/isms.service.ts

tofikwest mentioned this pull request Jul 7, 2026

fix(isms): serialize approve/decline to prevent concurrent double-publish (CS-701) #3366

Merged

vercel Bot temporarily deployed to staging – portal July 7, 2026 20:21 Inactive

vercel Bot temporarily deployed to staging – app July 7, 2026 20:21 Inactive

vercel Bot deployed to staging – comp-framework-editor July 7, 2026 20:23 View deployment

cubic-dev-ai Bot reviewed Jul 7, 2026

View reviewed changes

tofikwest mentioned this pull request Jul 7, 2026

fix(isms): centrally serialize all content edits against approve (CS-701) #3367

Merged

vercel Bot temporarily deployed to staging – portal July 7, 2026 20:40 Inactive

vercel Bot temporarily deployed to staging – app July 7, 2026 20:40 Inactive

vercel Bot deployed to staging – comp-framework-editor July 7, 2026 20:41 View deployment

cubic-dev-ai Bot reviewed Jul 7, 2026

View reviewed changes

tofikwest mentioned this pull request Jul 7, 2026

fix(isms): lock control-link edits before write (CS-701) #3369

Merged

vercel Bot temporarily deployed to staging – portal July 7, 2026 20:58 Inactive

vercel Bot temporarily deployed to staging – app July 7, 2026 20:58 Inactive

vercel Bot deployed to staging – comp-framework-editor July 7, 2026 20:59 View deployment

tofikwest mentioned this pull request Jul 7, 2026

fix(integrations): don't persist unverified GitHub installation_id (CS-710) #3370

Merged

vercel Bot temporarily deployed to staging – portal July 7, 2026 21:15 Inactive

vercel Bot temporarily deployed to staging – app July 7, 2026 21:15 Inactive

vercel Bot deployed to staging – comp-framework-editor July 7, 2026 21:17 View deployment

cubic-dev-ai Bot reviewed Jul 7, 2026

View reviewed changes

tofikwest merged commit 4e9353a into release Jul 7, 2026
14 checks passed

claudfuen added the released label Jul 7, 2026

Uh oh!

Conversation

github-actions Bot commented Jul 7, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by cubic

Uh oh!

vercel Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tofikwest commented Jul 7, 2026

Uh oh!

tofikwest commented Jul 7, 2026

Uh oh!

cubic-dev-ai Bot commented Jul 7, 2026

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tofikwest commented Jul 7, 2026

Uh oh!

tofikwest commented Jul 7, 2026

Uh oh!

cubic-dev-ai Bot commented Jul 7, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tofikwest commented Jul 7, 2026

Uh oh!

tofikwest commented Jul 7, 2026

Uh oh!

cubic-dev-ai Bot commented Jul 7, 2026

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai Bot Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tofikwest commented Jul 7, 2026

Uh oh!

tofikwest commented Jul 7, 2026

Uh oh!

cubic-dev-ai Bot commented Jul 7, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

claudfuen commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Jul 7, 2026 •

edited by cubic-dev-ai Bot

Loading

vercel Bot commented Jul 7, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot Jul 7, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot Jul 7, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot Jul 7, 2026 •

edited

Loading