Vite-plus adoption#20
Open
JohannesKonings wants to merge 2 commits intomainfrom
Open
Conversation
Co-authored-by: Johannes Konings <JohannesKonings@users.noreply.github.com>
|
Cursor Agent can help with this pull request. Just |
📝 WalkthroughWalkthroughTooling and formatting overhaul: migrate Vite → vite-plus, add TanStack AI packages and vp scripts, introduce aiDevtools stub plugin and PR VP check workflow, remove separate oxfmt/oxlint configs, and apply widespread import/format cleanups across source, infra, and docs (no substantive runtime logic changes). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can customize the tone of the review comments and chat replies.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vscode/settings.json:
- Line 3: Update the OXC formatter config path: replace the current
oxc.fmt.configPath value that points to "./vite.config.ts" with a valid Oxfmt
config file (e.g., ".oxfmtrc.json" or ".oxfmtrc.jsonc") or remove the
oxc.fmt.configPath setting to allow auto-discovery; ensure the target file
contains Oxfmt keys like printWidth, tabWidth, and semi so the formatter will be
applied correctly.
In `@lib/constructs/WebappApi.ts`:
- Around line 1-10: The import of the CDK class "Function" shadows a restricted
global name; change the import to an alias (e.g., import { Function as
LambdaFunction } from 'aws-cdk-lib/aws-lambda') and then update all usages in
this file (references to Function) to the alias (LambdaFunction) so the
noShadowRestrictedNames lint rule is satisfied.
In `@src/webapp/styles.css`:
- Around line 10-11: In the font-family declaration in styles.css (the
comma-separated list containing -apple-system, BlinkMacSystemFont, 'Segoe UI',
'Roboto', 'Oxygen', 'Ubuntu', 'Cantarell', 'Fira Sans', 'Droid Sans', 'Helvetica
Neue', sans-serif), remove the unnecessary single quotes around single-word font
names (Roboto, Oxygen, Ubuntu, Cantarell) while keeping quotes for multi-word
names like 'Segoe UI' and 'Helvetica Neue' so the list complies with the
font-family-name-quotes stylelint rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c17c0bdb-aa3c-4059-a716-0b83f29613ff
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (68)
.cta.json.cursor/skills/tanstack-use-mcp/SKILL.md.oxfmtrc.json.oxlintrc.json.vscode/extensions.json.vscode/settings.jsonAGENTS.mdREADME.mdaccountSetup/bin/app.tsaccountSetup/cdk.jsonaccountSetup/lib/account-setup-stack.test.tsaccountSetup/lib/account-setup-stack.tsaccountSetup/lib/app-config.tsaccountSetup/lib/constructs/GitHubActionsOidcSetup.test.tsaccountSetup/lib/constructs/GitHubActionsOidcSetup.tscdk.jsoncomponents.jsondocs/PLAN-DB-PERSONS.mdknip.jsonlib/constructs/WebappApi.tslib/constructs/WebappAssetsDeployment.tslib/constructs/WebappDistribution.tslib/constructs/WebappServer.tspackage.jsonscripts/seed-persons.tsscripts/test-bedrock-budget.tsscripts/test-bedrock-chat.tssrc/lambda/stream-to-events.tssrc/webapp/components/SyncStatus.tsxsrc/webapp/components/persons/AddressCard.tsxsrc/webapp/components/persons/AddressForm.tsxsrc/webapp/components/persons/AddressFormModal.tsxsrc/webapp/components/persons/BankAccountCard.tsxsrc/webapp/components/persons/BankAccountForm.tsxsrc/webapp/components/persons/BankAccountFormModal.tsxsrc/webapp/components/persons/ConfirmationModal.tsxsrc/webapp/components/persons/ContactForm.tsxsrc/webapp/components/persons/ContactFormModal.tsxsrc/webapp/components/persons/ContactInfoCard.tsxsrc/webapp/components/persons/CreatePersonModal.tsxsrc/webapp/components/persons/EmploymentCard.tsxsrc/webapp/components/persons/EmploymentForm.tsxsrc/webapp/components/persons/EmploymentFormModal.tsxsrc/webapp/components/persons/PersonDetailPanel.tsxsrc/webapp/components/persons/PersonEditModal.tsxsrc/webapp/components/persons/PersonForm.tsxsrc/webapp/components/ui/badge.tsxsrc/webapp/components/ui/button.tsxsrc/webapp/data/fake-persons.tssrc/webapp/db-collections/persons.tssrc/webapp/hooks/useDbPersons.tssrc/webapp/hooks/useSseSync.tssrc/webapp/integrations/bedrock-adapter/index.tssrc/webapp/integrations/electrodb/entities.tssrc/webapp/integrations/electrodb/personsClient.tssrc/webapp/integrations/tanstack-query/root-provider.tsxsrc/webapp/lib/bedrock-budget-config.tssrc/webapp/lib/bedrock-budget.tssrc/webapp/routes/__root.tsxsrc/webapp/routes/demo/api.bedrock-budget.tssrc/webapp/routes/demo/api.tanchat.tssrc/webapp/routes/demo/db-person.tsxsrc/webapp/routes/demo/start.server-funcs.tsxsrc/webapp/routes/demo/tanchat.csssrc/webapp/routes/demo/tanchat.tsxsrc/webapp/styles.csstsconfig.jsonvite.config.ts
💤 Files with no reviewable changes (2)
- .oxfmtrc.json
- .oxlintrc.json
Comment on lines
+1
to
+10
| import { | ||
| AccessLogFormat, | ||
| EndpointType, | ||
| LambdaRestApi, | ||
| LogGroupLogDestination, | ||
| MethodLoggingLevel, | ||
| ResponseTransferMode, | ||
| } from 'aws-cdk-lib/aws-apigateway'; | ||
| import { Function } from 'aws-cdk-lib/aws-lambda'; | ||
| import { LogGroup } from 'aws-cdk-lib/aws-logs'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the lint rule and all `Function` imports/usages in this module
fd -HI 'biome*.{json,jsonc}'
rg -n "noShadowRestrictedNames" -S
rg -nP --type=ts "import\\s*\\{[^}]*\\bFunction\\b[^}]*\\}\\s*from 'aws-cdk-lib/aws-lambda'" -C2
rg -nP --type=ts "\\bwebappServer\\s*:\\s*Function\\b|\\breadonly\\s+webappApi\\b|\\bFunction\\b" lib/constructs/WebappApi.ts -C2Repository: JohannesKonings/tanstack-aws
Length of output: 1938
🏁 Script executed:
#!/bin/bash
# Check for Biome configuration
find . -type f \( -name "biome.json" -o -name "biome.jsonc" -o -name "package.json" \) | head -20
# Check package.json for biome reference
rg -l "biome" . --type jsonRepository: JohannesKonings/tanstack-aws
Length of output: 112
🏁 Script executed:
#!/bin/bash
# Check the content of Biome config to see if noShadowRestrictedNames is enabled
find . -type f -name "biome.json*" -exec cat -n {} \;Repository: JohannesKonings/tanstack-aws
Length of output: 2357
🏁 Script executed:
#!/bin/bash
# Check WebappServer.ts and WebappFunctionUrl.ts to see if Function is used as type annotation
cat -n lib/constructs/WebappServer.ts | head -40Repository: JohannesKonings/tanstack-aws
Length of output: 1805
🏁 Script executed:
#!/bin/bash
# Check WebappFunctionUrl.ts for Function usage
cat -n lib/constructs/WebappFunctionUrl.ts | head -40Repository: JohannesKonings/tanstack-aws
Length of output: 826
Alias the Function import to avoid shadowing the restricted global name.
Biome's noShadowRestrictedNames rule (enabled via recommended ruleset) flags this pattern. Import the CDK Function class under an alias instead.
♻️ Proposed fix
-import { Function } from 'aws-cdk-lib/aws-lambda';
+import { Function as LambdaFunction } from 'aws-cdk-lib/aws-lambda';
@@
type WebappApiProps = {
- webappServer: Function;
+ webappServer: LambdaFunction;
};🧰 Tools
🪛 Biome (2.4.6)
[error] 9-9: Do not shadow the global "Function" property.
(lint/suspicious/noShadowRestrictedNames)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/constructs/WebappApi.ts` around lines 1 - 10, The import of the CDK class
"Function" shadows a restricted global name; change the import to an alias
(e.g., import { Function as LambdaFunction } from 'aws-cdk-lib/aws-lambda') and
then update all usages in this file (references to Function) to the alias
(LambdaFunction) so the noShadowRestrictedNames lint rule is satisfied.
Comment on lines
+10
to
+11
| -apple-system, BlinkMacSystemFont, 'Segoe UI', 'Roboto', 'Oxygen', 'Ubuntu', 'Cantarell', | ||
| 'Fira Sans', 'Droid Sans', 'Helvetica Neue', sans-serif; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify stylelint rule/config and the affected declaration
fd -HI 'stylelint*.{json,js,cjs,mjs,yml,yaml}'
rg -n "font-family-name-quotes" -S
rg -n "Roboto|Oxygen|Ubuntu|Cantarell" src/webapp/styles.css -C2Repository: JohannesKonings/tanstack-aws
Length of output: 660
🏁 Script executed:
#!/bin/bash
# Search for stylelint config files using corrected patterns
fd -HI -e json -e js -e cjs -e mjs -e yml -e yaml stylelint
# Also check for common config file locations
fd -HI -e json -e js -e yaml '(stylelint|lint|eslint).*config'
# Check if there are any package.json files with stylelint config
fd -HI package.json -x grep -l "stylelint" {}Repository: JohannesKonings/tanstack-aws
Length of output: 89
🌐 Web query:
stylelint font-family-name-quotes rule CSS quotes single word font names
💡 Result:
font-family-name-quotescontrols when font family names must/mustn’t be quoted; it does not choose single vs double quotes. Usestring-quotesto enforce'vs"instead. [1], [2]- For single-word font names like
Arial(valid CSS identifiers), the usual “don’t quote unless needed” setups will reject quotes:"always-where-required"→font-family: Arial, sans-serif;OK, but"Arial"is a lint error. [1]"always-where-recommended"→ also allowsArialunquoted, but requires quotes for names where quotes are recommended (e.g., containing whitespace/digits/punctuation other than hyphens). [1]
- If you want single quotes when quoting is needed, configure e.g.:
{
"rules": {
"font-family-name-quotes": "always-where-recommended",
"string-quotes": "single"
}
}[1], [2]
Notes:
- Generic/system keywords like
serif,sans-serif,system-ui,monospace, etc. should never be quoted (the rule enforces that). [1] "always-unless-keyword"is the option that forces quotes even on single-word names likeArial. [1]
Sources: [1] Stylelint rule docs (font-family-name-quotes), [2] Stylelint changelog (quote-style-specific options removed; use string-quotes).
🏁 Script executed:
#!/bin/bash
# Read the stylelint configuration
cat -n .stylelintrc.jsonRepository: JohannesKonings/tanstack-aws
Length of output: 1288
Remove unnecessary quotes around single-word font family names.
'Roboto', 'Oxygen', 'Ubuntu', and 'Cantarell' violate the font-family-name-quotes rule enabled by the stylelint configuration. Single-word font names do not require quotes and must be unquoted; only multi-word names like 'Segoe UI' require quotes.
♻️ Proposed fix
- -apple-system, BlinkMacSystemFont, 'Segoe UI', 'Roboto', 'Oxygen', 'Ubuntu', 'Cantarell',
+ -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, Cantarell,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| -apple-system, BlinkMacSystemFont, 'Segoe UI', 'Roboto', 'Oxygen', 'Ubuntu', 'Cantarell', | |
| 'Fira Sans', 'Droid Sans', 'Helvetica Neue', sans-serif; | |
| -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, Cantarell, | |
| 'Fira Sans', 'Droid Sans', 'Helvetica Neue', sans-serif; |
🧰 Tools
🪛 Stylelint (17.4.0)
[error] 9-11: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 10-10: Unexpected quotes around "Roboto" (font-family-name-quotes)
(font-family-name-quotes)
[error] 10-10: Unexpected quotes around "Oxygen" (font-family-name-quotes)
(font-family-name-quotes)
[error] 10-10: Unexpected quotes around "Ubuntu" (font-family-name-quotes)
(font-family-name-quotes)
[error] 10-10: Unexpected quotes around "Cantarell" (font-family-name-quotes)
(font-family-name-quotes)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/webapp/styles.css` around lines 10 - 11, In the font-family declaration
in styles.css (the comma-separated list containing -apple-system,
BlinkMacSystemFont, 'Segoe UI', 'Roboto', 'Oxygen', 'Ubuntu', 'Cantarell', 'Fira
Sans', 'Droid Sans', 'Helvetica Neue', sans-serif), remove the unnecessary
single quotes around single-word font names (Roboto, Oxygen, Ubuntu, Cantarell)
while keeping quotes for multi-word names like 'Segoe UI' and 'Helvetica Neue'
so the list complies with the font-family-name-quotes stylelint rule.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/webapp/routes/demo/tanchat.tsx (1)
307-364:⚠️ Potential issue | 🟠 MajorPrevent stale budget responses from overwriting fresh state.
fetchBudgetcan run concurrently (initial load,RUN_FINISHED, manual retry). Older requests can resolve later and overwrite newerbudgetstate.🛠️ Proposed fix (request sequencing guard)
function ChatPage() { const [runLog, setRunLog] = useState<RunLogEntry[]>([]); const [budget, setBudget] = useState<BudgetState>({ overBudget: false, limit: DAILY_LIMIT_USD, loading: true, }); const mountedRef = useRef(true); + const budgetRequestSeqRef = useRef(0); useEffect( () => () => { mountedRef.current = false; }, [], ); const fetchBudget = useCallback(async () => { + const requestSeq = ++budgetRequestSeqRef.current; const BUDGET_TIMEOUT_MS = 20_000; setBudget((prev) => ({ ...prev, loading: true, error: undefined })); const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), BUDGET_TIMEOUT_MS); @@ - if (!mountedRef.current) { + if (!mountedRef.current || requestSeq !== budgetRequestSeqRef.current) { return; } setBudget(next); } catch (err) { clearTimeout(timeoutId); - if (!mountedRef.current) { + if (!mountedRef.current || requestSeq !== budgetRequestSeqRef.current) { return; } const message = err instanceof Error ? err.name === 'AbortError' ? 'Budget request timed out (20s)' : err.message : String(err);Also applies to: 366-368, 392-392
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/webapp/routes/demo/tanchat.tsx` around lines 307 - 364, fetchBudget can have out-of-order responses overwrite newer state; add a request-sequencing guard using a ref counter (e.g., fetchBudgetRequestIdRef) that you increment each time fetchBudget is called, capture the current id inside the async call, and only call setBudget (and return early on error paths) if the captured id matches the ref's latest value and mountedRef.current is true; apply the same sequencing check to the other budget-related callers referenced (the manual retry and RUN_FINISHED handlers around the other fetchBudget invocations) so stale responses cannot overwrite fresh state.lib/constructs/WebappDistribution.ts (1)
126-162:⚠️ Potential issue | 🟠 Major
originBehaviorKindis validated but never used, causing Function URL mode to be silently broken.The
defaultBehavioralways usesRestApiOrigin(webappServerApi)regardless of theoriginBehaviorKindparameter value. ThewebappServerFunctionUrlis never referenced in the logic. Currently, the only active call site (lib/constructs/Webapp.ts:67) uses'apiGw'mode and works correctly, but the commented-out'functionUrl'variant (lib/constructs/Webapp.ts:55) would fail silently, routing to API Gateway instead of the Function URL.Restore the origin-selection logic in
defaultBehavioror remove the'functionUrl'option fromDistributionPropsuntil it can be properly supported.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/constructs/WebappDistribution.ts` around lines 126 - 162, The defaultBehavior always constructs a RestApiOrigin and ignores the originBehaviorKind flag, so restore the conditional origin selection: in the defaultBehavior assignment inspect originBehaviorKind (or OriginBehaviorKind if that's the used identifier) and when it's 'apiGw' use RestApiOrigin(webappServerApi) with the existing properties, and when it's 'functionUrl' use FunctionUrlOrigin.withOriginAccessControl(webappServerFunctionUrl) and include the LambdaEdge entry using sigv4SignerEdgeFunction (EventType ORIGIN_REQUEST, IncludeBody: true) plus the same AllowedMethods, CachePolicy, OriginRequestPolicy, ResponseHeadersPolicy/ResponseHeadersPolicy.SECURITY_HEADERS and ViewerProtocolPolicy to match the commented logic; alternatively remove the 'functionUrl' option from DistributionProps if you prefer not to support it.lib/constructs/WebappAssetsDeployment.ts (1)
32-36:⚠️ Potential issue | 🟠 MajorMake the bootstrap KMS key lookup conditional.
The code unconditionally looks up
alias/cdk-bootstrap-keyand will fail during synthesis for any environment that doesn't have this alias (e.g., standard CDK bootstrap). The comment states this is only needed when the bucket uses a custom key, but the code doesn't honor that condition. Add an optionalbootstrapKeyAlias?: stringprop and only perform theKey.fromLookup()andgrantDecrypt()if it's provided.Suggested fix
type WebappAssetsDeploymentProps = { assetsBucket: Bucket; + bootstrapKeyAlias?: string; distribution: Distribution; }; @@ - const cdkBootstrapKey = Key.fromLookup(this, 'CdkBootstrapKey', { - aliasName: 'alias/cdk-bootstrap-key', - }); - cdkBootstrapKey.grantDecrypt(bucketDeployment.handlerRole); + if (props.bootstrapKeyAlias) { + const cdkBootstrapKey = Key.fromLookup(this, 'CdkBootstrapKey', { + aliasName: props.bootstrapKeyAlias, + }); + cdkBootstrapKey.grantDecrypt(bucketDeployment.handlerRole); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/constructs/WebappAssetsDeployment.ts` around lines 32 - 36, The code unconditionally looks up and grants decrypt on alias/cdk-bootstrap-key which breaks synth in accounts without that alias; add an optional prop bootstrapKeyAlias?: string to the WebappAssetsDeployment construct props and only call Key.fromLookup(this, 'CdkBootstrapKey', { aliasName: bootstrapKeyAlias }) and cdkBootstrapKey.grantDecrypt(bucketDeployment.handlerRole) when bootstrapKeyAlias is provided; update the constructor signature to accept props.bootstrapKeyAlias and guard the lookup/grant with an if (props.bootstrapKeyAlias) check so environments without a custom bootstrap key skip the lookup.
♻️ Duplicate comments (1)
.vscode/settings.json (1)
2-2:⚠️ Potential issue | 🟠 MajorUse an actual Oxfmt config here.
Line 2 still points
oxc.fmt.configPathatvite.config.ts, which is a Vite+ config, not an Oxfmt formatter config. That means VS Code formatting can drift fromvp fmteven when the CLI is configured correctly.For the VS Code OXC extension, what config files and schema does `oxc.fmt.configPath` support? Can it read formatter settings from a `vite.config.ts` / Vite+ config file?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vscode/settings.json at line 2, Update the VS Code setting oxc.fmt.configPath to point to a real Oxfmt formatter config instead of vite.config.ts (remove the Vite config reference); set it to your Oxfmt config filename (for example .oxfmtrc or oxfmt.config.js / oxfmt.config.json) and ensure the referenced file contains the Oxfmt formatter settings/schema the extension expects (do not expect the extension to parse Vite/Vite+ config files).
🧹 Nitpick comments (2)
src/webapp/routes/__root.tsx (1)
3-3: Scope the lint suppression to the single exception instead of the whole file.A file-wide
oxlint-disablecan hide future violations outside the intended spot.Suggested change
-// oxlint-disable func-style +// Keep global linting enabled; suppress only where needed. @@ +// oxlint-disable-next-line func-style function RootDocument({ children }: { children: React.ReactNode }) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/webapp/routes/__root.tsx` at line 3, Remove the file-wide "// oxlint-disable func-style" and instead place a targeted suppression immediately adjacent to the specific function that needs it (e.g., add "// oxlint-disable-next-line func-style" or "// oxlint-disable-line func-style" directly above/on the offending function declaration in src/webapp/routes/__root.tsx), so the rule is only disabled for that single exception rather than the entire file.package.json (1)
21-23: Keepintent installexplicit instead of running it frompostinstall.
intent.mddescribes this as a sync step that updatesAGENTS.mdwhen skill-providing dependencies change. Wiring it intopostinstallmeans every local install and thevp installstep in.github/workflows/pr-vp-check.ymlnow performs repo-file generation as a dependency-install side effect.Suggested change
"intent:install": "vp exec intent install", "intent:list": "vp exec intent list", - "postinstall": "vp exec intent install", "knip": "knip",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 21 - 23, Remove the automatic postinstall hook that runs "vp exec intent install" so repository files are not generated as a side effect of every install; edit package.json to delete or disable the "postinstall" script key and leave the explicit "intent:install" and "intent:list" scripts intact so maintainers invoke intent install manually when needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-vp-check.yml:
- Around line 3-5: The pull_request trigger's types list currently contains only
opened, reopened, and synchronize so PRs marked ready_for_review won't run the
workflow; update the pull_request.types array in the workflow (the on:
pull_request types) to include "ready_for_review" alongside "opened",
"reopened", and "synchronize" so the workflow runs when a draft PR is marked
ready for review.
In `@AGENTS.md`:
- Around line 11-17: Update the documentation entries for the npm scripts named
"compile" and "webapp:build" to match the actual scripts: change the "compile"
description to indicate it runs plain "tsc" (not "tsc --noEmit"), and change the
"webapp:build" description to mention it first runs "node
scripts/generate-robots-txt.ts" to generate robots.txt and then runs "vp build"
(Vite/VuePress build), ensuring the AGENTS.md lines referencing compile and
webapp:build reflect these exact behaviors.
- Around line 31-47: The YAML under the intent-skills block has a mis-indented
mapping for the task "configure Nitro server build and deployment behavior" —
the `load:` key for that entry must be indented to match the other skill
mappings; update the `load:` line so it has two leading spaces (i.e., align `
load:` with the other entries) within the `skills:` block to restore valid YAML
parsing.
In `@lib/constructs/WebappAssetsDeployment.ts`:
- Line 1: The code currently converts import.meta.url with new
URL(import.meta.url).pathname which preserves URL-encoding and yields incorrect
Windows paths; update WebappAssetsDeployment (and any usage that derives the
current file directory) to import { fileURLToPath } from 'url' and replace new
URL(import.meta.url).pathname with fileURLToPath(import.meta.url), then use
path.dirname(...) or path.resolve(...) as needed so Source.asset(...) receives a
proper filesystem path across platforms.
In `@vite.config.ts`:
- Around line 24-39: The current aiDevtoolsStubPlugin.config computes
useAiDevtoolsStub using "isSsrBuild === true || command === 'build'", which
wrongly applies stub aliases to client production builds; change the condition
in the config for aiDevtoolsStubPlugin so useAiDevtoolsStub is only true when
isSsrBuild === true (remove the command === 'build' clause) so that
devtoolsStubAliases are applied only for SSR builds inside the config function.
---
Outside diff comments:
In `@lib/constructs/WebappAssetsDeployment.ts`:
- Around line 32-36: The code unconditionally looks up and grants decrypt on
alias/cdk-bootstrap-key which breaks synth in accounts without that alias; add
an optional prop bootstrapKeyAlias?: string to the WebappAssetsDeployment
construct props and only call Key.fromLookup(this, 'CdkBootstrapKey', {
aliasName: bootstrapKeyAlias }) and
cdkBootstrapKey.grantDecrypt(bucketDeployment.handlerRole) when
bootstrapKeyAlias is provided; update the constructor signature to accept
props.bootstrapKeyAlias and guard the lookup/grant with an if
(props.bootstrapKeyAlias) check so environments without a custom bootstrap key
skip the lookup.
In `@lib/constructs/WebappDistribution.ts`:
- Around line 126-162: The defaultBehavior always constructs a RestApiOrigin and
ignores the originBehaviorKind flag, so restore the conditional origin
selection: in the defaultBehavior assignment inspect originBehaviorKind (or
OriginBehaviorKind if that's the used identifier) and when it's 'apiGw' use
RestApiOrigin(webappServerApi) with the existing properties, and when it's
'functionUrl' use
FunctionUrlOrigin.withOriginAccessControl(webappServerFunctionUrl) and include
the LambdaEdge entry using sigv4SignerEdgeFunction (EventType ORIGIN_REQUEST,
IncludeBody: true) plus the same AllowedMethods, CachePolicy,
OriginRequestPolicy,
ResponseHeadersPolicy/ResponseHeadersPolicy.SECURITY_HEADERS and
ViewerProtocolPolicy to match the commented logic; alternatively remove the
'functionUrl' option from DistributionProps if you prefer not to support it.
In `@src/webapp/routes/demo/tanchat.tsx`:
- Around line 307-364: fetchBudget can have out-of-order responses overwrite
newer state; add a request-sequencing guard using a ref counter (e.g.,
fetchBudgetRequestIdRef) that you increment each time fetchBudget is called,
capture the current id inside the async call, and only call setBudget (and
return early on error paths) if the captured id matches the ref's latest value
and mountedRef.current is true; apply the same sequencing check to the other
budget-related callers referenced (the manual retry and RUN_FINISHED handlers
around the other fetchBudget invocations) so stale responses cannot overwrite
fresh state.
---
Duplicate comments:
In @.vscode/settings.json:
- Line 2: Update the VS Code setting oxc.fmt.configPath to point to a real Oxfmt
formatter config instead of vite.config.ts (remove the Vite config reference);
set it to your Oxfmt config filename (for example .oxfmtrc or oxfmt.config.js /
oxfmt.config.json) and ensure the referenced file contains the Oxfmt formatter
settings/schema the extension expects (do not expect the extension to parse
Vite/Vite+ config files).
---
Nitpick comments:
In `@package.json`:
- Around line 21-23: Remove the automatic postinstall hook that runs "vp exec
intent install" so repository files are not generated as a side effect of every
install; edit package.json to delete or disable the "postinstall" script key and
leave the explicit "intent:install" and "intent:list" scripts intact so
maintainers invoke intent install manually when needed.
In `@src/webapp/routes/__root.tsx`:
- Line 3: Remove the file-wide "// oxlint-disable func-style" and instead place
a targeted suppression immediately adjacent to the specific function that needs
it (e.g., add "// oxlint-disable-next-line func-style" or "//
oxlint-disable-line func-style" directly above/on the offending function
declaration in src/webapp/routes/__root.tsx), so the rule is only disabled for
that single exception rather than the entire file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f6b3b9c-239f-43be-92cc-8ebc22facee5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
.github/workflows/pr-vp-check.yml.vscode/settings.jsonAGENTS.mdbin/app.tsintent.mdlib/constructs/Webapp.tslib/constructs/WebappAssetsDeployment.tslib/constructs/WebappDistribution.tslib/constructs/WebappServer.tspackage.jsonscripts/test-bedrock-budget.tsscripts/test-bedrock-chat.tssrc/webapp/components/SyncStatus.tsxsrc/webapp/components/example-AIAssistant.tsxsrc/webapp/components/example-GuitarRecommendation.tsxsrc/webapp/db-collections/todos.tssrc/webapp/integrations/bedrock-adapter/index.tssrc/webapp/lib/bedrock-budget.tssrc/webapp/lib/demo-store-devtools.tsxsrc/webapp/lib/demo-store.tssrc/webapp/routes/__root.tsxsrc/webapp/routes/demo/start.server-funcs.tsxsrc/webapp/routes/demo/store.tsxsrc/webapp/routes/demo/tanchat.tsxsrc/webapp/routes/example.guitars/$guitarId.tsxsrc/webapp/utils/demo.tools.tsvite.config.ts
✅ Files skipped from review due to trivial changes (3)
- lib/constructs/Webapp.ts
- src/webapp/db-collections/todos.ts
- src/webapp/lib/bedrock-budget.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/webapp/components/SyncStatus.tsx
- lib/constructs/WebappServer.ts
- scripts/test-bedrock-budget.ts
- src/webapp/routes/demo/start.server-funcs.tsx
- src/webapp/integrations/bedrock-adapter/index.ts
Comment on lines
+3
to
+5
| on: | ||
| pull_request: | ||
| types: [opened, reopened, synchronize] |
There was a problem hiding this comment.
Include ready_for_review in the trigger list.
Draft PRs that are marked ready without a fresh commit will skip this workflow entirely, so the first VP run depends on someone manually rerunning it or pushing again.
Suggested change
on:
pull_request:
- types: [opened, reopened, synchronize]
+ types: [opened, reopened, synchronize, ready_for_review]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pr-vp-check.yml around lines 3 - 5, The pull_request
trigger's types list currently contains only opened, reopened, and synchronize
so PRs marked ready_for_review won't run the workflow; update the
pull_request.types array in the workflow (the on: pull_request types) to include
"ready_for_review" alongside "opened", "reopened", and "synchronize" so the
workflow runs when a draft PR is marked ready for review.
Comment on lines
+11
to
+17
| - **Dev server**: `pnpm webapp:dev` — starts the Vite+ dev server on port 3000 | ||
| - **Lint**: `pnpm lint` (Vite+ / Oxlint) — pre-existing warnings/errors exist in the codebase | ||
| - **Format check**: `pnpm format:check` (Vite+ / Oxfmt) | ||
| - **Tests**: `pnpm test` (Vite+ / Vitest) — runs CDK infrastructure tests in `accountSetup/` | ||
| - **TypeScript check**: `pnpm compile` (tsc --noEmit) | ||
| - **Build**: `pnpm webapp:build` | ||
| - **Unified checks**: `pnpm check` (Vite+ format + lint) | ||
| - **Build**: `pnpm webapp:build` (Vite+ build, Nitro output) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that documented commands match package.json scripts
# Check the actual script definitions
echo "=== Checking package.json scripts referenced in AGENTS.md ==="
jq -r '.scripts | {
"webapp:dev": .["webapp:dev"],
"lint": .lint,
"format:check": .["format:check"],
"test": .test,
"compile": .compile,
"check": .check,
"webapp:build": .["webapp:build"]
}' package.jsonRepository: JohannesKonings/tanstack-aws
Length of output: 352
Update documentation to match actual script implementations.
Two commands have inaccurate descriptions:
- compile: Documented as
tsc --noEmit, but actual script is justtsc(missing the--noEmitflag) - webapp:build: Documented as "Vite+ build, Nitro output", but the actual script runs
node scripts/generate-robots-txt.ts && vp build(the robots.txt generation step is not mentioned)
Update AGENTS.md lines 11-17 to reflect the exact command behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` around lines 11 - 17, Update the documentation entries for the npm
scripts named "compile" and "webapp:build" to match the actual scripts: change
the "compile" description to indicate it runs plain "tsc" (not "tsc --noEmit"),
and change the "webapp:build" description to mention it first runs "node
scripts/generate-robots-txt.ts" to generate robots.txt and then runs "vp build"
(Vite/VuePress build), ensuring the AGENTS.md lines referencing compile and
webapp:build reflect these exact behaviors.
Comment on lines
+31
to
+47
| <!-- intent-skills:start --> | ||
|
|
||
| # Skill mappings - when working in these areas, load the linked skill file into context. | ||
|
|
||
| skills: | ||
|
|
||
| - task: "configure TanStack DB collections and adapters" | ||
| load: "node_modules/.pnpm/@tanstack+db@0.5.33_typescript@5.9.3/node_modules/@tanstack/db/skills/db-core/collection-setup/SKILL.md" | ||
| - task: "build TanStack DB live queries and filtering" | ||
| load: "node_modules/.pnpm/@tanstack+db@0.5.33_typescript@5.9.3/node_modules/@tanstack/db/skills/db-core/live-queries/SKILL.md" | ||
| - task: "implement optimistic DB mutations and transactions" | ||
| load: "node_modules/.pnpm/@tanstack+db@0.5.33_typescript@5.9.3/node_modules/@tanstack/db/skills/db-core/mutations-optimistic/SKILL.md" | ||
| - task: "wire up Vite devtools plugin configuration" | ||
| load: "node_modules/@tanstack/devtools-vite/skills/devtools-vite-plugin/SKILL.md" | ||
| - task: "configure Nitro server build and deployment behavior" | ||
| load: "node_modules/nitro/skills/nitro/SKILL.md" | ||
| <!-- intent-skills:end --> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that skill file paths in intent-skills block exist
echo "=== Checking if skill files exist ==="
# Check each skill file path from AGENTS.md
skill_paths=(
"node_modules/.pnpm/@tanstack+db@0.5.33_typescript@5.9.3/node_modules/@tanstack/db/skills/db-core/collection-setup/SKILL.md"
"node_modules/.pnpm/@tanstack+db@0.5.33_typescript@5.9.3/node_modules/@tanstack/db/skills/db-core/live-queries/SKILL.md"
"node_modules/.pnpm/@tanstack+db@0.5.33_typescript@5.9.3/node_modules/@tanstack/db/skills/db-core/mutations-optimistic/SKILL.md"
"node_modules/@tanstack/devtools-vite/skills/devtools-vite-plugin/SKILL.md"
"node_modules/nitro/skills/nitro/SKILL.md"
)
for path in "${skill_paths[@]}"; do
if [ -f "$path" ]; then
echo "✓ Found: $path"
else
echo "✗ Missing: $path"
# Try to find similar paths
dir=$(dirname "$path")
base=$(basename "$path")
echo " Searching for alternatives..."
fd -t f "$base" "$(dirname "$dir")" 2>/dev/null | head -3
fi
doneRepository: JohannesKonings/tanstack-aws
Length of output: 808
🏁 Script executed:
# Read AGENTS.md to verify the indentation issue and understand the context
cat -n AGENTS.md | sed -n '31,50p'Repository: JohannesKonings/tanstack-aws
Length of output: 1167
Fix indentation on line 46 in intent-skills block.
The load: key on line 46 lacks proper indentation. All other entries have 2-space indentation ( load:), but this entry has none, which will break YAML parsing:
Indentation fix
- task: "configure Nitro server build and deployment behavior"
-load: "node_modules/nitro/skills/nitro/SKILL.md"
+ load: "node_modules/nitro/skills/nitro/SKILL.md"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <!-- intent-skills:start --> | |
| # Skill mappings - when working in these areas, load the linked skill file into context. | |
| skills: | |
| - task: "configure TanStack DB collections and adapters" | |
| load: "node_modules/.pnpm/@tanstack+db@0.5.33_typescript@5.9.3/node_modules/@tanstack/db/skills/db-core/collection-setup/SKILL.md" | |
| - task: "build TanStack DB live queries and filtering" | |
| load: "node_modules/.pnpm/@tanstack+db@0.5.33_typescript@5.9.3/node_modules/@tanstack/db/skills/db-core/live-queries/SKILL.md" | |
| - task: "implement optimistic DB mutations and transactions" | |
| load: "node_modules/.pnpm/@tanstack+db@0.5.33_typescript@5.9.3/node_modules/@tanstack/db/skills/db-core/mutations-optimistic/SKILL.md" | |
| - task: "wire up Vite devtools plugin configuration" | |
| load: "node_modules/@tanstack/devtools-vite/skills/devtools-vite-plugin/SKILL.md" | |
| - task: "configure Nitro server build and deployment behavior" | |
| load: "node_modules/nitro/skills/nitro/SKILL.md" | |
| <!-- intent-skills:end --> | |
| <!-- intent-skills:start --> | |
| # Skill mappings - when working in these areas, load the linked skill file into context. | |
| skills: | |
| - task: "configure TanStack DB collections and adapters" | |
| load: "node_modules/.pnpm/@tanstack+db@0.5.33_typescript@5.9.3/node_modules/@tanstack/db/skills/db-core/collection-setup/SKILL.md" | |
| - task: "build TanStack DB live queries and filtering" | |
| load: "node_modules/.pnpm/@tanstack+db@0.5.33_typescript@5.9.3/node_modules/@tanstack/db/skills/db-core/live-queries/SKILL.md" | |
| - task: "implement optimistic DB mutations and transactions" | |
| load: "node_modules/.pnpm/@tanstack+db@0.5.33_typescript@5.9.3/node_modules/@tanstack/db/skills/db-core/mutations-optimistic/SKILL.md" | |
| - task: "wire up Vite devtools plugin configuration" | |
| load: "node_modules/@tanstack/devtools-vite/skills/devtools-vite-plugin/SKILL.md" | |
| - task: "configure Nitro server build and deployment behavior" | |
| load: "node_modules/nitro/skills/nitro/SKILL.md" | |
| <!-- intent-skills:end --> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` around lines 31 - 47, The YAML under the intent-skills block has a
mis-indented mapping for the task "configure Nitro server build and deployment
behavior" — the `load:` key for that entry must be indented to match the other
skill mappings; update the `load:` line so it has two leading spaces (i.e.,
align ` load:` with the other entries) within the `skills:` block to restore
valid YAML parsing.
| @@ -1,9 +1,9 @@ | |||
| import path from 'node:path'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file exists and read the relevant lines
cat -n lib/constructs/WebappAssetsDeployment.ts | head -40Repository: JohannesKonings/tanstack-aws
Length of output: 1620
🏁 Script executed:
# Also check if fileURLToPath is used elsewhere in the codebase
rg "fileURLToPath" --type tsRepository: JohannesKonings/tanstack-aws
Length of output: 238
Use fileURLToPath() to properly convert import.meta.url to a filesystem path.
new URL(import.meta.url).pathname leaves URL-encoded characters intact and produces /C:/...-style paths on Windows, causing Source.asset(...) to point at non-existent directories.
🛠️ Suggested fix
import path from 'node:path';
+import { fileURLToPath } from 'node:url';
@@
const sourcePath = path.join(
- path.dirname(new URL(import.meta.url).pathname),
+ path.dirname(fileURLToPath(import.meta.url)),
'../../.output/public',
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/constructs/WebappAssetsDeployment.ts` at line 1, The code currently
converts import.meta.url with new URL(import.meta.url).pathname which preserves
URL-encoding and yields incorrect Windows paths; update WebappAssetsDeployment
(and any usage that derives the current file directory) to import {
fileURLToPath } from 'url' and replace new URL(import.meta.url).pathname with
fileURLToPath(import.meta.url), then use path.dirname(...) or path.resolve(...)
as needed so Source.asset(...) receives a proper filesystem path across
platforms.
Comment on lines
+24
to
+39
| const aiDevtoolsStubPlugin = { | ||
| config(_config: unknown, { command, isSsrBuild }: { command: string; isSsrBuild?: boolean }) { | ||
| // Nitro/build pulls in @tanstack/ai-devtools-core (Solid.js) whose server build | ||
| // Lacks setStyleProperty. Use stub for SSR and for production build (Nitro phase). | ||
| const useAiDevtoolsStub = isSsrBuild === true || command === 'build'; | ||
|
|
||
| if (!useAiDevtoolsStub) { | ||
| return undefined; | ||
| } | ||
|
|
||
| return { | ||
| resolve: { | ||
| alias: devtoolsStubAliases, | ||
| }, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Vite's plugin confighook, doescommand === "build"apply to both client and SSR builds, and isisSsrBuild the correct flag for SSR-only config?
💡 Result:
-
Yes. In a Vite plugin’s
confighook,command === 'build'is true for both a normal client build (vite build) and an SSR build (vite build --ssr)—they’re both still the build command. Vite’s docs explicitly describevite buildvsvite build --ssras building different environments, not differentcommandvalues. [1] -
Yes,
isSsrBuildis the intended flag when you want SSR-build-only conditional config (i.e., distinguishvite build --ssrfromvite build). Vite documentsisSsrBuild(andisPreview) as optional flags specifically to differentiate build types. [2]
Practical implication: don’t use command === 'build' to mean “client build”; it means “some kind of build”. Use isSsrBuild === true for SSR-only config, and isSsrBuild === false (or not true) for client-only config. [2]
Sources
- Vite “Environment API for Frameworks” – “
vite buildandvite build --ssr… build the client only and ssr only environments …” [1] - Vite “Configuring Vite” – conditional config mentions
isSsrBuildto detect SSR builds [2]
Remove command === 'build' to limit stub aliases to SSR builds only.
Currently, line 28 applies the aliases to any build command, which affects both client and SSR builds. Since isSsrBuild is the correct flag to distinguish SSR builds, the condition should exclude the command check and apply stubs only when isSsrBuild === true, preventing the stubs from reaching the client-side bundle during production builds.
Suggested change
- const useAiDevtoolsStub = isSsrBuild === true || command === 'build';
+ const useAiDevtoolsStub = isSsrBuild === true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vite.config.ts` around lines 24 - 39, The current aiDevtoolsStubPlugin.config
computes useAiDevtoolsStub using "isSsrBuild === true || command === 'build'",
which wrongly applies stub aliases to client production builds; change the
condition in the config for aiDevtoolsStubPlugin so useAiDevtoolsStub is only
true when isSsrBuild === true (remove the command === 'build' clause) so that
devtoolsStubAliases are applied only for SSR builds inside the config function.
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.
Migrate repository tooling to Vite+ to centralize and streamline development workflows.
Summary by CodeRabbit
New Features
Chores
CI
Documentation