Conversation
…hen new versions come out [dev] [Marfuen] mariano/cs-273-framework-versioning
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
There was a problem hiding this comment.
6 issues found across 98 files
Note: This PR contains a large number of files. During the trial, cubic reviews up to 50 files per PR. Paid plans get a higher limit.
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/controls/controls.service.ts">
<violation number="1" location="apps/api/src/controls/controls.service.ts:437">
P1: Policy queries still omit `isArchived: false` in modified paths, so user-archived policies can be linked or returned despite the new archival rule.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/page.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/page.tsx:19">
P2: Avoid calling `/update-status` unconditionally here; it bypasses the feature-flag gate and adds an unnecessary API request on every framework page load.</violation>
</file>
<file name="apps/framework-editor/app/(pages)/frameworks/[frameworkId]/versions/hooks/useFrameworkDraftDiff.ts">
<violation number="1" location="apps/framework-editor/app/(pages)/frameworks/[frameworkId]/versions/hooks/useFrameworkDraftDiff.ts:100">
P2: Add stale-response protection to this async fetch; overlapping requests can overwrite state with older draft diff data.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/components/SyncConfirmDialog.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/components/SyncConfirmDialog.tsx:112">
P3: The preserved-edits summary only counts control edits; include preserved task/policy edits too so confirmation text matches the actual preview.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/review-update/page.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/review-update/page.tsx:25">
P2: This redirect condition conflates API failures with the “no update available” case. Non-2xx errors are currently masked as a normal redirect instead of being handled as errors.</violation>
</file>
<file name="apps/api/src/policies/policies.controller.ts">
<violation number="1" location="apps/api/src/policies/policies.controller.ts:184">
P1: Archive filtering is only applied to non-version PDF paths; providing `versionId` can bypass the new archived-policy guard.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
|
|
||
| const policies = await db.policy.findMany({ | ||
| where: { id: { in: policyIds }, organizationId }, | ||
| where: { id: { in: policyIds }, organizationId, archivedAt: null }, |
There was a problem hiding this comment.
P1: Policy queries still omit isArchived: false in modified paths, so user-archived policies can be linked or returned despite the new archival rule.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/controls/controls.service.ts, line 437:
<comment>Policy queries still omit `isArchived: false` in modified paths, so user-archived policies can be linked or returned despite the new archival rule.</comment>
<file context>
@@ -426,7 +434,7 @@ export class ControlsService {
const policies = await db.policy.findMany({
- where: { id: { in: policyIds }, organizationId },
+ where: { id: { in: policyIds }, organizationId, archivedAt: null },
select: { id: true },
});
</file context>
| @@ -181,14 +181,14 @@ export class PoliciesController { | |||
| ) { | |||
There was a problem hiding this comment.
P1: Archive filtering is only applied to non-version PDF paths; providing versionId can bypass the new archived-policy guard.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/policies/policies.controller.ts, line 184:
<comment>Archive filtering is only applied to non-version PDF paths; providing `versionId` can bypass the new archived-policy guard.</comment>
<file context>
@@ -181,14 +181,14 @@ export class PoliciesController {
const [policy, allControls] = await Promise.all([
db.policy.findFirst({
- where: { id, organizationId },
+ where: { id, organizationId, archivedAt: null },
select: {
id: true,
</file context>
| const [frameworkRes, updateStatusRes] = await Promise.all([ | ||
| serverApi.get<any>(`/v1/frameworks/${frameworkInstanceId}`), | ||
| serverApi.get<{ data: FrameworkUpdateStatus }>( | ||
| `/v1/frameworks/${frameworkInstanceId}/update-status`, |
There was a problem hiding this comment.
P2: Avoid calling /update-status unconditionally here; it bypasses the feature-flag gate and adds an unnecessary API request on every framework page load.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/page.tsx, line 19:
<comment>Avoid calling `/update-status` unconditionally here; it bypasses the feature-flag gate and adds an unnecessary API request on every framework page load.</comment>
<file context>
@@ -16,45 +13,23 @@ interface PageProps {
+ const [frameworkRes, updateStatusRes] = await Promise.all([
+ serverApi.get<any>(`/v1/frameworks/${frameworkInstanceId}`),
+ serverApi.get<{ data: FrameworkUpdateStatus }>(
+ `/v1/frameworks/${frameworkInstanceId}/update-status`,
+ ),
+ ]);
</file context>
| const result = await apiClient<{ data: DraftDiff }>( | ||
| `/framework/${frameworkId}/draft-diff`, | ||
| ); | ||
| setData(result?.data); |
There was a problem hiding this comment.
P2: Add stale-response protection to this async fetch; overlapping requests can overwrite state with older draft diff data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/framework-editor/app/(pages)/frameworks/[frameworkId]/versions/hooks/useFrameworkDraftDiff.ts, line 100:
<comment>Add stale-response protection to this async fetch; overlapping requests can overwrite state with older draft diff data.</comment>
<file context>
@@ -0,0 +1,113 @@
+ const result = await apiClient<{ data: DraftDiff }>(
+ `/framework/${frameworkId}/draft-diff`,
+ );
+ setData(result?.data);
+ } catch (err) {
+ setError(err instanceof Error ? err : new Error('Failed to fetch draft diff'));
</file context>
| ]); | ||
|
|
||
| // No update available (latest version matches current) → bounce back. | ||
| if (!frameworkRes.data || !previewRes.data?.data) { |
There was a problem hiding this comment.
P2: This redirect condition conflates API failures with the “no update available” case. Non-2xx errors are currently masked as a normal redirect instead of being handled as errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/review-update/page.tsx, line 25:
<comment>This redirect condition conflates API failures with the “no update available” case. Non-2xx errors are currently masked as a normal redirect instead of being handled as errors.</comment>
<file context>
@@ -0,0 +1,42 @@
+ ]);
+
+ // No update available (latest version matches current) → bounce back.
+ if (!frameworkRes.data || !previewRes.data?.data) {
+ redirect(`/${orgId}/frameworks/${frameworkInstanceId}`);
+ }
</file context>
| {linkChanges !== 1 ? 's' : ''} will be rewired | ||
| </Text> | ||
| )} | ||
| {preview.controls.updatedPreserved.length > 0 && ( |
There was a problem hiding this comment.
P3: The preserved-edits summary only counts control edits; include preserved task/policy edits too so confirmation text matches the actual preview.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/components/SyncConfirmDialog.tsx, line 112:
<comment>The preserved-edits summary only counts control edits; include preserved task/policy edits too so confirmation text matches the actual preview.</comment>
<file context>
@@ -0,0 +1,130 @@
+ {linkChanges !== 1 ? 's' : ''} will be rewired
+ </Text>
+ )}
+ {preview.controls.updatedPreserved.length > 0 && (
+ <Text size="sm" variant="muted">
+ {preview.controls.updatedPreserved.length} control edit
</file context>
packages/db/tsconfig only excluded *.test.ts, but the framework-versioning backfill spec is named *.spec.ts (and imports from bun:test). tsc picked it up during the API's Docker image build and failed with TS2307 on bun:test. Verified locally by running `docker build -f apps/api/Dockerfile.multistage --target builder` through to completion. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(db): exclude *.spec.ts from tsc build packages/db/tsconfig only excluded *.test.ts, but the framework-versioning backfill spec is named *.spec.ts (and imports from bun:test). tsc picked it up during the API's Docker image build and failed with TS2307 on bun:test. Verified locally by running `docker build -f apps/api/Dockerfile.multistage --target builder` through to completion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(frameworks): make M:N link reversal idempotent in sync + rollback Rollback on staging failed with Prisma P2025: "No 'Control' record was found for a disconnect on many-to-many relation 'ControlToPolicy'". Prisma 7's implicit-M:N disconnect is strict — it throws if the edge isn't there for any reason (sync recorded `connected` for a no-op connect, a manual edit removed the edge between sync and rollback, etc.). One bad edge fails the whole transaction, breaking the 100%-reliable-undo guarantee we promise customers. Switch both sync-apply and rollback's replayUndo to operate on the _ControlToPolicy / _ControlToTask junction tables directly: - INSERT ... ON CONFLICT (A, B) DO NOTHING for connects - DELETE ... WHERE (A, B) IN (...) for disconnects Same raw-SQL pattern main's initialize-organization.ts uses for its bulk edge inserts. Naturally idempotent — tolerant of pre-existing edges on insert, tolerant of missing edges on delete. Also adds a minimal @db mock in framework-sync-apply.spec.ts so Prisma.sql / Prisma.join runtime accesses don't trigger real PrismaClient init in tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Mariano <marfuen98@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 3.31.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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 end-to-end framework versioning with publish, diff, sync, and rollback, plus UI to review and apply updates. Also fixes rollback reliability by making policy/task link reversal idempotent, and keeps Docker builds green by excluding
*.spec.tsinpackages/db/tsconfig.json.New Features
archivedAt IS NULL) for controls, tasks, policies, and requirement maps.Migration
FrameworkVersion,FrameworkSyncOperation, andarchivedAtcolumns; pins instances viacurrentVersionId).bun -w packages/db run backfill:framework-versions(data migration is included for prod).is-framework-versioning-enabledflag for target orgs to expose the new UI.Written for commit 3ae8f5e. Summary will update on new commits.