Skip to content

fix(project-files): hold the Project and permission-group locks shared for non-charging file operations - #9058

Merged
waleedlatif1 merged 4 commits into
stagingfrom
fix/project-file-lock-modes
Oct 15, 2026
Merged

waleedlatif1 merged 4 commits into
stagingfrom
fix/project-file-lock-modes

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Changes only flag-gated Project file paths (behind the projects flag).

  • Project file reads and writes that do not charge storage hold the Project lock shared; storage-charging operations (create, update_content, upload_complete, versions.revert, extract_archive) declare chargesStorage and take it exclusively up front, so a shared hold is never upgraded; prepareProjectFileAccounting refuses a context that was not authorized exclusively, so a missing flag fails deterministically instead of deadlocking under concurrency. The save/set/restore of lock_timeout moves into a shared withLockTimeout helper. Lifecycle, payer transfer, retention, and account deletion stay exclusive and still exclude every file operation.
  • The Project file authorizer holds the organization's permission-group lock shared (acquirePermissionGroupOrgReadLock): group writes, workspace creation, and OAuth refresh still wait for it and the capability decision stays current through commit, but file operations no longer serialize on it. The 5s lock_timeout now bounds only the acquisition and the caller's setting is restored.
  • Project storage accounting locks the payer row last and only for a non-zero delta (the global order: Project → owner row → directory → files → payer); the Project row lock taken at prepare keeps the payer identity fixed. Zero-delta writes read usage unlocked.
  • readProjectFileContent and version content reads drop an unused preflight transaction (prepareFromSnapshot); their snapshot use case authorizes, and the final transaction still revalidates after the storage read.
  • Collaborative-document seed/persist retry only on moved revisions (ProjectFileRevisionConflictError) or cached-history conflicts; Project lock timeouts and lifecycle conflicts surface immediately instead of being retried and reported as a document conflict. Persist reads at most the largest editable file size.
  • Archive/restore find files with cached document state in one query instead of locking and decoding every file, and post-commit retirements run at most 8 at a time.
  • Project file id/root-name reference lookups no longer load the whole folder tree.
  • Removes the blanket knip ignores over the Project file contracts: unused aliases are deleted or un-exported; the four files still holding exports consumed only by stacked PRs keep a per-file ignore naming them.

Stacked Project file PRs: #9017 merges cleanly on top of this; #9018 and later need a rebase anyway. Expected conflicts there:

Type of Change

  • Bug fix

Testing

  • New integration checks in projects/files/__integration__/authorization.integration.ts: concurrent reads and a folder create proceed while a read holds the Project, lock_timeout is restored, and exclusive Project and permission-group holders still wait; a storage-charging write still excludes shared holders
  • New projects/files/application/documents.test.ts: lock conflicts are surfaced, revision conflicts still retry (verified red on the old predicate)
  • biome on changed files; type-check, audits, and integration suites run in CI

Checklist

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

@vercel

vercel Bot commented Oct 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 Oct 15, 2026 12:28am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[High impact] The PR appears safe to merge; both previous findings are fixed and no new blocking issue was found.

Summary

Project file operations that do not charge storage now share the Project and permission-group locks. Storage-charging operations keep the Project lock exclusive.

  • Non-charging Project file work can share its locks.
  • Storage accounting locks the payer only when bytes change.
  • Content reads and document saves use narrower snapshots and retries.
  • Archive and restore retire only cached documents in bounded batches.
  • File reference lookups load folder details only when needed.
  • Unused Project file contract exports are made private or removed.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Authorize Project file operation] --> B{Charges storage?}
  B -->|Yes| C[Exclusive Project lock]
  B -->|No| D[Shared Project lock]
  C --> E[Shared permission-group lock]
  D --> E
  E --> F[Check access and perform file work]
  F --> G{Non-zero storage change?}
  G -->|Yes| H[Lock payer and update usage]
  G -->|No| I[Commit]
  H --> I
Loading

Reviews (2) · Last reviewed commit: "fix(project-files): retire a restored do..." · Reviewed by Greptile

Comment thread apps/sim/lib/projects/files/application/authorization.ts Outdated
Comment thread apps/sim/lib/projects/files/application/documents.test.ts Outdated
@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 Oct 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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 32 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit 4d4de52 into staging Oct 15, 2026
53 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/project-file-lock-modes branch October 15, 2026 00:45

This branch was previously deployed

1 inactive deployment
Preview — 0712cdf7 Deployed Oct 15, 2026 by vercel[bot]
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