Repository navigation
Bundle the Builder Code starter as a core template layered over Chat - #6936
Conversation
BuilderIO/builder-agent-native-starter turned templates/chat into a blank app with ~26 exact search/replace edits, an overlay, and a delete list on every sync, so each upstream wording change broke the starter. The finished tree now lives in core as packages/core/src/templates/fusion-starter and the mirror materializes it directly. - The template ships only the eight skills it changes. Its scaffold policy (sourceTemplate fusion-starter, additionalSourceTemplates chat) lets materialize fill in the other Chat starter skills and lets `skills update scaffold` keep the overrides; turn-into-app, turn-into-skill, workspace-conventions and self-modifying-code stay out. - rewriteNetlifyToml leaves a template's own standalone netlify.toml alone, so the starter keeps `pnpm migrate:production && pnpm db:migrate`. - push-starter-template.yml materializes fusion-starter as "app". - Scaffold E2E materializes it against local core and runs install and typecheck when core changes; a PR step warns when a Chat skill that the starter overrides changes. - guard-no-unscoped-queries no longer registers commented-out example tables as ownable.
Visual recap — generation failedRecap authentication failed — the |
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
There was a problem hiding this comment.
Builder reviewed your changes and found 4 potential issues 🔴
Review Details
Code Review Summary
This PR packages the Fusion starter as a first-class Core scaffold, adds inherited skill handling, updates template publishing and CI coverage, and preserves standalone Netlify migration commands. The overall direction is sound: keeping the starter tree in Core removes brittle textual patching, while materialization and skills tests exercise important lifecycle behavior. This is a standard-risk change because it modifies scaffold/runtime behavior and release wiring.
Key findings
- 🔴 HIGH — Fresh scaffold install fails: the package manifest uses
catalog:dependency specifiers, but its bundled pnpm workspace config defines no catalog, so standalone installation cannot resolve dependencies. - 🟡 MEDIUM — App identity mismatch: provider requests remain scoped to
chat, and agent chat remains scoped toapp, after scaffolding under a different name; this can break per-app connection grants and scope chat state incorrectly. - 🟡 MEDIUM — Netlify migrations may use pooled DB URL: the build command does not map Netlify's unpooled URL to the variable read by Drizzle config.
- 🟡 MEDIUM — Thread navigation targets a missing route: the starter accepts
threadIdand resolves it to/chat/:id, but ships no chat route.
🧪 Browser testing: Will run after this review (PR touches UI code).
| const runtime = createProviderApiRuntime({ | ||
| appId: "chat", |
There was a problem hiding this comment.
🟡 Provider requests use Chat’s app authorization scope
This runtime (and the factory options at line 42) hardcodes appId: "chat", but the scaffold rewrites the agent-chat plugin to the destination app ID and does not rewrite this action. Provider connection grants are scoped by app ID, so a generated app may fail to use its own grants or resolve connections granted to Chat instead; derive both values from the generated app identity.
There was a problem hiding this comment.
Not changing this here. actions/provider-api-request.ts now comes straight from templates/chat; the starter layer no longer carries its own copy. The hardcoded appId: "chat" is the same in every app scaffolded from Chat, so it's a Chat template issue, and fixing it in Chat would also fix the starter. Leaving it out of this PR to keep its scope to the starter.
Generated by Claude Code
The first version bundled the whole starter tree, so 46 non-skill files and every dependency pin were frozen copies of Chat that would drift. A bundled template can now declare a template-layer.json: materialize copies its base, deletes the listed paths, replaces whole files and skill folders with the layer's own, and merges the layer's package.json fields into the base's. fusion-starter keeps only the 33 files it changes plus a partial package.json, so Chat's config, UI primitives, icons, server wiring, netlify.toml and dependency bumps reach the starter on every sync. db:migrate runs through the migrate:production script, which removes the netlify rewrite exception, and the inherited-skill fill-in is gone because the base copy brings Chat's skills. The layer inherits Chat's workspace overrides. CI now also runs the starter scaffold step for templates/chat changes, the mirror triggers on them, and the PR annotation warns about any Chat file or skill the layer overrides.
samijaber
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Two checks were red on a32a060. Neither is caused by this PR:
Generated by Claude Code |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Whole-file overrides of Chat files still drifted: once the starter owned Layout.tsx or DEVELOPING.md, Chat's later edits to them never arrived. The layer now stores each change to a Chat file as a unified diff (`<file>.patch`, applied with jsdiff). Chat edits away from the patched lines flow through on every materialize; an edit to the patched lines makes the materialize fail, so the Scaffold E2E step catches it in the Chat PR that caused it. A layer file that would replace a Chat file wholesale is now an error; only new files and replaced symlinks (CLAUDE.md) are stored whole. `skills update scaffold` assembles a layer's skills the same way, Chat's skill folders plus the layer's patches, so the identical reference files no longer need copies. `pnpm template-layer expand|diff` turns a layer into an editable tree and back. The advisory override annotation is gone: a stale patch now fails the build instead.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…indings guard:template-layers applies every bundled layer to its base in a temp dir (about a second, no install) so a templates/chat edit that breaks a fusion-starter patch fails in that PR. When it does, pnpm template-layer rebase three-way merges each patched file across the old and new base, then diff turns the resolved tree back into patches. AGENTS.md points Chat editors at both, under a new starter-patch-drift friction key. Review fixes: ship an empty drizzle/migrations journal so the release migrate:production && db:migrate is a no-op on a fresh app; keep Chat's agent-chat appId in the patch so the scaffold rewrites it to the app's name; drizzle.config falls back to NETLIFY_DATABASE_URL_UNPOOLED for the direct migration connection; create's local copy goes through the same layer-aware copy as materialize, and materialize --to <ref> fetches a layer from core's template folder and its base from templates/ at the same ref.
|
Two more checks failed on 7d06ba9 (the
No fix exists for either yet. bda9cb3 re-runs both. Generated by Claude Code |
|
We've been automatically notified and are looking into it. Push a new commit to re-trigger the review, or contact support@builder.io if this keeps happening. Error ID: |
| "guard:beta-e2e-suite": "tsx scripts/guard-beta-e2e-suite.ts" | ||
| "guard:beta-e2e-suite": "tsx scripts/guard-beta-e2e-suite.ts", | ||
| "template-layer": "tsx scripts/template-layer.ts", | ||
| "guard:template-layers": "tsx scripts/template-layer.ts check" |
There was a problem hiding this comment.
this is a verification script that agents can use to verify that changes to Chat app don't break any of the diffs built on top of them
| - The Fusion starter (`packages/core/src/templates/fusion-starter`) is patches | ||
| over `templates/chat`. After editing Chat, run `pnpm guard:template-layers`. | ||
| If a patch fails, run `pnpm template-layer rebase fusion-starter --out | ||
| .tmp/fs`, resolve conflicts keeping the starter's intent, then | ||
| `pnpm template-layer diff fusion-starter --from .tmp/fs`. Never delete a | ||
| failing patch. |
There was a problem hiding this comment.
instructions so agents know how to test their Chat changes vis-a-vis the fusion-starter overrides
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The starter removes Chat's /chat/:threadId route, but its navigation hook still turned a navigate threadId into that URL and the navigate action still advertised the option. Both are gone, along with the inherited spec case that asserted it. Layer delete entries must now resolve inside the destination, so a stray ../ or absolute path in template-layer.json fails instead of deleting outside the scaffold.
|
CI failures on bdd8803:
Generated by Claude Code |
|
SSR cold-start smoke failed on bda9cb3 with "plan: FAILED — import took 3007ms (budget 3000ms)", 7 ms over Plan's budget. Clips (927 ms) and Assets (840 ms) passed in the same run. This PR doesn't touch Plan's server bundle: its only new dependency, Generated by Claude Code |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The lexical containment check let a delete entry under an in-destination symlink to an outside directory reach that directory, because rmSync follows the symlinked parent. The real path of the nearest existing parent must now also stay inside the real destination.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
SSR cold-start smoke: Plan failed at 3008 ms against its 3000 ms budget, on e64aef3 and 3007 ms on bda9cb3. This PR didn't cause it. I built Plan for Netlify locally from this branch and from its
Plan's import time sits right at the budget on Generated by Claude Code |
|
Generate + run standalone Chat: failed on 1fc3a49. This PR didn't cause it.
No fix exists yet to port. I'm re-running the failed job once. Generated by Claude Code |
|
Content DB tests: failed on 1fc3a49, but this PR didn't cause it. One Playwright case failed: This PR doesn't touch Generated by Claude Code |
|
Design canvas interaction acceptance: failed on 1fc3a49, but this PR didn't cause it. Two Design editor Playwright cases couldn't find their elements:
After that, the "Run focused Design regression cases" step hit its 10-minute timeout. The same job also failed on #6974 (job 112940307814), an unrelated PR. This PR doesn't touch Generated by Claude Code |
There was a problem hiding this comment.
Builder reviewed your changes and found 3 potential issues 🔴
Review Details
Code Review Summary
This PR packages Fusion as a Chat-derived template layer, adds patch drift checks and materialization support, and adapts the generated app shell, starter guidance, migration setup, and workspace identity. The layering approach is sound overall: unified patches preserve upstream changes outside edited regions, while the guard and materialization tests make drift visible. I classified this as high risk because template application writes filesystem content and the PR changes generated app authorization/identity behavior.
Key findings
- 🔴 HIGH — A patch can follow a symlink at its final target and write outside the scaffold destination.
- 🟡 MEDIUM — Any
tool-searchcall bypasses the app-operation guard, even when unrelated. - 🟡 MEDIUM — Common domain operations such as archiving are not recognized by the guard.
- Lower priority: the generated PWA manifest retains the generic
Appname.
Two prior findings were verified fixed and their threads resolved: symlinked-ancestor writes and workspace Fusion agent-chat identity. The existing provider authorization-scope finding remains open and was not reposted.
🧪 Browser testing: Skipped — the browser-test-planner agent is unavailable; the dev server is running.
|
|
||
| const requestText = context.requestText ?? latestUserText(context.messages); | ||
| if (!looksLikeDomainOperationRequest(requestText)) return null; | ||
| if (calledToolSearch(context.toolCalls)) return null; |
There was a problem hiding this comment.
🟡 Unrelated tool searches bypass the app-operation guard
This returns as soon as any tool-search call appears, without checking that it searched for the requested operation or found no matching action. An unrelated search therefore lets a domain-operation request proceed to a source-code handoff without the guard's intended action check.
There was a problem hiding this comment.
Not changing this in this PR. server/agent/app-operation-guard.ts is byte-identical to the guard the starter already ships on builder-agent-native-starter main; this PR moves the file into Agent-Native without changing its behavior. It's a best-effort nudge, not an authorization boundary: it retries once with expandToolSurface, and a tool-search call counts as the agent having looked for an action. Requiring the search to match the request would be a separate behavior change to the starter's agent, so it belongs in its own PR.
Generated by Claude Code
| } from "@agent-native/core/server"; | ||
|
|
||
| const DOMAIN_OPERATION_PATTERN = | ||
| /\b(create|add|make|build|update|edit|delete|remove|list|show|open|save|publish|submit|send|export|import)\b/i; |
There was a problem hiding this comment.
🟡 Common domain operations bypass the app-operation guard
The operation matcher omits common verbs such as archive, restore, and complete. Requests using those verbs can receive a source-code handoff without triggering this guard; include the operations the starter expects to route through app actions or use a less partial intent check.
There was a problem hiding this comment.
Not changing this in this PR, for the same reason as the thread above. The verb list is the starter's existing guard, carried over byte for byte. Adding archive/restore/complete would change which requests trigger a retry for every starter app. That's worth doing as its own tuning change, so this PR stays a move with no behavior change.
Generated by Claude Code
|
Slides caret navigation in macOS: failed on 394af2d during Generated by Claude Code |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
BenGirone
left a comment
BenGirone
left a comment
There was a problem hiding this comment.
Looks good!
But I would want to see an end to end experiment before we merge this. Things I'd rather see than believe:
- This results in the same or similar enough template being generated.
- Results in the builder code harness are the same or better.
Seeing as every builder-code harness project uses this template, I don't think its too big of an ask for a full demo as a sanity check validation. Could permanently mess up some projects if there's something off
samijaber
changed the title
Scaffold guidance only recognized catalog templates, so builder-code-starter apps lost templateRef, templateSource, coreVersion and shape. A layer now counts as first-party through its base, matching what starter main records today.
|
@BenGirone Here's part 1 of the end-to-end check: is the generated template the same or similar enough? I ran the mirror's command (
This check found one real regression, now fixed in 78a6c8d. Bundled layer templates weren't treated as first-party, so the starter's Part 2 is the Builder Code harness run. I'm pushing the post-sync tree to a throwaway Generated by Claude Code |
|
The experiment branch for the end-to-end clip is ready: It's starter To record the Builder Code clip, point a local ai-services at this branch instead of Generated by Claude Code |
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🟡
Review Details
Incremental Code Review Summary
This update renames the bundled layered scaffold from Fusion to Builder Code and updates its template paths, workflow labels, tests, skill policy, and metadata. The layer-materialization architecture and the previously added symlink-target containment check remain intact. I classified this as high risk because template application mutates generated filesystem trees and the PR includes authorization-sensitive behavior.
New findings
- 🟡 MEDIUM — Workspace materialization deletes the Builder Code starter’s local
internationalizationskill and does not restore it. - 🟡 MEDIUM — The unscoped-query guard’s anchored regex misses valid exported table declarations later on a line.
- Lower priority:
template-layer checkderives its path from encodedURL.pathname, failing for checkout paths with spaces or non-ASCII characters.
The open provider-scope and two app-operation-guard findings remain unchanged and were not reposted. A prior symlink patch-target issue is resolved. Some parallel review agents timed out; findings from the completed agents were reviewed and aggregated.
🧪 Browser testing: Skipped — the browser-test-planner agent is unavailable; this PR includes UI changes.
| /export\s+const\s+([a-zA-Z_$][\w$]*)\s*=\s*(?:[a-zA-Z_$][\w$]*Table|table)\s*\(\s*"([^"]+)"\s*,\s*\{/gm; | ||
| // Anchored to the line start so a commented-out example table in a | ||
| // template does not register its name as ownable repo-wide. | ||
| /^[ \t]*export\s+const\s+([a-zA-Z_$][\w$]*)\s*=\s*(?:[a-zA-Z_$][\w$]*Table|table)\s*\(\s*"([^"]+)"\s*,\s*\{/gm; |
There was a problem hiding this comment.
🟡 The table guard misses inline exported declarations
Anchoring the regex at the start of a line avoids comment examples, but it also misses valid declarations after another statement on that line, such as const x = 1; export const notes = pgTable("notes", { ...ownableColumns() }). The table then is not treated as ownable and later query-scope checks cannot catch unscoped access; strip comments before matching or parse the source and cover inline exports in a test.
There was a problem hiding this comment.
Not changing this. The anchor exists so commented-out example tables (// export const notes = …) stop registering as ownable. Without it, the starter's own drizzle/schema.ts example trips the guard. The case it misses is a second statement on the same line as an export const … = pgTable(…). oxfmt, which every app here runs, always splits those onto separate lines, so no formatted source reaches that shape. Stripping comments or parsing the file would add a parser to a line-based guard to cover input the formatter never produces.
Generated by Claude Code
…he layer script path Workspace linking pruned shared framework skills unless a template was factory, so a starter app added to a workspace lost its internationalization guidance. Template-local skills now come from one map. template-layer check resolves its directory with fileURLToPath so checkouts under paths with spaces work.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Very re-assuring! I see stuff that looks like bloat (email-branding), but we can deal with that later in the initiative. Excited to see it get cloned and used by the harness! |
The published core strips spec/test files from its bundled Chat, and npm drops symlinks, so the starter's navigate.spec.ts patch had no target there and materializing from npm threw. The layer now deletes that spec instead of patching it. template-layer check also applies every layer to a package-shaped copy of its base (no spec/test files or symlinks), using the same filter the prepack sync applies, so a patch the published package cannot satisfy fails in CI.
There was a problem hiding this comment.
Builder reviewed your changes and has a few items to flag 🟡
Review Details
Code Review Summary
This incremental update addresses the published-template materialization failure: actions/navigate.spec.ts is now a delete entry rather than a patch target, and guard:template-layers checks the layer against a package-shaped base with spec/test files and symlinks removed. I verified the prior high-severity finding is fixed and resolved its review thread. The new packaged-base check is a useful regression guard alongside the existing symlink containment checks.
Existing open findings
- Provider requests still inherit Chat’s authorization
appId; this is the same issue already covered by the open provider-scope thread, so it is not reposted. - The app-operation guard still exempts any
tool-searchcall and its operation regex still omits common domain verbs. - The anchored ownable-table regex still misses an exported declaration following another statement on the same line; the new test only exercises a declaration at line start.
I did not accept the template-catalog report as a new issue: the normal create/add-app route is intentionally not the supported entry point for this starter; direct materialization is the documented path. Risk remains High because this PR changes filesystem-mutating template materialization. No new actionable issues from this update are submitted; existing open comments remain visible. 🧪 Browser testing: Skipped — browser-test-planner is unavailable in this environment.
4 participants
Why
BuilderIO/builder-agent-native-starter (what Builder Code clones) is
templates/chatturned into a blank app by a patch: about 26 exact search/replace edits, an overlay and about 40 deletions, run on every sync. Each upstream wording change broke it: starter #409, #413, then #6872 (20 broken edits, fixed in starter #415). This replaces #6930. It keeps the starter in sync with Chat, and when a Chat change conflicts with the starter's, CI fails in that Chat PR instead of in a later starter sync.Design: a patch layer over Chat
A bundled template can declare
template-layer.jsonwith{ base, delete }.template materializethen:templates/chat)<file>.patch, a unified diff applied withdiff(jsdiff 9), to the base's<file>package.jsoninto Chat's as JSONA whole layer file that would overwrite a regular base file is an error. The only file stored whole that replaces something is
CLAUDE.md, which replaces Chat's symlink. Deletes, new files and patches must all resolve inside the scaffold, including through symlinks.Keeping it in sync with Chat:
guard:template-layers. It runs inpnpm guards, so on every PR, and takes about a second with no install: it applies every bundled layer to its base in a temp dir. The Scaffold E2E install-plus-typecheck step also runs on Chat changes.pnpm template-layer rebase builder-code-starter --out .tmp/fs. It 3-way merges each patched file across the old base (origin/main) and the new base, leaving conflict markers only where both sides changed the same lines. Resolve those, then runpnpm template-layer diff builder-code-starter --from .tmp/fsto regenerate the patches. To edit the starter itself:pnpm template-layer expand builder-code-starter --out <dir>, edit, thendiff.AGENTS.mdtells anyone editing Chat to run the guard and how to rebase, tracked by the newstarter-patch-driftfriction key.builder-code-startercontents (35 files):AGENTS.md,DESIGN.md,DEVELOPING.md,README.md,_index.tsx(blank canvas),root.tsx,Layout.tsx, the navigation actions and hook,agent-native.config.ts, the manifest, the guardedmigrate-production.ts, and theagent-chat/authplugins.appIdstays Chat's, so the scaffold rewrites it to the app's name, in standalone and workspace scaffolds alike.drizzle/migrations/meta/_journal.jsonso the releasedb:migrateis a no-op on a fresh app;server/db.ts; the app-operation guard (unchanged from the starter's current one);config.tswithhomePath: "/"; a smallpnpm-workspace.yaml; andCLAUDE.md.drizzle.config.tsusesDATABASE_URL_UNPOOLED, thenNETLIFY_DATABASE_URL_UNPOOLED, for the direct migration connection.package.json: name, description,devwithout--open,db:generate/db:migrate,migrate:productionchained withpnpm db:migrate, the Drizzle/Neon/dotenv deps,drizzle-orm0.45.2, the pnpm 10.14 pin, andscaffold.template: "builder-code-starter"Everything else is Chat's on every sync: config, UI, CSS, icons, server wiring,
netlify.toml, specs, skill references, 13 whole skills, and dependency bumps.Supporting changes:
builder-code-starterpolicy (the Chat starter set plus 3 skills;turn-into-app,turn-into-skill,workspace-conventionsandself-modifying-codeare not in it). For a layered template,skills update scaffoldbuilds the skill sources as Chat's skill folders plus the layer's patches, so it works from the published package.create's local copy uses the same layer-aware copy.materialize --to <ref>fetches the layer from core's template folder and its base fromtemplates/at the same ref.push-starter-template.ymlmaterializes--template builder-code-starter --name appand triggers ontemplates/chat/**and the layer.guard-no-unscoped-queries: the table regex is anchored to the line start, so commented-out example tables no longer register as ownable.diffis added to core dependencies.Verification
pnpm guards: all 88 checks pass.oxfmtis clean and coretsc --noEmitpasses.template-layer.spec.ts: patch, add, delete and merge; a base edit away from the patch still applying; a base edit to the patched lines failing; a whole-file override being refused; a symlink replaced safely; deletes, new files and patches refused when they would escape the scaffold through.., an absolute path or a symlink; subtree-only application--to <ref>layer download test with a fake downloaderappIdroot.tsxand the end of Chat'sDEVELOPING.md.guard:template-layersfailed and printed the fix command.rebasereported conflicts in exactly those two files.diffregenerated the patches and the guard passed.pnpm install,typecheck,buildandagent-native doctorare clean, andpnpm db:migratesucceeds on a fresh tree.skills update scaffold: the patched skills and Chat's skills are both restored correctly, 21 skills in total, and none of the 4 excluded skills appear.Rollout
template) and this PR.skills updateknows thebuilder-code-starterpolicy.sync-conflictPR. This is expected:mainholds the old patch output. Resolve it withgit checkout origin/template -- $(git ls-tree -r --name-only origin/template | grep -v '^\.github/')This was verified in a clone of starter
main(2866f5e): install,skills update, dedupe, typecheck, build and doctor pass, and the next template push merges with no conflicts.scaffold.template: "chat"and keep Chat's skills.🤖 Generated with Claude Code
https://claude.ai/code/session_01GM6TRjcTQFm6kUVaAeSgzh