Skip to content

[codex] Add structured asset access context#3378

Open
juliusmarminge wants to merge 1 commit into
codex/structure-project-favicon-errorsfrom
codex/structure-asset-access-errors
Open

[codex] Add structured asset access context#3378
juliusmarminge wants to merge 1 commit into
codex/structure-project-favicon-errorsfrom
codex/structure-asset-access-errors

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • add typed asset operations and the full requested resource to AssetAccessError
  • retain the existing user-facing messages and exact causes
  • make thread IDs, file paths, attachment IDs, and favicon roots available without parsing message text

Validation

  • pnpm vp test apps/server/src/assets/AssetAccess.test.ts (7 tests)
  • pnpm vp check (20 pre-existing warnings)
  • pnpm vp run typecheck

Stacked on #3369, which is stacked on #3342, so it extends the existing asset error work rather than conflicting with it.


Note

Medium Risk
Contract change requires every AssetAccessError construction to include new fields; behavior is unchanged but touches asset-access error paths that are security-adjacent.

Overview
AssetAccessError now carries typed operation and resource fields alongside existing message and optional cause, via a new AssetAccessOperation literal union in contracts.

Every failure path in issueAssetUrl (workspace files, attachments, project favicons, signing key) and in the WebSocket assetsCreateUrl workspace-context resolution now sets a specific operation (e.g. validate-workspace-path, inspect-workspace-asset, resolve-workspace-context) and echoes the requested AssetResource.

User-facing messages and underlying causes are unchanged; tests assert the new fields on representative path-validation and inspection failures.

Reviewed by Cursor Bugbot for commit eeacdc5. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add structured operation and resource fields to AssetAccessError

  • Extends AssetAccessError in packages/contracts/src/assets.ts with two required fields: operation (a constrained AssetAccessOperation literal) and resource (the AssetResource being processed).
  • Updates all error construction sites in AssetAccess.issueAssetUrl and the workspace RPC handler in ws.ts to populate these fields with operation-specific literals.
  • Risk: AssetAccessError construction is now a breaking change — all call sites must supply operation and resource or the schema validation will fail.

Macroscope summarized eeacdc5.

Co-authored-by: codex <codex@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: dc3bb70f-2403-4cbd-9c96-e05b00b7913e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/structure-asset-access-errors

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Jun 20, 2026
@macroscopeapp

macroscopeapp Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

This PR adds structured context (operation type and resource info) to existing error objects for improved debugging. The changes are purely additive, don't alter control flow, and include corresponding test updates.

You can customize Macroscope's approvability policy. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant