Add --org to remote SQL, graph, and MCP reads - #1362
Conversation
Review fixes on the operator cross-org read path. A 403 answering an explicit --org is the server refusing the *selector*, not the credential, so the shared 401/403 refresh policy was wrong for it: the attach path forced a token refresh, re-sent the read the server had already denied (a second entry in that org's audit trail), and then advised re-running 'hyp remote login', which cannot grant operator standing. Both the verb attach path and the stdio proxy now stop at the first 403 and say what was refused. Also: adding the selector no longer rewrites the target's own query string (URL.searchParams.set re-serializes the whole query as form encoding); the stdio proxy resolves built-in targets the same way the verb path does, so 'hyp mcp serve --remote hyperparam --org *' reaches the shipped server instead of reporting an unknown target; an empty --org value is named as such rather than blamed on a missing --remote; and 'hyp query --help' plus CLI_REFERENCE document the flag the usage strings already advertise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review:
|
Round 2 review fixes on the operator cross-org read path. remote_verb.js reads `.status` off the caught rejection to decide whether an explicit --org met a 403. It read it unguarded, ahead of isAuthError's own `!!err && typeof err === 'object'` test, so a rejection that is not an object (the MCP client awaits fetch directly and rethrows whatever it rejected with) threw a TypeError out of the catch block. The real failure was then replaced by `hyp query sql: Cannot read properties of undefined (reading 'status')`. Optional-chain the read, matching the guard the helper below it already keeps. Also: the stdio proxy's comment above the failure-detail branches said a 401/403 there is a dead credential, which the org-403 branch added beside it is not. Say what each branch covers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A 403 under an explicit --org is still terminal (a retry only earns a second denial in that org's audit trail), but the message asserted a cause the client cannot see. A deployment that answers a revoked credential with 403 rather than 401, or a proxy in front of it, produces the same status, and the operator was told "this account may not read that org; re-login cannot grant operator standing" - advised away from the one move that fixes it. Name both causes and the remedy for the second, and say "refused" of the request rather than of a read, since the same 403 can answer the MCP handshake. Behavior is unchanged: still no refresh and still no re-send. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review round 2:
|
| # | Severity | Status |
|---|---|---|
| 1 | Low | Fixed in 5c06f48d |
| 2 | Low-medium | Fixed in d8c3f9ae |
| 3 | Low | Open - server-contract question |
| 4 | Low | Open - needs a PR-body line I cannot add |
| 5 | Low | Open - carried from round 1, non-blocking |
| 6 | Low | Open - LLP judgment + merge-order risk |
Triage: stuck on a merge-order blockerNeutral's triage rung examined PR #1362 at head The blocker: merging before hyparam/hypaware-server#437 produces silently wrong answersRound 2 flagged this; triage verified it independently on both sides rather than trusting the prose. What was verified:
Consequence if this PR merges first: Non-blocking findings (recorded so the whole PR is visible)Using round 2's numbering:
Questions for a human, with optionsOn the blocker, pick one:
Secondary (answer if convenient): for (3), is the empty How to unstickreply with a comment on this PR (or push to the branch); neutral monitors this thread and will re-engage with your guidance on its next tick. |
Remote SQL, graph neighbors, and the stdio MCP proxy now accept
--org <label|*>. For example,hyp query sql --remote hyperparam --org '*' "select org, count(*) from ai_gateway_messages group by org"sends the selector on the derived MCP URL, including the initialize requests. The selector is a transport flag and is rejected without--remote; endpoint derivation preserves path prefixes and other query parameters.The privacy document states that deployment operators can read forwarded data across orgs and that these reads are audited. The companion server enforces operator standing and implements the per-org union and audit fan-out.
Validation:
node --test test/core/remote-mcp-endpoint.test.js: all 9 tests passed, including SQL, graph, the proxy, URL preservation, and local/missing-value rejection.npm run typecheck: passed after installing this branch's declared dependencies in its own worktree.npm test: 6,060 passed, 3 failed, 3 skipped. The sandbox-launch and overlapping-spool-read failures passed on an isolated rerun. The gateway testa body refused with 421 is drained only up to a capstill failed and reproduced identically on a pristine copy of the starting commit, d42c794. No unrelated test or gateway code was changed.git diff --check: passed.CPU and memory review: one optional string parsed per invocation and one URL search parameter update per remote connection. No new per-row work, retained state, runtime dependencies, or CPU/memory concern.
The managed deployment acceptance gate is documented in the companion server PR; it needs an operator login on that deployment.
Companion server PR: https://github.com/hyparam/hypaware-server/pull/437