fix(server): keep GET /v1/tools Tool response schema wire-additive (0.2.6)#10
Open
andrei-hasna wants to merge 10 commits into
Open
fix(server): keep GET /v1/tools Tool response schema wire-additive (0.2.6)#10andrei-hasna wants to merge 10 commits into
andrei-hasna wants to merge 10 commits into
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.
The self-host refactor enriched GET /v1/tools (full ToolDef + custom tools) and grew the OpenAPI Tool schema's required set to [id,label,envVar,defaultDir,bin]. The deployed 0.1.x server only guaranteed [id,label] and never emitted defaultDir, so the change narrowed an existing response contract and the server-redeploy safety gate flagged it non-additive. Revert Tool.required to [id,label] and document the extra ToolDef fields as optional, making the HTTP response contract a strict superset of the deployed version. Runtime behavior is unchanged; old /v1 clients keep working. No route removed/renamed; rename + custom-tool endpoints stay additive alongside. Bump @hasna/accounts 0.2.5 -> 0.2.6.
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.
Summary
Makes the
accounts-serveHTTP wire surface a strict superset of the deployed 0.1.x server so the self-hosted redeploy is drop-in safe for OLD /v1 fleet clients.The non-additive change (now fixed): the self-host refactor enriched
GET /v1/tools(returns the fullToolDef+ custom tools from the new cloudcustom_toolsregistry) and, in doing so, grew the OpenAPIToolresponse schema'srequiredset to[id,label,envVar,defaultDir,bin]. The deployed server only guaranteed[id,label]and never emitteddefaultDir, so this narrowed an existing response contract — the server-redeploy safety gate flagged it non-additive.Fix:
Tool.requiredreverted to[id,label]with the extraToolDeffields documented as optional (src/server/openapi.ts). Runtime behavior is unchanged (handler still returns the full ToolDef + custom tools); old /v1 clients — which parse the response without strict validation — keep working.ToolDefInput(the new POST /v1/tools request body) keeps its required fields (new endpoint, no old client sends it).Wire-additivity (verified)
POST /v1/accounts/{tool}/{name}/rename,POST /v1/tools,DELETE /v1/tools/{id}.GET /v1/tools, now a proven superset (all 12 built-ins keep id/label/envVar/bin/builtin, plus new optional fields).0003_custom_tools.sql=CREATE TABLE IF NOT EXISTS(idempotent/additive).Deployed + verified
accounts-prod:4(ARM64, oss-fleet-prod, us-east-1); prior rev:2retained for rollback. Migration 0003 applied first (health check is/health, so migrate-before-serve was mandatory).tools add/remove,rename) now work; local mode still works (reversible).Note: this branch also carries the previously-unpushed self-host refactor commits (origin/main was at fe0eb29) — they reflect the code already published to npm (0.2.5) and deployed.