feat(locks): let agents lease files, and let the user switch a builtin off - #56
Merged
Conversation
…n off Three things that belong together: the guard could not be relaxed even when the user wanted it, agents had no way to say "I am working on this file", and FleetDeck had no way to see what C3 knows. ACCESS GUARD -- builtins become disableable, in two tiers. The credential vault (secrets.enc, cred_state.json) stays absolute: it already has a dedicated guard and its own human-only escalation, so an opt-out there would be nothing but a shorter route to the same secrets. The other four -- .env*, .c3/**, .claude/settings*.json, .git/** -- a human can switch off. Switching one off needs BOTH a config entry and a keyring attestation, the construction the credential vault already uses. Either alone is inert: config without attestation fails closed, attestation without config is never read. The point is the prompt-injection case -- an agent that manages to write config.json, which is exactly the move that would grant itself write access to ~/.claude/settings.json, still cannot produce the attestation, so the builtin keeps enforcing. Global scope only, because project scopes may only tighten; a project-scope entry is a loud config error rather than a silent no-op, so the UI can never claim a builtin is off while evaluation enforces it. `c3 access builtin disable` asks the user to retype the glob, and says plainly when the glob is the file that configures the agent itself. AGENT LOCKS -- leases (docs/agent-locks.md Layer B). Layer A stopped torn writes. It does nothing about two agents refactoring the same module for ten minutes, which needs a lease held across many edits with a declared intent. services/agent_locks.py implements FleetDeck's semantics deliberately -- (repo_id, relpath) keys, all-or-nothing acquisition over a sorted path list so opposite-order requests cannot deadlock, monotonic fencing so force-release makes a returning holder stale by construction, and TTL as the real release mechanism because agents forget to release. State is a file under an OS file lock in the TARGET project, so this keeps working with no daemon running -- the property that made C3 rather than FleetDeck its owner. c3_edit checks for a holder and takes its own lease, after the policy guards, never before: an agent must not be told a file is busy when it was never allowed to write it. Acquisition is implicit by default -- explicit calls produce better intent strings but agents forget them, and a lease nobody takes protects nobody. force_release is absent from the agent tool and lives in `c3 locks force-release`, ledger-logged. Building it corrected the spec twice, recorded in place rather than patched over. The c3_project row was wrong -- that proxy already builds a foreign runtime for the target, so the lock file was never at risk. But chasing it found something worse: lease identity came from a session id that is empty in plenty of real contexts, and two DIFFERENT agents both resolving to "" counted as one session and stopped blocking each other. Silent under-blocking, the exact failure a lock exists to prevent. Identity now falls back to the pid and never to "". FLEETDECK INTEGRATION -- read-only, and provably so. fleetdeck/c3_locks.py renders C3's leases in the deck without owning them; a test asserts the module contains no write calls at all, because a second owner is the bug the ownership split exists to avoid. A repo it cannot read reports available=False rather than "0 locks", which would claim the repo is clear when we simply do not know. Phase 3 was gated on denial data and was built without it, on Dimitri's call. The questions that data would have answered are listed in the spec as decided by judgement, so a later reader knows which choices are unvalidated. The Hub UI tab is not built. 1789 tests pass (70 new). Claude-Session: https://claude.ai/code/session_01Sbd9NbQfDcoJvvURD7KCVf
normalize_relpath came from fleetdeck/paths.py, which assumes a Windows host: only drive-letter and /mnt forms counted as absolute. C3 ships cross-platform, so on Linux and macOS every absolute path fell through to the repo-relative branch. A lease taken as 'services/router.py' was looked up as 'tmp/xyz/services/router.py' and the gate found nothing. Same class as the Layer A sidecar bug two phases ago: a key computable two ways is not a key. Both times this Windows box hid it and CI on the other platforms caught it. A POSIX absolute path now resolves against the root when it lives under it. A leading-slash path that does not is still read as repo-relative, because '/src/api.py' is how agents write relpaths rather than a failed absolute path. The regression test uses a real temp root and asserts the absolute and relative spellings of one file agree. A literal '/tmp/x' assertion proves nothing on Windows, where os.path.abspath rewrites it to the current drive -- my first attempt at this test made exactly that mistake. Also added a guard that the store side and gate side compute the same key, because when they diverge every gate test passes vacuously (None reads as 'not held'). Ruff import ordering in edit.py fixed. Claude-Session: https://claude.ai/code/session_01Sbd9NbQfDcoJvvURD7KCVf
Third instance of one bug: a key computable two ways is not a key. Windows CI home is the 8.3 alias RUNNER~1. handle_edit calls Path.resolve() before locking, expanding it to runneradmin, while project_path keeps the alias -- so normalize_relpath decided the file was outside its own repo, raised outside_repo, and the lease was silently never taken. Leases were inert on Windows. Root cause was inherited: FleetDeck normalizes purely lexically, which is correct for a Windows-only daemon that never sees a resolved path. C3 does see one. canonical_root and normalize_relpath now put both sides through realpath, which also collapses the macOS /var to /private/var split. realpath is non-strict, so create-mode paths that do not exist yet still normalize -- the property the lexical rule protected is kept. Reproduced the CI condition locally with GetShortPathNameW rather than reasoning about it: pre-fix the two spellings disagree (outside_repo), post-fix they agree. test_short_name_root_agrees pins it and skips where 8.3 generation is off, because a silent pass there would be the same trap. Added a POSIX symlink case for the /var split and a resolved-vs-unresolved assertion that is trivially true on a dev box and load-bearing on CI. 1794 pass, ruff clean. Claude-Session: https://claude.ai/code/session_01Sbd9NbQfDcoJvvURD7KCVf
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three things that belong together: the guard couldn't be relaxed even when you wanted it, agents had no way to say "I'm working on this file", and FleetDeck had no way to see what C3 knows.
1789 tests pass, 0 failures (70 new).
1. Access Guard — builtins become disableable, in two tiers
Tier 0 (absolute):
secrets.enc,cred_state.json. Never disableable — they already have a dedicated guard and their own human-only escalation (agent_readable), so an opt-out would be nothing but a shorter route to the same secrets.Tier 1 (disableable):
**/.env*,**/.c3/**,**/.claude/settings*.json,**/.git/**.Switching one off needs both a config entry and a keyring attestation — the construction
credential_store.verify_agent_readablealready uses. Either alone is inert.The case this defends: an agent that manages to write
config.json— precisely the move that would grant itself write access to~/.claude/settings.json— still can't produce the attestation, so the builtin keeps enforcing.test_config_without_attestation_stays_enforcedpins it.Global scope only, because project scopes may only ever tighten. A project-scope entry is a loud config error, not a silent no-op, so the UI can never claim a builtin is off while evaluation enforces it.
c3 access builtin disable <glob>requires retyping the glob, and says plainly when the glob is the file that configures the agent itself.2. Agent Locks — leases (spec Layer B)
Layer A stopped torn writes. It does nothing about two agents refactoring one module for ten minutes.
services/agent_locks.pyadopts FleetDeck's semantics deliberately:(repo_id, relpath)keys, all-or-nothing acquisition over a sorted path list (so opposite-order requests can't deadlock), monotonic fencing (soforce_releasemakes a returning holder stale by construction), and TTL as the real release mechanism — nothing assumes an agent remembers to release.State is a file under an OS file lock in the target project, so this works with no daemon running. That's the property that made C3 rather than FleetDeck the owner (§11).
c3_editchecks for a holder then takes its own lease — after the policy guards, never before: an agent must never be told a file is busy when it was never allowed to write it.test_access_guard_denial_outranks_leasepins the ordering.force_releaseis deliberately absent from the agent tool. It lives inc3 locks force-release, ledger-logged.Building it corrected the spec twice — recorded in place, not patched over
c3_projectrow was wrong. That proxy already builds a foreign runtime for the target (project.py:450), so the lock file was never at risk.""counted as one session and stopped blocking each other. Silent under-blocking: the exact failure a lock exists to prevent. Identity now falls back topid-<os.getpid()>and never to"".3. FleetDeck integration — read-only, and provably so
fleetdeck/c3_locks.pyrenders C3's leases without owning them.test_module_never_writesgreps the module for write calls, because a second owner is the bug the ownership split exists to avoid. A repo it can't read reportsavailable: Falserather than0 locks— which would claim the repo is clear when we simply don't know.(Lands on AgentSync's local-only
fleetdeckbranch, same as before.)What's NOT done, and why
The Hub UI tab. It needs a new component plus wiring in
sidebar.js/app.jsand REST endpoints, and it deserves browser verification rather than being rushed into a security-adjacent PR. P4 is therefore half: the FleetDeck reader is in, the tab is not.Phase 3 was gated on denial data and built without it, on your call. §14's open questions were answered by judgement, not evidence — implicit acquisition, leases don't block reads, no
c3_impactwiring, file-level granularity. The spec now lists these explicitly as unvalidated so a later reader knows which choices to revisit once real data exists.https://claude.ai/code/session_01Sbd9NbQfDcoJvvURD7KCVf