WIP: fix refresh extension not working#2170
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.
|
Here's a visual recap of what changed: Open the full interactive recap |
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🔴
Review Details
Code Review Summary
PR #2170 adds two agent-facing actions for extension tool_data: an access-controlled upsert and a scoped read/list operation, plus guidance in the extensions skill. The implementation uses parameterized SQL, ensures the extension tables exist, enforces editor/viewer access, caps individual writes at 1 MiB, and supports both PostgreSQL and SQLite conflict syntax. The overall architecture is appropriate and avoids exposing raw SQL or iframe bridge details to the agent.
Risk level: High, because this change introduces security-sensitive scoped reads and persistent data mutation.
Key Findings
- 🔴 HIGH —
extension-data-getdoes not rejectscope="org"when no organization context exists; it queries using an empty organization id instead of failing closed. - 🟡 MEDIUM — List responses are limited by row count but not aggregate size, so 1,000 individually valid 1 MiB values can create an excessively large agent result and exhaust serialization/context resources.
🧪 Browser testing: Skipped — PR only modifies backend action wiring and agent documentation, with no user-facing UI changes.
| : `AND scope = 'user' AND lower(owner_email) = ?`; | ||
| const scopeArgs = | ||
| scope === "org" | ||
| ? [orgId ?? ""] |
There was a problem hiding this comment.
🔴 Reject organization reads without organization context
When scope="org", this path passes orgId ?? "" to SQL without first requiring an organization context. Fail closed as extension-data-set does; otherwise org-scoped reads from a non-org context can silently query an invalid scope and may expose rows if empty/null organization identifiers exist.
Additional Info
Found by one code-review agent; matches the existing validation pattern in extension-data-set and extension routes.
| return { | ||
| extensionId, | ||
| collection, | ||
| scope, | ||
| count: result.rows?.length ?? 0, | ||
| items: result.rows ?? [], |
There was a problem hiding this comment.
🟡 Bound list responses by aggregate size
The list operation caps rows at 1,000 but has no aggregate response-size limit, while each stored value may be up to 1 MiB. A populated collection can therefore produce a very large agent tool result, exhausting serialization or model context resources; add a byte/character cap or return a bounded, explicitly truncated result.
Additional Info
Found by one code-review agent; based on the 1 MiB per-item write limit and 1,000-item read limit.
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🟡
Review Details
Incremental Code Review Summary
The latest commit adds the intended refresh signal after extension-data-set: it updates the extension's tools.updated_at and calls notifyExtensionChangeForResource so mounted extension consumers can re-fetch data. The access checks, parameterized SQL, and existing scope behavior remain otherwise unchanged. The two prior findings remain open and were not reposted: missing org-context validation in extension-data-get and unbounded aggregate list responses.
Risk level: High, because this PR persists agent-controlled data and coordinates access-scoped refresh behavior.
New Findings
- 🟡 MEDIUM — The
tool_dataupsert andtools.updated_atrefresh update are separate database operations. A later failure can return an error after data is persisted but before the refresh signal is recorded. - 🟡 MEDIUM — Every data item refresh fans out persistent application-state writes to all share recipients, which can exhaust the action budget for shared extensions refreshed item-by-item.
The refresh direction is sound, but the mutation and notification path should be made resilient before relying on it for regular dashboard refreshes.
🧪 Browser testing: Will run after this review (PR changes extension refresh behavior).
| await client.execute({ | ||
| sql: `UPDATE tools SET updated_at = ? WHERE id = ?`, | ||
| args: [now, extensionId], | ||
| }); |
There was a problem hiding this comment.
🟡 Make the data write and refresh update atomic
The tool_data upsert commits before tools.updated_at is updated. If this later update fails, the action returns an error after persisting the new data but without the cache-busting refresh signal, leaving mounted extensions stale. Wrap the data and refresh-record updates in one transaction and notify only after commit.
Additional Info
Found by both incremental code-review agents; severity normalized to medium by majority.
| sql: `UPDATE tools SET updated_at = ? WHERE id = ?`, | ||
| args: [now, extensionId], | ||
| }); | ||
| await notifyExtensionChangeForResource(extensionId); |
There was a problem hiding this comment.
🟡 Avoid per-recipient writes on every data refresh
Calling notifyExtensionChangeForResource for every item refresh can issue one persistent application_state upsert per share recipient. For a shared extension refreshed item-by-item, this creates unbounded write fan-out and can exhaust the action/runtime budget; coalesce or batch the durable notification instead.
Additional Info
Found by one incremental code-review agent; the helper's per-recipient fan-out makes this actionable for the documented refresh workflow.

Summary
Adds two new agent actions,
extension-data-setandextension-data-get, that let the agent directly read and write an extension'stool_datastore without going through the iframe bridge or raw SQL.Problem
Agents had no reliable way to refresh data used by an extension (e.g. a dashboard) from the agent side. The only path was routing through the sandboxed iframe bridge or raw SQL, which is not accessible/safe for agent-driven updates, making it impossible for the agent to seed or refresh data that an extension reads via
extensionData.get()at render time.Solution
Introduce two dedicated actions that wrap access-controlled reads/writes to the
tool_datatable, allowing an agent to fetch fresh data from providers and persist it directly for an extension to pick up on next load.Key Changes
extension-data-setaction: upserts an item into an extension's data store (tool_data), enforcing editor access, a 1MB payload size limit, and scope (userororg) handling with Postgres/SQLite-compatible upsert conflict clauses.extension-data-getaction: reads a single item or lists items from an extension's data store, enforcing viewer access, with support foruser,org, orallscope filtering and a configurable result limit.ensureExtensionsTables()before executing and useresolveAccessto enforce permission checks tied to the extension.SKILL.mddocumentation to describe the new agent-side extension data access pattern, including guidance that this is the correct path for agent-driven dashboard refreshes.To clone this PR locally use the Github CLI with command
gh pr checkout 2170You can tag me at @BuilderIO for anything you want me to fix or change