-
Notifications
You must be signed in to change notification settings - Fork 31
feat: 🎸 atmn v2 #60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: 🎸 atmn v2 #60
Conversation
|
Skipped: This PR changes more files than the configured file change limit: ( |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 issues found across 363 files
Confidence score: 3/5
- Customer counts from
useNukeData.tscan silently cap at 100 becausefetchCustomersignores the API’stotal, so larger datasets will display the wrong tally. - String interpolation in
planBuilder.tsleavesplan.nameandplan.groupunescaped, which will break generated plans containing apostrophes. useNuke.ts’supdateElapseduses a stalestartTime, so elapsed timing never updates until a rerender, undermining the hook’s purpose.- Pay close attention to
atmn/src/lib/hooks/useNukeData.ts,atmn/source/core/builders/planBuilder.ts,atmn/src/lib/hooks/useNuke.ts– these are where the correctness and syntax issues occur.
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="atmn/src/lib/hooks/useNukeData.ts">
<violation number="1" location="atmn/src/lib/hooks/useNukeData.ts:41">
P1: Customer count may be incorrect. `fetchCustomers` has a default limit of 100 and only returns the array (discarding the `total` from the API response). If there are more than 100 customers, `customers.length` will show at most 100 instead of the actual count. For a nuke/deletion UI, this could mislead users about how much data will be deleted.
Consider either:
1. Modifying `fetchCustomers` to return the full response including `total`
2. Or making a separate API call that returns just the count</violation>
</file>
<file name="atmn/source/core/builders/planBuilder.ts">
<violation number="1" location="atmn/source/core/builders/planBuilder.ts:52">
P2: Inconsistent string escaping: `plan.group` is not escaped for single quotes like `plan.description` is. This will cause syntax errors in generated code if the group name contains apostrophes.</violation>
<violation number="2" location="atmn/source/core/builders/planBuilder.ts:68">
P1: Inconsistent string escaping: `plan.description` is escaped for single quotes, but `plan.name` is not. If the plan name contains an apostrophe (e.g., "Team's Plan"), the generated code will have a syntax error.</violation>
</file>
<file name="atmn/src/lib/hooks/useNuke.ts">
<violation number="1" location="atmn/src/lib/hooks/useNuke.ts:58">
P1: Stale closure bug: `updateElapsed` captures `startTime` from the initial render when it's `0`. Since `setStartTime(Date.now())` schedules an async state update, the condition `if (startTime > 0)` will always be false when `updatePhase` calls `updateElapsed`. The `totalElapsed` value will never update.
Use a ref instead of state for `startTime` to avoid the stale closure:</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| return { | ||
| orgName: org.name, | ||
| orgSlug: org.slug, | ||
| customersCount: customers.length, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Customer count may be incorrect. fetchCustomers has a default limit of 100 and only returns the array (discarding the total from the API response). If there are more than 100 customers, customers.length will show at most 100 instead of the actual count. For a nuke/deletion UI, this could mislead users about how much data will be deleted.
Consider either:
- Modifying
fetchCustomersto return the full response includingtotal - Or making a separate API call that returns just the count
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At atmn/src/lib/hooks/useNukeData.ts, line 41:
<comment>Customer count may be incorrect. `fetchCustomers` has a default limit of 100 and only returns the array (discarding the `total` from the API response). If there are more than 100 customers, `customers.length` will show at most 100 instead of the actual count. For a nuke/deletion UI, this could mislead users about how much data will be deleted.
Consider either:
1. Modifying `fetchCustomers` to return the full response including `total`
2. Or making a separate API call that returns just the count</comment>
<file context>
@@ -0,0 +1,49 @@
+ return {
+ orgName: org.name,
+ orgSlug: org.slug,
+ customersCount: customers.length,
+ plansCount: plans.length,
+ featuresCount: features.length,
</file context>
| const snippet = ` | ||
| export const ${idToVar({ id: plan.id, prefix: "plan" })} = plan({ | ||
| id: '${plan.id}', | ||
| name: '${plan.name}',${descriptionStr}${groupStr}${addOnStr}${autoEnableStr}${priceStr} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Inconsistent string escaping: plan.description is escaped for single quotes, but plan.name is not. If the plan name contains an apostrophe (e.g., "Team's Plan"), the generated code will have a syntax error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At atmn/source/core/builders/planBuilder.ts, line 68:
<comment>Inconsistent string escaping: `plan.description` is escaped for single quotes, but `plan.name` is not. If the plan name contains an apostrophe (e.g., "Team's Plan"), the generated code will have a syntax error.</comment>
<file context>
@@ -0,0 +1,255 @@
+ const snippet = `
+export const ${idToVar({ id: plan.id, prefix: "plan" })} = plan({
+ id: '${plan.id}',
+ name: '${plan.name}',${descriptionStr}${groupStr}${addOnStr}${autoEnableStr}${priceStr}
+ features: [
+ ${planFeaturesStr}
</file context>
| name: '${plan.name}',${descriptionStr}${groupStr}${addOnStr}${autoEnableStr}${priceStr} | |
| name: '${plan.name.replace(/'/g, "\\'")}',${descriptionStr}${groupStr}${addOnStr}${autoEnableStr}${priceStr} |
| const [activePhase, setActivePhase] = useState< | ||
| "customers" | "plans" | "features" | null | ||
| >(null); | ||
| const [startTime, setStartTime] = useState<number>(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Stale closure bug: updateElapsed captures startTime from the initial render when it's 0. Since setStartTime(Date.now()) schedules an async state update, the condition if (startTime > 0) will always be false when updatePhase calls updateElapsed. The totalElapsed value will never update.
Use a ref instead of state for startTime to avoid the stale closure:
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At atmn/src/lib/hooks/useNuke.ts, line 58:
<comment>Stale closure bug: `updateElapsed` captures `startTime` from the initial render when it's `0`. Since `setStartTime(Date.now())` schedules an async state update, the condition `if (startTime > 0)` will always be false when `updatePhase` calls `updateElapsed`. The `totalElapsed` value will never update.
Use a ref instead of state for `startTime` to avoid the stale closure:</comment>
<file context>
@@ -0,0 +1,187 @@
+ const [activePhase, setActivePhase] = useState<
+ "customers" | "plans" | "features" | null
+ >(null);
+ const [startTime, setStartTime] = useState<number>(0);
+ const [currentElapsed, setCurrentElapsed] = useState<number>(0);
+
</file context>
| plan.group && plan.group !== "" && plan.group !== null | ||
| ? `\n group: '${plan.group}',` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Inconsistent string escaping: plan.group is not escaped for single quotes like plan.description is. This will cause syntax errors in generated code if the group name contains apostrophes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At atmn/source/core/builders/planBuilder.ts, line 52:
<comment>Inconsistent string escaping: `plan.group` is not escaped for single quotes like `plan.description` is. This will cause syntax errors in generated code if the group name contains apostrophes.</comment>
<file context>
@@ -0,0 +1,255 @@
+ : "";
+
+ const groupStr =
+ plan.group && plan.group !== "" && plan.group !== null
+ ? `\n group: '${plan.group}',`
+ : "";
</file context>
| plan.group && plan.group !== "" && plan.group !== null | |
| ? `\n group: '${plan.group}',` | |
| plan.group && plan.group !== "" && plan.group !== null | |
| ? `\n group: '${plan.group.replace(/'/g, "\\'")}'` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 21 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="atmn/src/views/react/init/steps/HandoffStep.tsx">
<violation number="1" location="atmn/src/views/react/init/steps/HandoffStep.tsx:50">
P1: The app will exit after 100ms even when guide creation fails. In the original code, `exit()` was only called on success. Now it's called unconditionally, preventing users from seeing error messages. Consider checking `guidesState` before calling `exit()`, or have `create()` return a success indicator.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| setState("creating"); | ||
| await create(hasPricing); | ||
| setState("complete"); | ||
| setTimeout(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: The app will exit after 100ms even when guide creation fails. In the original code, exit() was only called on success. Now it's called unconditionally, preventing users from seeing error messages. Consider checking guidesState before calling exit(), or have create() return a success indicator.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At atmn/src/views/react/init/steps/HandoffStep.tsx, line 50:
<comment>The app will exit after 100ms even when guide creation fails. In the original code, `exit()` was only called on success. Now it's called unconditionally, preventing users from seeing error messages. Consider checking `guidesState` before calling `exit()`, or have `create()` return a success indicator.</comment>
<file context>
@@ -47,58 +45,11 @@ export function HandoffStep({
- }
+ await create(hasPricing);
+ setState("complete");
+ setTimeout(() => {
+ exit();
+ }, 100);
</file context>
Summary by cubic
Introduces atmn v2 with a plans-based config, OAuth login, a new pull that generates autumn.config.ts (and optional SDK types), and a revamped push with interactive/headless flows. Upgrades to API v2 and adds a safer, UI-powered CLI.
New Features
Migration
Written for commit 6a47dee. Summary will update on new commits.