Skip to content

refactor: delegate workspace trust to plugins#2744

Merged
Davidknp merged 5 commits into
mainfrom
refactor-extract-agent-trust-services
Jul 1, 2026
Merged

refactor: delegate workspace trust to plugins#2744
Davidknp merged 5 commits into
mainfrom
refactor-extract-agent-trust-services

Conversation

@jschwxrz

@jschwxrz jschwxrz commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator
  • add a trust capability so provider plugins own workspace trust behavior
  • move claude, copilot and cursor trust logic into provider implementations
  • replace app-side provider-specific trust services with WorkspaceTrustService
  • add remote PluginFs support for ssh workspace trust
  • make PluginFs writes atomic and preserve non-not-found read errors

@greptile-apps

greptile-apps Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR delegates workspace trust behavior from two monolithic app-layer services (ClaudeTrustService, CursorTrustService) into per-provider plugin implementations, introducing a new ITrustBehavior capability and a PluginFs abstraction for sandboxed file access.

  • A new PluginFs interface (local + remote SSH) is added, scoped to the agent home directory, with atomic writes and path-escape protection. The WorkspaceTrustService replaces the old provider-specific services and delegates trust writes to whichever ITrustBehavior the provider plugin declares.
  • Claude, Copilot, and Cursor each gain a standalone trust implementation under packages/plugins, removing the provider-specific branching that was baked into the app layer.
  • The remote SSH trust path is generalised through createRemotePluginFs, enabling any future plugin to implement SSH workspace trust without app-level wiring.

Confidence Score: 4/5

The refactor is well-structured and trust logic is faithfully reproduced in the plugin layer; the two flagged concerns are limited to error-handling edge cases in the new PluginFs utilities and do not affect the happy path.

The delete method in plugin-fs.ts silently discards all errors rather than only ENOENT, inconsistent with the explicit non-not-found error preservation the PR applies to read. The remote fileSystem() helper is called per-operation rather than once, meaning a transient failure surface is slightly larger. Neither issue affects current trust writes (which use read/write/exists), but both are worth tightening before additional plugins rely on the PluginFs API.

apps/emdash-desktop/src/main/core/agents/plugin-fs.ts and apps/emdash-desktop/src/main/core/agents/remote-plugin-fs.ts would benefit from a second look; all other changed files look correct.

Important Files Changed

Filename Overview
apps/emdash-desktop/src/main/core/agents/plugin-fs.ts New local PluginFs implementation — atomic writes and path-escape protection are solid; delete silently swallows all errors instead of only ENOENT, inconsistent with the explicit read fix in this same PR.
apps/emdash-desktop/src/main/core/agents/remote-plugin-fs.ts New remote PluginFs over SSH — fileSystem() is called on every operation rather than once; errors thrown here propagate differently than the prior graceful early-return pattern.
apps/emdash-desktop/src/main/core/agents/workspace-trust.ts Central WorkspaceTrustService — clean delegation pattern with correct error isolation; lock key is slightly coarser (per homedir) than before (per config file) but functionally correct.
packages/core/src/agents/plugins/helpers/trust.ts Shared buildJsonConfigTrustBehavior helper — well-extracted, throws on corrupt/non-object config (caught at service layer), correctly passes paths through PluginFs.
packages/plugins/src/agents/impl/cursor/trust.ts Cursor trust behavior — uses relative paths through PluginFs (rooted at homedir), slugify logic preserved from old service, idempotency check correct.
packages/plugins/src/agents/impl/claude/trust.ts Claude trust behavior — cleanly delegates to buildJsonConfigTrustBehavior with the same withTrustedPath logic as the deleted service.
packages/core/src/agents/plugins/capabilities/trust.ts New trust capability definition — Zod schema with supported/none variants, correct default of none.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[startSession] --> B[WorkspaceTrustService.maybeAutoTrust]
    B --> C{Provider has ITrustBehavior?}
    C -- No --> Z[No-op]
    C -- Yes --> D{autoTrustWorktrees or force?}
    D -- No --> Z
    D -- Yes --> E[resolveTrustTarget]
    E --> F{host.kind}
    F -- local --> G[createPluginFs homedir]
    F -- ssh --> H[resolveRemoteHome + createRemotePluginFs]
    G --> I[withHomeLock]
    H --> I
    I --> J[behavior.trustWorkspace]
    J --> K{Provider}
    K -- claude --> L[buildJsonConfigTrustBehavior .claude.json]
    K -- copilot --> M[buildJsonConfigTrustBehavior .copilot/config.json]
    K -- cursor --> N[buildCursorTrustBehavior .cursor/projects/slug]
    L --> O[fs.read + merge + fs.write atomic]
    M --> O
    N --> P[fs.exists check + fs.write marker]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[startSession] --> B[WorkspaceTrustService.maybeAutoTrust]
    B --> C{Provider has ITrustBehavior?}
    C -- No --> Z[No-op]
    C -- Yes --> D{autoTrustWorktrees or force?}
    D -- No --> Z
    D -- Yes --> E[resolveTrustTarget]
    E --> F{host.kind}
    F -- local --> G[createPluginFs homedir]
    F -- ssh --> H[resolveRemoteHome + createRemotePluginFs]
    G --> I[withHomeLock]
    H --> I
    I --> J[behavior.trustWorkspace]
    J --> K{Provider}
    K -- claude --> L[buildJsonConfigTrustBehavior .claude.json]
    K -- copilot --> M[buildJsonConfigTrustBehavior .copilot/config.json]
    K -- cursor --> N[buildCursorTrustBehavior .cursor/projects/slug]
    L --> O[fs.read + merge + fs.write atomic]
    M --> O
    N --> P[fs.exists check + fs.write marker]
Loading

Comments Outside Diff (1)

  1. apps/emdash-desktop/src/main/core/agents/plugin-fs.ts, line 47-53 (link)

    P2 The delete method catches and discards every error, not just ENOENT. This PR explicitly preserved non-not-found errors in read (and the test verifies it), but delete applies no such distinction — a permissions error or EBUSY would be silently swallowed and callers would see success.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/emdash-desktop/src/main/core/agents/plugin-fs.ts
    Line: 47-53
    
    Comment:
    The `delete` method catches and discards every error, not just ENOENT. This PR explicitly preserved non-not-found errors in `read` (and the test verifies it), but `delete` applies no such distinction — a permissions error or EBUSY would be silently swallowed and callers would see success.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
apps/emdash-desktop/src/main/core/agents/plugin-fs.ts:47-53
The `delete` method catches and discards every error, not just ENOENT. This PR explicitly preserved non-not-found errors in `read` (and the test verifies it), but `delete` applies no such distinction — a permissions error or EBUSY would be silently swallowed and callers would see success.

```suggestion
    async delete(path: string): Promise<void> {
      try {
        await fs.unlink(resolveSafe(path));
      } catch (error: unknown) {
        if (isFileNotFoundException(error)) return;
        throw error;
      }
    },
```

### Issue 2 of 2
apps/emdash-desktop/src/main/core/agents/remote-plugin-fs.ts:32-36
`fileSystem()` is called on every `read`, `write`, `exists`, and `list` operation rather than once at construction time. In the old code, a single call was made before any I/O and a failure produced a graceful early return with a warning. Here, a failure inside any operation throws directly and propagates through the `withHomeLock` try-catch, which produces a less specific warning message. If `IFilesRuntime.fileSystem()` can fail transiently, concurrent operations on the same remote FS each pay that failure cost independently.

Reviews (1): Last reviewed commit: "refactor(agents): delegate workspace tru..." | Re-trigger Greptile

Comment thread apps/emdash-desktop/src/main/core/agents/remote-plugin-fs.ts Outdated
@Davidknp Davidknp merged commit 457972c into main Jul 1, 2026
1 check passed
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.

2 participants