fix: address cubic review findings on onboarding PR#2783
Merged
Conversation
P1: Add metadata.set('policies', true) after policy fan-out so the
tracker boolean flag is set.
P1: Log batchTriggerAndWait failures in vendor/risk mitigation fan-outs
instead of silently ignoring them.
P2: Strip {{#if}}/{{/if}} markers from mixed-content nodes so template
syntax doesn't leak into rendered policies.
P2: Fix stale onboardingTriggerJobId locking publish button in
ToDoOverview.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 5 files
Confidence score: 3/5
- There is a concrete regression risk in
apps/app/src/trigger/tasks/onboarding/process-policy-template.ts: marker stripping currently removes all matching condition markers in a subtree, which can corrupt nested conditional block boundaries. - Given the issue severity (7/10) and confidence (7/10), this is more than a minor edge case and could produce user-visible template processing errors, so this carries some merge risk.
- Pay close attention to
apps/app/src/trigger/tasks/onboarding/process-policy-template.ts- nested conditional marker handling may be over-stripping and breaking block structure.
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/app/src/trigger/tasks/onboarding/process-policy-template.ts">
<violation number="1" location="apps/app/src/trigger/tasks/onboarding/process-policy-template.ts:78">
P1: Marker stripping removes all matching condition markers in the subtree, which can corrupt nested conditional block boundaries.</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.
The global regex flag in stripMarkerText would remove ALL matching
{{#if}}/{{/if}} markers in a subtree, corrupting boundaries of nested
conditional blocks. Removed the g flag so only the first occurrence
(the one that triggered the match) is stripped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Covers placeholder replacement, inline/multi-node/nested conditionals, mixed content nodes, edge cases, buildVariables, buildFlags, processTemplate. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
claudfuen
pushed a commit
that referenced
this pull request
May 7, 2026
# [3.45.0](v3.44.2...v3.45.0) (2026-05-07) ### Bug Fixes * address cubic review findings on onboarding PR ([#2783](#2783)) ([bd43e8a](bd43e8a)) * **onboarding:** handle zero-item steps in tracker ([#2785](#2785)) ([a196339](a196339)) * revert page-level hooks to useRealtimeRun to fix missing auth context ([#2780](#2780)) ([5a406f9](5a406f9)) ### Features * **frameworks:** remove is-framework-versioning-enabled feature flag ([#2781](#2781)) ([016f379](016f379)) ### Performance Improvements * **build:** skip TS in next build, reduce sentry upload, add CI typecheck ([#2782](#2782)) ([26e1667](26e1667)) * **onboarding:** optimize onboarding pipeline from ~5min to ~2min ([b14db0b](b14db0b))
Contributor
|
🎉 This PR is included in version 3.45.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.
Summary
Fixes 4 issues from the Cubic review on #2778.
P1: Missing
metadata.set('policies', true)(onboard-organization.ts)The boolean was initialized to
falsebut never flipped after policy fan-out.P1: Log
batchTriggerAndWaitfailures (generate-vendor-mitigation.ts,generate-risk-mitigation.ts)Both fan-out tasks now capture batch results and log failed run IDs. No throw — individual tasks already retry 5 times.
P2: Strip
{{#if}}/{{/if}}markers from mixed-content nodes (process-policy-template.ts)Added
stripMarkerTexthelper. When a node contains{{#if var}}alongside real text and the condition is true, the marker is now stripped before processing.P2: Stale
onboardingTriggerJobIdlocking publish button (frameworks-scores.helper.ts)The API now returns
nullforonboardingTriggerJobIdwhentriggerJobCompletedis true, preventing stale/completed job IDs from reachingToDoOverview.Issue #4 (gate polling by
shouldSubscribeToRun) was already addressed in #2780.Test plan
🤖 Generated with Claude Code
Summary by cubic
Fixes four onboarding issues from the Cubic review and adds comprehensive tests for the policy template processor. Sets the correct policy metadata, logs fan-out failures, cleans template artifacts without breaking nested conditionals, and avoids a stale job ID blocking publish.
metadata.set('policies', true)after policy fan-out.batchTriggerAndWaitin risk/vendor mitigation tasks (no throw; retries already in place).{{#if}}/{{/if}}markers from mixed-content nodes in policy templates; remove only the first matching marker so nested conditionals remain intact.nullforonboardingTriggerJobIdwhentriggerJobCompletedis true to prevent the publish button from staying locked.Written for commit da742c9. Summary will update on new commits.