Skip to content

fix(executor): load permission config once per run and retry transient reads - #7847

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/permission-config-per-run
Sep 15, 2026
Merged

waleedlatif1 merged 2 commits into
stagingfrom
fix/permission-config-per-run

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Every block runs on a shallow copy of the execution context, so the permission-config memo written onto it was thrown away and the full permission-group config (workspace, org owner, billing state, subscription, group) reloaded from the database before every block
  • Memoize the in-flight load in a run-scoped map shared by every block copy, keyed by governed subject + workspace. Parallel branches share one load, failed loads are evicted, child workflows get their own map
  • Move the bounded transient-read retry (3 attempts, 25-100ms backoff, connection errors only, still fails closed) from the tool-only wrapper into the shared loader. The block-level gate that actually loads first had no retry, so one dropped query failed the whole run
  • Block errors no longer surface a raw database query error (SQL + bound params); the message is replaced like the tool path already does, and only the redacted cause is logged
  • Redact bound params in the execution failure cause log
  • Behavior note: a permission group edit made mid-run now applies from the next run instead of the next block, which matches the original memo design

Type of Change

  • Bug fix

Testing

  • New tests: cache survives per-block copies, concurrent loads coalesce, per-workspace keys, failed loads evicted, transient retry, no retry on SQL errors, fail-closed on exhaustion, cancellation, no memo on unseeded contexts, root context seeds the cache, block error hides SQL/params
  • Verified the new tests fail against the old code
  • Affected vitest suites, type-check, lint, check:audits all pass

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…t reads

Every block ran on a shallow copy of the execution context, so the permission-config memo written onto it was discarded and the full permission-group config reloaded from the database before every block. That per-block load had no retry, so a single transient database error failed the whole run, and the raw query error (SQL and bound parameters) surfaced as the block error.

- Memoize the in-flight load in a run-scoped map shared by every block copy, keyed by governed subject and workspace; failed loads are evicted
- Move the bounded transient-read retry from the tool-only wrapper into the shared loader so block, model, agent and tool gates all get it
- Replace a database query error's message in the block error handler and log only its redacted cause
- Redact bound parameters in the execution failure cause log
@vercel

vercel Bot commented Sep 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Sep 15, 2026 3:05am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no actionable correctness, security, or repository-rule violations remain.

Summary

This PR makes permission configuration loading run-scoped and resilient while preventing database query details from reaching execution outputs.

  • Shares in-flight permission configuration loads across shallow per-block context copies.
  • Retries bounded transient database read failures and evicts failed cache entries.
  • Preserves caller cancellation for permission checks outside a cached run.
  • Redacts database query details from block errors and execution failure logs.
  • The previous cancellation finding was addressed and its thread was manually resolved.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Permission gate] --> B{Run cache available?}
  B -->|No| C[Load with caller or context abort signal]
  B -->|Yes| D{Subject and workspace cached?}
  D -->|Yes| E[Await shared in-flight result]
  D -->|No| F[Create load using run abort signal]
  F --> G[Store promise in run cache]
  C --> H{Transient database failure?}
  G --> H
  H -->|Yes, attempts remain| I[Bounded backoff and retry]
  H -->|No| J[Return permission config]
  H -->|Failure exhausted| K[Fail closed]
  K --> L[Evict failed cache entry]
  J --> M[Evaluate permission gate]
Loading

Reviews (2) · Last reviewed commit: "fix(executor): honor caller cancellation..."

Comment thread apps/sim/tools/index.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@waleedlatif1
waleedlatif1 merged commit a77abd6 into staging Sep 15, 2026
25 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/permission-config-per-run branch September 15, 2026 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant