fix(tables): prevent truncated sandbox mounts - #7075
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThis follow-up completes the table-mount fix by routing every table through a complete versioned snapshot and distinguishing stale snapshots from snapshots with incomplete provenance.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/copilot/tools/handlers/function-execute.ts | Routes every table mount through a complete snapshot, rejects stale snapshots, and accounts for snapshot bytes in cloud and local mount limits. |
| apps/sim/lib/table/rows/secret-provenance.ts | Replaces boolean snapshot safety with explicit safe, unsafe-provenance, and stale classifications using rows-version checks around the provenance query. |
| apps/sim/lib/copilot/tools/handlers/function-execute.test.ts | Updates mount coverage for universal snapshots, stale rejection, provenance handling, and aggregate size limits. |
| apps/sim/lib/table/rows/secret-provenance.test.ts | Covers all three snapshot-safety classifications, including a version change during provenance resolution. |
| apps/sim/lib/core/config/feature-flags.ts | Removes the fully rolled-out table snapshot feature-flag registration. |
| apps/sim/lib/core/config/env.ts | Removes the obsolete environment fallback for the snapshot feature flag. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Resolve input table] --> B[Create or reuse versioned CSV snapshot]
B --> C[Check rows version and provenance]
C -->|Stale| D[Reject and request retry]
C -->|Unsafe provenance| E[Mark provenance incomplete]
C -->|Safe| F[Continue]
E --> F
F --> G{Cloud storage?}
G -->|Yes| H[Check URL mount limits]
H --> I[Mount by presigned URL]
G -->|No| J[Check buffered mount limits]
J --> K[Download and mount snapshot content]
Reviews (5): Last reviewed commit: "fix(tables): reject stale sandbox snapsh..." | Re-trigger Greptile
There was a problem hiding this comment.
1 issue found across 4 files
Confidence score: 3/5
- In
apps/sim/lib/copilot/tools/handlers/function-execute.test.ts, the inline path still truncates query results to a partial page when the data exceeds the 5MB budget, becausequeryRowscuts atpageCutBytes; this means the test could pass while real execution silently drops rows. Add a case that verifies the complete result set is returned or that pagination is explicitly handled.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/lib/copilot/tools/handlers/function-execute.test.ts">
<violation number="1" location="apps/sim/lib/copilot/tools/handlers/function-execute.test.ts:581">
P2: The inline path still truncates: a table with at most `DEFAULT_QUERY_LIMIT` rows that exceeds the 5MB query-result budget is returned as a partial page by `queryRows` (bounded pages cut at `pageCutBytes`), and `executeFunctionExecute` mounts it without checking `nextCursor`. The new standalone test asserts 'without truncation' only by setting `rowCount` to the row limit, so it never exercises this byte-budget truncation for the very bug the PR fixes. Route the inline mount by byte budget too (or fail closed on a non-null `nextCursor`), and add a test for a small-row-count, high-byte table.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
a50a27a to
c04a28c
Compare
|
@cursor review |
|
Skipping Bugbot: Bugbot is disabled for this repository. Visit the Bugbot dashboard to update your settings. |
|
@cursor review |
|
Skipping Bugbot: Bugbot is disabled for this repository. Visit the Bugbot dashboard to update your settings. |
|
@cursor review |
|
Skipping Bugbot: Bugbot is disabled for this repository. Visit the Bugbot dashboard to update your settings. |
Summary
Type of Change
Testing
bun run lintChecklist