Skip to content

fix(acp): check directory trust before starting the ACP server - #1847

Merged
akramcodez merged 2 commits into
Nano-Collective:mainfrom
addyCooks:fix/acp-startup-checks
Oct 20, 2026
Merged

akramcodez merged 2 commits into
Nano-Collective:mainfrom
addyCooks:fix/acp-startup-checks

Conversation

@addyCooks

Copy link
Copy Markdown
Collaborator

Description

nanocoder --acp now checks directory trust before it loads the working directory's project config, the same check nanocoder run, the daemon and verify make. Previously only those entry points did.

  • --acp refuses a directory that isn't trusted, before any project config is loaded. The refusal is printed on stderr (stdout carries the protocol) and the process exits 1.
  • --trust-directory bypasses it for one run; NANOCODER_TRUST_DIRECTORY=1 trusts and saves the directory, as for run.
  • The VS Code extension passes --trust-directory only when the workspace is trusted in VS Code's Workspace Trust, so it keeps working in folders you've trusted there. In Restricted Mode it doesn't pass the flag.
  • The refusal message now lives in directoryTrustRefusal (config/preferences.ts) and is shared by --acp and verify.
  • The startup benchmark launches --acp in an untrusted checkout, so it now passes --trust-directory to keep measuring real startup rather than the refusal.
  • Docs, CLI help and a changeset updated.

Behaviour change: editors other than VS Code (Zed etc.) that launch nanocoder --acp in a folder that hasn't been trusted will now be refused until the folder is trusted, NANOCODER_TRUST_DIRECTORY=1 is set, or --trust-directory is added to the agent's args. This is covered in docs/features/acp.md.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Changeset

  • Added a changeset (pnpm changeset) describing this change for the changelog

Testing

Automated Tests

  • New features include passing tests in .spec.ts/tsx files
  • All existing tests pass (pnpm test:all completes successfully)
  • Tests cover both success and error scenarios

Manual Testing

  • Tested with Ollama
  • Tested with OpenRouter
  • Tested with OpenAI-compatible API
  • Tested MCP integration (if applicable)

Checklist

  • If this was for an open issue, I was assigned to it
  • Code follows project style guidelines
  • Self-review completed
  • Documentation updated (if needed)
  • No breaking changes (or clearly documented)
  • Appropriate logging added using structured logging (see CONTRIBUTING.md)

`nanocoder --acp` now refuses a directory that isn't trusted, the same check
`nanocoder run` makes, before it loads that directory's project config.
`--trust-directory` bypasses it for one run.

The VS Code extension passes `--trust-directory` only when the workspace is
trusted in VS Code's Workspace Trust.

The refusal message is now shared (`directoryTrustRefusal`) between ACP and
`verify`.
@github-actions github-actions Bot added area:docs Documentation area:vscode VS Code extension and host integration labels Oct 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

nc-review: comments — 2 nits

@addyCooks — a few things worth a look, none blocking.

PR #1847 closes a real gap: nanocoder --acp was loading a working directory's project config (providers, hooks, MCP) without first checking the directory trust gate that run, daemon and verify already apply. The fix threads --trust-directory through to runAcpServer, refuses on stderr with exit 1 before any project config is loaded, and shares a new directoryTrustRefusal helper with verify so the refusal wording stays in sync. The VS Code extension only passes --trust-directory when VS Code's Workspace Trust is on, and the startup benchmark now boots --acp with --trust-directory so it keeps measuring real startup rather than the new refusal. Tests for the helper and for acpCliArgs cover the refusal message, the fingerprint-recheck wording, and trust=true/false/undefined. No correctness or security issue found; one small completeness nit about a missing persisted confirmation message.

⚪ nit · completeness · source/acp/acp-server.ts:35

runAcpServer checks directoryTrustRefusal and exits, but does not surface a Marked ... as trusted (NANOCODER_TRUST_DIRECTORY=1) line on the persisted path the way source/plain/shell.ts does (writeStatus(...) on trust.persisted). When a user sets NANOCODER_TRUST_DIRECTORY=1 for a one-shot nanocoder --acp invocation, the directory is silently recorded and there is no on-stderr confirmation that the next start will accept it. Consider mirroring the plain-shell pattern. This is a UX nit, not a correctness bug — the editor's own UI can still show the refusal and the next-start trust is otherwise visible via trustedDirectories.

⚪ nit · tests · source/config/preferences.spec.ts

The test titled 'directoryTrustRefusal words a changed fingerprint differently from an unknown directory' actually exercises the 'upgrade' recheck branch (bare string entry in trustedDirectories) and asserts the /needs re-approval after the upgrade/ wording. That is a fine test of directoryTrustRefusal's wording, but the title promises a 'changed'-fingerprint case which is not asserted. Either rename the test or add a sibling case that uses an object entry with a stale fingerprint to assert the 'changed' wording — the trustRecheck unit tests already cover both branches, but the public helper's contract is now worth pinning directly.


🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional

Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.

@github-actions github-actions Bot added the agent:comments nc-review left non-blocking findings label Oct 19, 2026
…TrustRefusal

The existing test was titled for a changed fingerprint but exercised the
path-only upgrade case. Rename it and add a case with a stale fingerprint.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:comments nc-review left non-blocking findings area:docs Documentation area:vscode VS Code extension and host integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants