Add creative context and framework workflow updates#2190
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
# Conflicts: # templates/content/actions/create-document.test.ts # templates/content/actions/create-document.ts # templates/content/actions/edit-document.ts # templates/content/actions/update-document.ts
|
One additional packaging concern not covered by the existing threads:
Please set the peer lower bound to the first core release that ships these subpaths (or otherwise enforce the compatible core pairing), and add a package-install/compatibility check so the published metadata cannot drift from the runtime requirement. |
|
Another concrete data-integrity issue in the Figma resync path:
Impact: a normal Figma resync can fail to persist the new version as soon as it contains an unchanged embedded asset. Please make the persisted media row identity version-aware (while keeping blob identity stable), or include the source version in the row ID and add a resync test covering unchanged assets. |
|
Here's a visual recap of what changed: Open the full interactive recap
|
Review notesReviewed the full 612-file diff against merge-base. Everything below was verified by reading the code — and in three cases by executing it — rather than inferred from the diff. There's a "verified clean" section at the end so the checks that passed are on record and don't get re-litigated. Blocking-ish1.
So installing 2. The new resolver's own comment says the remote-tag lookup exists so "a reset workflow counter can never move the stable channel backwards." It cannot do that: for (const line of tags.split("\\n")) {Through the YAML block scalar → shell single quotes → Ran the exact script with a stubbed tag list ( It reduces to Should-fix3. content — a membership list is used as a write-authorization gate
Three write paths gate on
An org 4. content — migration v74 deletes rows at boot (additive-only rule)
The survivor is 5. content — dead guard produces phantom workspace pages
const accessibleIds = new Set(spaces.map((space) => space.id));
for (const [index, space] of spaces.entries()) {
if (!accessibleIds.has(space.id)) continue; // can never be trueThe set is built from the same array it guards, so the 6. plan — the new destructive-collapse guard misses
7. clips — the idempotent-retry shortcut can report a good upload as "incomplete"
If the final chunk POST succeeds server-side but the client never sees the response (the exact case this PR hardens) and retries, a missing/failed app-state read returns 8. creative-context — a stale
9. core — newly-public ingestion API is unbounded Both ship as public API here (
10. analytics — bar charts lose date gap-filling at every grain
const pivoted = pivotRows(rawRows, panel.config.pivot, {
fillDateGaps: panel.chartType !== "bar",
});Added for the new weekly-bucketed panel, but it applies to all bar+pivot charts. A daily-grain bar+pivot chart (e.g. a week with 5 zero-signup days) previously got zero-value rows from 11. analytics — duplicate panel ids can be saved, orphaning a panel forever Filters are deduped by id ( 12. core client — new agent tabs hardcode English
13. core client — ambient resource-context ownership collision
Nits
Verified clean (recording so these don't get re-litigated)
One process noteAt 612 files / ~62k additions, this bundles a brand-new public package, a desktop release-channel change, and 12 template surfaces into a single revert unit. Both blocking-ish findings sit in the packaging/release-plumbing slice — the part least exercised by CI and hardest to see in a diff this size. That slice might be worth splitting out and landing on its own. |
|
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: |
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🟡
Review Details
Code Review Summary
This incremental review covers the latest fix batch in PR #2190. The five previously-open findings were rechecked and are fixed: context jobs now apply organization scope, OAuth callbacks no longer add a revocation-bypassing app allow-list entry, Google Drive preserves incremental scopes, the Picker Promise has a timeout, and SQL validation now covers metadata/media text. Those stale review threads were resolved.
The new access-control and storage-boundary hardening is directionally sound, with focused tests added around A2A access, design context access, provider OAuth, and normalization. I found two remaining storage-boundary gaps in the expanded validation: provenance is still serialized directly into SQL without a size check, and media locator fields can carry arbitrarily large values. Both can bypass the intended raw-payload/blob-storage boundary.
Key Findings
- 🟡 MEDIUM — Bound item provenance before SQL persistence.
- 🟡 MEDIUM — Bound or reject oversized media locator fields before SQL persistence.
Risk classification remains High because this PR spans OAuth, tenant authorization, public agent access, and persistent data handling.
🧪 Browser testing: Could not verify — the dev server was healthy, but all 17 planned cases were blocked because browser-testing executors had no Chrome/navigation tools available.
| export function assertContextItemSqlTextLimits( | ||
| item: Pick< | ||
| NormalizedContextItem, | ||
| | "content" | ||
| | "summary" | ||
| | "mimeType" | ||
| | "metadata" | ||
| | "chunks" | ||
| | "media" | ||
| | "edges" |
There was a problem hiding this comment.
🟡 Apply the SQL payload limit to item provenance
assertContextItemSqlTextLimits now checks metadata, chunk/media/edge fields, and text limits, but it still omits NormalizedContextItem.provenance. The ingest path serializes provenance directly into the SQL provenance column, so an importer can place an oversized raw/provider payload there and bypass the storage/blob boundary. Include provenance in the validation and apply the JSON byte limit (or move oversized provenance to private blob storage).
Additional Info
Reported by 1 of 3 review agents; verified in the current normalization and ingest paths.
There was a problem hiding this comment.
Fixed in 810a3b6. The SQL guard now applies the structured metadata limit to item provenance as well, so oversized provider payloads fail closed before provenance is persisted.
| "move raw payloads to private blob storage before ingest", | ||
| ); | ||
| } | ||
| for (const media of item.media ?? []) { |
There was a problem hiding this comment.
🟡 Bound media locator fields before SQL persistence
The expanded media validation bounds OCR/caption/alt text and metadata, but not url, storageKey, or provenanceUrl. These values are persisted by the ingest path, so an oversized locator (including a data: URL) can still bypass the SQL/blob storage boundary. Apply a byte limit to persisted locators and reject inline data URLs rather than storing them.
Additional Info
Reported by 1 of 3 review agents; verified in the current normalization and ingest paths.
There was a problem hiding this comment.
Fixed in 810a3b6. Media URL, storage-key, and provenance-URL locators now have byte limits, and inline data URLs are rejected before SQL persistence.

Summary
@agent-native/creative-contextpackage and integrate it with the relevant templatesValidation
corepack pnpm run guardscorepack pnpm --filter @agent-native/core buildcorepack pnpm --filter @agent-native/creative-context buildpreptypecheck passed; repo-wide test concurrency exposed dispatch/docs timeout and environment-only errors, which passed in focused reruns