fix(storage): stabilize Accounts cloud registry routing#12
Merged
Conversation
…ore + ApiStore) Consolidate the registry behind one Store abstraction: LocalStore (on-box JSON) and ApiStore (self-hosted/cloud <API_URL>/v1 + bearer). resolveStore() is the mode resolver from env. CLI commands and MCP tools now read/write the registry only through the Store — no scattered cloud/local branches, no raw fetch/sqlite outside the Store/server. Extends cloud-awareness to list/show/use/active/current/ env/launch/shell/set/rename/detect/path/remove/doctor. - New src/lib/store.ts: AccountsStore interface + LocalStore + ApiStore + resolveStore - Add update()/rename() to the cloud client; add PATCH-backed update + new POST /v1/accounts/:tool/:name/rename server endpoint (repo + schema + OpenAPI + SDK). NOTE: rename endpoint requires an ECS redeploy of accounts-serve. - Depend on @hasna/contracts@0.5.2 - Tests for Store transport selection/routing, cloud update/rename, server rename - Bump version to 0.2.0 Machine-local orchestration (apply/switch/run/login/pick/supervisor) and the applied/toolLocks maps remain per-machine by design; they resolve the profile record through the Store.
Complete the single-AccountsStore refactor so no CLI command, MCP tool, or SDK method reads/writes the accounts registry via the local JSON store in self_hosted/cloud (api) mode. - resolveSupervisorLaunch is now async and resolves profiles + the active selection through the Store (was calling getProfile/currentProfile/ appliedProfile directly from profiles.js). The machine-local applied pointer name still lives on-box, but its record is resolved via the Store. - Supervised launch/restart marks the active selection via store.useProfile instead of the local-only useProfile, eliminating split-brain current writes in api mode. - Delete dead/duplicate ensureProfileForLogin + findProfileByName from import-profile.ts (login uses the Store-routed existingOrCreateProfile); drop the stale export and tests.
…rage through the Store Removes the dead dual code path flagged by adversarial review: - storage.ts kept ONLY the local registry primitives (accountsHome, storePath, profilesDir, loadStore, saveStore). Deleted AccountsStorageMode local|remote|hybrid, parseMode, the S3 client, storagePush/Pull/Sync, snapshots, and all HASNA_ACCOUNTS_S3_* / STORAGE_MODE env plumbing. The only storage abstraction is now AccountsStore (LocalStore + ApiStore). - Removed the CLI `storage status|push|pull|sync` command group and its print helpers (they bypassed the Store and used forbidden remote/hybrid words). - Dropped the @aws-sdk/client-s3 dependency and its build-script external flag. Fixes the live crash: a stale HASNA_ACCOUNTS_STORAGE_MODE=remote|hybrid|s3 (left from the S3 era) made every registry command throw via the contracts resolver. deriveEnv now only honors the canonical local|self_hosted|cloud words (mode's sole authority is to force local) and strips any other value so it can never reach the resolver; URL+KEY presence remains the cloud toggle. Reworked health/readiness storage section to report local vs self_hosted (api) via the Store resolver instead of the deleted S3 status, and rewrote storage.test.ts to cover the local primitives plus the legacy-mode crash regression.
In cloud mode 'accounts tools add/remove' wrote to the local accounts.json and never reached the shared cloud registry (split-brain). Add a cloud custom_tools table + POST/DELETE/GET /v1/tools server routes, route the CLI and MCP tool commands through AccountsStore.listTools/addTool/removeTool, and have ApiStore refresh a machine-local resolution cache so synchronous getTool/listTools still resolve cloud-registered tools for launch/apply. Bump 0.2.1.
The health/readiness command and doctor cross-checks read profiles, custom tools, and the per-tool 'current' selection from the local JSON file via loadStore(), even in api (self_hosted/cloud) mode where those are cloud-owned. On a flipped machine with an empty/stale local file this reported empty or stale profiles and a wrong active selection while claiming transport=api — a split-brain / routing-bar violation. - getAccountsReadiness is now async and reads profiles, tools, and current through resolveStore(); 'applied' stays machine-local. - doctor reads current through the Store and applied via loadAppliedMap(). - add loadAppliedMap(): reads the machine-local applied pointer without the local-profile pruning loadStore() applies, so it stays correct in api mode (profiles live in the cloud) and lets doctor still flag stale pointers. - delete the dead toolsFromStore() local-merge helper. - regression test proves api-mode readiness reflects the Store, not the file.
accounts list crashed (exit 1, "unknown tool") against the shared cloud registry whenever it held profiles for tools not in the client's local tool set (e.g. aicopilot, browserplan): per-profile prelaunch rendering called getTool(), which threw and aborted the whole listing. - prelaunchSummaryFor now degrades an unknown tool to an unsupported summary instead of throwing. - list hydrates the local tool cache from the active Store first, so cloud-registered custom tools resolve fully (best-effort). - action() wrapper now surfaces cloud HasnaHttpError as a single clean error line + exit 1 (with a redeploy hint on 404) instead of dumping an uncaught Node stack trace, which is what rename/tools add/tools remove did when they hit a stale server missing those routes. Adds a regression test that lists profiles referencing unknown tools.
…oint
Mutating registry calls (accounts rename, tools add, tools remove) route
correctly through the Store to endpoints that already exist in src/server.
When the connected accounts-serve is an older deployed build, those routes
are absent and return a generic route-missing 404 ({"error":"not found"}).
Surface an actionable 'redeploy accounts-serve' diagnostic for that case
instead of a raw HTTP failure, while leaving genuine entity-404s untouched.
Adds regression coverage. Bump 0.2.4.
…istent profile The ApiStore.remove path threw a plain Error for entity-not-found and multi-tool ambiguity, so the CLI action() wrapper (which only cleans up AccountsError + HasnaHttpError) re-threw it as an unhandled exception, dumping a stack trace that leaked the internal cli.js path. Local mode already threw AccountsError. Now cloud mode matches: a single clean 'error: no profile named ...' line + exit 1. Adds regression tests.
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.
Tasks
8b45b77f-9218-4aa3-b59d-b0b695c59cea466d092b-b6d6-4882-a120-851e553e6142Restack Strategy
83844e4through the Tool-schema source change inba5f8c4.651c8ae, keeps package version0.1.33, and excludes Claude commitea5a744.fe3162f,e42bf68, anddc173f3.ea5a744; no history was force-rewritten.Scope And Blast Radius
self_hosted/cloudconfiguration.Compatibility
No breaking removal is approved. Deprecated storage constants, types, status/snapshot functions,
ensureProfileForLogin, andaccounts storage status|push|pull|syncremain.accountsStorageSnapshotKey,storagePush,storagePull, andstorageSyncpreserve their old optionalNodeJS.ProcessEnvargument. Retired CLIpush|pull|synccommands parse--jsonbefore returning the deterministic retirement diagnostic. A dedicated TypeScript compile fixture and CLI regressions enforce both contracts.Old-client/new-server remains compatible. New-client/old-server accepts minimal additive Tool responses and existing endpoints; rename/custom-tool mutations require the new server and fail with an actionable version diagnostic.
Migration And Deployment
accounts-migratethrough migrations0003and0004.current_selection_orphan_archive; 0004 archives orphan selections before removing them, preserves valid rows, and adds the cascading FK.Never run a pre-0003 migrator after 0003/0004 are recorded. The migration status/ledger rejects unknown applied migrations as a deterministic downgrade guard. Application rollback keeps the new migrator job with the older app image; otherwise forward-fix. Do not drop additive tables/FKs as an application rollback.
Validation
importProfilearity.git diff --check: passed.0.2.xclaims.Andrei Hasna <andrei@hasna.com>; no Co-Authored-By trailer.Adversarial Self-Review
No agent was spawned because the task explicitly prohibited it. The labeled self-review challenged migration data loss, no-op downgrade bypass, cooperative-lock ordering, remove-first/create-first race outcomes, copy failure after partial mutation, public declaration drift, ambient Accounts env contamination, authorship, and PR #11 provenance.
Reconciled findings: archive orphans before cleanup; enforce migration status compatibility before no-op; lock custom-tool/account mutations by tool id and validate custom-tool existence; keep copy inside cleanup; preserve the two-argument public import declaration; run full tests with CI-equivalent Accounts env sanitization.
Residual risk: migration 0004 changes live orphan rows and retains the backup-first requirement. Migration 0005 must remain installed during application rollback; pre-0005 migrators remain downgrade-blocked.
Final P1 Revision (980e75e)
Runtime Role Review (1b4db0c)