Repository navigation
Shared agent home: observe/plan/apply reconcile for ~/.agents - #66
Merged
Merged
Conversation
…redential-refusing MCP registry
… adoption, unreachable drift
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
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.
Task 1 of the agentctl multi-provider plan: the canonical shared agent home at
~/.agents, with each agent product getting a thin generated adapter rendered from it. Purely additive — no existing behaviour changes outside the clap struct and one line insrc/lib.rs.What it does, concretely
Edit
~/.agents/instructions/global.mdonce and the next--applypushes it to every product. Re-running with nothing changed prints "up to date", so it is safe on a hook or in a shell alias.Which products — one table, not five call sites
PROVIDERSinshared_home.rsgives each product its config directory, its global-instructions file, and where it keeps transcripts and any adoptable memory graph. Adapter rendering,classify()and memory discovery all iterate it.This was the main review finding on the branch: the module doc claimed "adding a provider is a registry entry rather than a
classifyarm", and that was false — twelve hardcoded.claude/.codexreferences across five places, so a third product meant five edits. Now it is one row, andevery_provider_in_the_table_is_handled_without_a_special_caseiterates the table asserting all three behaviours, so a row that would need a special case fails there.Checked rather than assumed: adding a Cursor row and changing nothing else renders
~/.cursor/rules/agent.mdand keeps every test green.The model
PathOwnershipdecides every write:SharedSourceGeneratedAdapterProviderNativeTranscriptEvidenceExternal~/.agents/pluginsisProviderNativedespite sitting inside the shared home.~/.agentsis OpenAI's own convention — codex-cli 0.148.0 discovers and rewrites~/.agents/plugins/marketplace.jsonimplicitly. Claiming it would put agentctl and Codex in a write race over one file.observe → plan → apply
Split so the middle step is pure:
observe()is the only pre-applyread,plan()is total over its input,apply()executes with atomic temp-then-rename writes.A plan cannot delete. There is no
Action::Remove; that is a property of the type, not a runtime check. A reconcile that runs on every hook event and can delete is one bad reading away from taking a user's own instructions with it — the same failure this repo just shipped three fixes for on the session registry.An existing memory graph is adopted in place. Copying would duplicate every note and fork its history on the next write. Discovery returns nothing when more than one candidate exists, counting across all products: adopting the wrong project's graph points every product at someone else's notes, while a missed adoption is a no-op the user can correct.
Drift
Each render records the bytes it wrote, under
adapters/. That stamp separates we wrote this and the source has since moved — ours to re-render — from a human changed it — theirs, reported and never overwritten. Without it the two are indistinguishable, and guessing "drift" breaks the main use case, because then editing the shared instructions reaches nobody.Credentials
McpRegistry::validatedrefuses any env value that is not a${VAR}reference, naming the server and the variable. The registry is shared, version-controllable and destined for every product's config, so one literal secret there becomes a secret in all of them. It refuses rather than redacting: a redacting writer fails open on the shape it did not anticipate, silently. Validation runs in the dry run, before anything is rendered.Four defects the pre-PR review caught
All were invisible to
cargo test,clippyandfmt, which were green before the review.A regression to an unrelated flag. The new clap fields were spliced between
--restore-sessions's doc comment and its#[arg]. A doc comment binds to the next field, so--shared-homeinherited "restore your local Claude sessions… pair with--dry-run" and--restore-sessionsshipped with no help text at all.The reconcile never settled.
observe()never reported thememory/adopted-graphpointer thatplan()tests for, soAdoptInPlacere-planned forever on any machine with a Claude memory directory — the exact machine this targets. The idempotence test passed only because its tempdir had no.claude/projects, so the branch never executed.A documented guarantee with no code behind it.
observe()hardcodeddrifted: Vec::new(), soAction::ReportDriftand the CLI'sDRIFToutput were unreachable. "Never overwritten" held; "reported" did not.The genericity claim was false, per the section above.
McpRegistry::render_claudeis deleted rather than kept: it had no production caller, and rendering registries into native config is per-product work that belongs inPROVIDERS— a renderer named after one vendor is the shape this deliberately avoids.docs/shared-agent-home.mdnow lists it under "Not yet implemented" instead of implying the MCP side is finished.Verified end to end on a real filesystem, not only in tests: run 1 applies its actions, run 2 prints "up to date", hand-editing
~/.claude/CLAUDE.mdthen reportsDRIFT, and a staged memory directory is adopted where it lives.Verification
cargo fmt --checkclean,cargo clippy --all-targets -- -D warningsclean, 19 shared-home tests plus the rest of the suite. CI green on ubuntu and macOS.Locally three
usage_ledgertests fail on a loaded box; they pass in isolation and in CI, are untouched by this branch, and only one is a real failure —perf_eviction_stays_linear_with_many_rowsasserts a wall-clock ratio, and when it panics it poisonscache_test_lock(), so the other two die on.expect("test serialise mutex poisoned")as collateral.No
#[allow(dead_code)]was added.shared_homelives only in the lib — the bin reaches it asclaudectl::shared_home, the way it already reachesclaudectl::reaper.Not in this PR
Rendering the MCP and hook registries into native config, populating
sessions/index.jsonl, and migrating memory out of a product's tree. All three are listed indocs/shared-agent-home.mdunder "Not yet implemented" rather than left as silent gaps.