-
Notifications
You must be signed in to change notification settings - Fork 107
fix: resolve HIVEMIND_WORKSPACE_ID names to ids and warn on an unknown workspace #354
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ebecd71
a3e875c
e6540d8
e0b5547
13fea9f
1a80b04
420b0d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,9 @@ | |
| loadCredentials, | ||
| saveCredentials, | ||
| deleteCredentials, | ||
| lookupWorkspaceAlias, | ||
| } from "./auth-creds.js"; | ||
| import { findDirConfig } from "../dir-config.js"; | ||
|
|
||
| // Re-export so existing importers keep working without churn. | ||
| export { loadCredentials, saveCredentials, deleteCredentials }; | ||
|
|
@@ -55,15 +57,15 @@ | |
|
|
||
| // ── API Helpers ────────────────────────────────────────────────────────────── | ||
|
|
||
| async function apiGet(path: string, token: string, apiUrl: string, orgId?: string): Promise<unknown> { | ||
| async function apiGet(path: string, token: string, apiUrl: string, orgId?: string, signal?: AbortSignal): Promise<unknown> { | ||
| const headers: Record<string, string> = { | ||
| Authorization: `Bearer ${token}`, | ||
| "Content-Type": "application/json", | ||
| ...deeplakeClientHeader(), | ||
| ...hivemindOsHeader(), | ||
| }; | ||
| if (orgId) headers["X-Activeloop-Org-Id"] = orgId; | ||
| const resp = await fetch(`${apiUrl}${path}`, { headers }); | ||
| const resp = await fetch(`${apiUrl}${path}`, { headers, signal }); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Known pattern, not a new trust boundary: the "file data" is the org id from the nearest |
||
| if (!resp.ok) throw new Error(`API ${resp.status}: ${await resp.text().catch(() => "")}`); | ||
| return resp.json(); | ||
| } | ||
|
|
@@ -301,8 +303,7 @@ | |
| if (currentWs !== "default") { | ||
| try { | ||
| const wsList = await listWorkspaces(healed.token, apiUrl, creds.orgId); | ||
| const lcWs = currentWs.toLowerCase(); | ||
| const wsMatch = wsList.find(w => w.id === currentWs || (w.name && w.name.toLowerCase() === lcWs)); | ||
| const wsMatch = findWorkspace(wsList, currentWs); | ||
| if (!wsMatch) { | ||
| log(`workspace '${currentWs}' not in org ${creds.orgId} — reset to default`); | ||
| healed.workspaceId = "default"; | ||
|
|
@@ -326,8 +327,98 @@ | |
|
|
||
| // ── Workspace Commands ─────────────────────────────────────────────────────── | ||
|
|
||
| export async function listWorkspaces(token: string, apiUrl = DEFAULT_API_URL, orgId?: string): Promise<{ id: string; name: string }[]> { | ||
| const raw = await apiGet("/workspaces", token, apiUrl, orgId) as { data?: { id: string; name: string }[] } | { id: string; name: string }[]; | ||
| // An exact id always wins over a name: workspace A named "build" must not | ||
| // shadow workspace B whose id is "build". | ||
| export function findWorkspace( | ||
| wsList: { id: string; name: string }[], | ||
| ref: string, | ||
| ): { id: string; name: string } | undefined { | ||
| const lc = ref.toLowerCase(); | ||
| return wsList.find(w => w.id === ref) ?? wsList.find(w => w.name && w.name.toLowerCase() === lc); | ||
| } | ||
|
|
||
| export interface WorkspaceOverrideResult { | ||
| creds: Credentials; | ||
| // User-facing line when the override names a workspace the org doesn't | ||
| // have. Every write would 403 and capture would silently switch itself | ||
| // off, so SessionStart must say it out loud. | ||
| warning?: string; | ||
| } | ||
|
|
||
| // SessionStart must never hang on this lookup; the cached alias (if any) | ||
| // keeps working when the request is cut off. | ||
| const WORKSPACE_LOOKUP_TIMEOUT_MS = 5_000; | ||
|
|
||
| // The warning below lands in the model's context. A `.hivemind` is committed | ||
| // content from a cloned repo and workspace names come from the API, so | ||
| // neither may carry newlines, control characters, or unbounded text into it. | ||
| export function sanitizeForContext(value: string, max = 64): string { | ||
| const flat = value.replace(/[\p{Cc}\p{Cf}\s]+/gu, " ").trim(); | ||
| return flat.length > max ? `${flat.slice(0, max - 1)}…` : flat; | ||
| } | ||
|
|
||
| // `HIVEMIND_WORKSPACE_ID` (and a `.hivemind` workspaceId) are documented as | ||
| // workspace NAMES but the API only accepts ids in `/workspaces/{id}/...` — a | ||
| // name gets a 403 on every query. Resolve the reference against the EFFECTIVE | ||
| // org (env > .hivemind > login, same precedence as resolveDirConfig) and | ||
| // persist it in creds.workspaceAliases so every later (synchronous) hook maps | ||
| // it through loadConfig() without a round-trip. Runs every session, so a | ||
| // rename or deletion is picked up on the next start; the cache only carries | ||
| // the answer across hooks and network failures. Never throws. | ||
| export async function resolveWorkspaceOverride( | ||
| creds: Credentials, | ||
| log: (msg: string) => void = () => {}, | ||
| cwd: string = process.cwd(), | ||
| ): Promise<WorkspaceOverrideResult> { | ||
| const found = findDirConfig(cwd); | ||
| const ref = process.env.HIVEMIND_WORKSPACE_ID ?? found?.raw.workspaceId; | ||
| const token = process.env.HIVEMIND_TOKEN ?? creds.token; | ||
| if (!ref || ref === "default" || !token) return { creds }; | ||
| const orgId = process.env.HIVEMIND_ORG_ID ?? found?.raw.orgId ?? creds.orgId; | ||
| const apiUrl = process.env.HIVEMIND_API_URL ?? creds.apiUrl ?? DEFAULT_API_URL; | ||
| const cached = lookupWorkspaceAlias(creds.workspaceAliases, orgId, ref); | ||
| try { | ||
| const wsList = await listWorkspaces(token, apiUrl, orgId, AbortSignal.timeout(WORKSPACE_LOOKUP_TIMEOUT_MS)); | ||
| const match = findWorkspace(wsList, ref); | ||
| if (!match) { | ||
| const names = wsList.map(w => sanitizeForContext(w.name || w.id)).join(", ") || "(none)"; | ||
| const source = process.env.HIVEMIND_WORKSPACE_ID ? "HIVEMIND_WORKSPACE_ID" : "the nearest .hivemind file"; | ||
| log(`workspace '${ref}' not found in org ${orgId} (from ${source}${found ? `: ${found.path}` : ""})`); | ||
| return { | ||
| creds: cached ? forgetWorkspaceAlias(creds, orgId, ref) : creds, | ||
| warning: `Workspace '${sanitizeForContext(ref)}' (from ${source}) does not match any workspace in this org (available: ${names}); ` + | ||
| `capture and memory search will fail until it is fixed. Prefer \`hivemind workspace switch <name>\` over the env var.`, | ||
| }; | ||
| } | ||
| if (match.id !== ref) log(`workspace '${ref}' resolved to id '${match.id}'`); | ||
| return { creds: cached === match.id ? creds : rememberWorkspaceAlias(creds, orgId, ref, match.id) }; | ||
| } catch (e) { | ||
| log(`workspace resolve skipped (${(e as Error).message}); ${cached ? `using cached id '${cached}'` : "no cached id"}`); | ||
| return { creds }; | ||
| } | ||
| } | ||
|
|
||
| // Re-read credentials right before writing: many sessions start in parallel | ||
| // and one may have just healed the token. Only the alias map is merged in, | ||
| // so a stale in-memory snapshot can never roll back another session's write. | ||
| function rememberWorkspaceAlias(creds: Credentials, orgId: string, ref: string, id: string): Credentials { | ||
| const latest = loadCredentials() ?? creds; | ||
| const aliases = { ...latest.workspaceAliases, [orgId]: { ...latest.workspaceAliases?.[orgId], [ref.toLowerCase()]: id } }; | ||
| saveCredentials({ ...latest, workspaceAliases: aliases }); | ||
| return { ...creds, workspaceAliases: aliases }; | ||
| } | ||
|
|
||
| function forgetWorkspaceAlias(creds: Credentials, orgId: string, ref: string): Credentials { | ||
| const latest = loadCredentials() ?? creds; | ||
| const org = { ...latest.workspaceAliases?.[orgId] }; | ||
| delete org[ref.toLowerCase()]; | ||
| const aliases = { ...latest.workspaceAliases, [orgId]: org }; | ||
| saveCredentials({ ...latest, workspaceAliases: aliases }); | ||
| return { ...creds, workspaceAliases: aliases }; | ||
| } | ||
|
|
||
| export async function listWorkspaces(token: string, apiUrl = DEFAULT_API_URL, orgId?: string, signal?: AbortSignal): Promise<{ id: string; name: string }[]> { | ||
| const raw = await apiGet("/workspaces", token, apiUrl, orgId, signal) as { data?: { id: string; name: string }[] } | { id: string; name: string }[]; | ||
| const data = (raw as { data?: { id: string; name: string }[] }).data ?? (raw as { id: string; name: string }[]); | ||
| return Array.isArray(data) ? data : []; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ | |
| import { readFileSync } from "node:fs"; | ||
| import { dirname, join, resolve } from "node:path"; | ||
| import { loadConfig, type Config } from "./config.js"; | ||
| import { resolveWorkspaceRef } from "./commands/auth-creds.js"; | ||
|
|
||
| /** Committed (shared) and local (personal, gitignored) filenames, local first. */ | ||
| export const DIR_CONFIG_FILENAMES = [".hivemind.local", ".hivemind"] as const; | ||
|
|
@@ -136,12 +137,17 @@ export function resolveDirConfig( | |
| if (!found) return { config: base, collect: true, found: null }; | ||
|
|
||
| const orgLocked = !!(envOverride ? envOverride.HIVEMIND_ORG_ID : process.env.HIVEMIND_ORG_ID); | ||
| const wsLocked = !!(envOverride ? envOverride.HIVEMIND_WORKSPACE_ID : process.env.HIVEMIND_WORKSPACE_ID); | ||
| const envWs = envOverride ? envOverride.HIVEMIND_WORKSPACE_ID : process.env.HIVEMIND_WORKSPACE_ID; | ||
| const orgId = orgLocked ? base.orgId : (found.raw.orgId ?? base.orgId); | ||
| // Always resolve the RAW reference against the final org: `base.workspaceId` | ||
| // was already mapped by loadConfig() against the login org, which is the | ||
| // wrong map once this file routes the org elsewhere. | ||
| const wsRef = envWs || found.raw.workspaceId || base.workspaceId; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win Preserve the stored workspace reference for routed organizations.
Preserve the raw persisted workspace reference in 🤖 Prompt for AI Agents
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Premise does not hold: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
ast-grep outline src/commands/auth-login.ts src/commands/auth.ts src/hooks src/dir-config.ts --items all --type function --match 'switchWorkspace|healDriftedOrgToken|resolveDirConfig|loadRoutedConfig|main|on_session_start'
printf '\n--- Workspace switching and credential persistence ---\n'
rg -n -C 8 -P '\bswitchWorkspace\s*\(|\bworkspace\s+switch\b|\bfindWorkspace\s*\(' src/commands/auth-login.ts src/commands/auth.ts
printf '\n--- Legacy workspace matching in drift healing ---\n'
rg -n -C 12 -P '\bhealDriftedOrgToken\s*\(|\bfindWorkspace\s*\(' src/commands/auth.ts
printf '\n--- Session hook ordering for drift healing and workspace configuration ---\n'
rg -n -C 7 -P '\bhealDriftedOrgToken\s*\(|\bloadConfig\s*\(|\bresolveDirConfig\s*\(|\bloadRoutedConfig\s*\(' src/hooks src/dir-config.tsLength of output: 51391 🏁 Script executed: #!/bin/bash
set -euo pipefail
ast-grep outline src/config.ts --items all --type function,interface --match 'loadConfig|Config|Credentials'
printf '\n--- Current loadConfig workspace resolution ---\n'
rg -n -C 12 -P '\bfunction\s+loadConfig\b|\bworkspaceId\b|\bresolveWorkspaceRef\s*\(' src/config.ts
printf '\n--- Parent version of configuration and directory routing ---\n'
git diff 1a80b048d392b5a4a2cf21d5fd302017f95152ce^ 1a80b048d392b5a4a2cf21d5fd302017f95152ce -- src/config.ts src/dir-config.ts
printf '\n--- Existing legacy credential and organization-only routing tests ---\n'
rg -n -C 8 -P 'legacy|workspaceId.*name|organization-only|org.*rout|routes?.*org|found\.raw\.orgId' tests/ src/commands/auth.tsLength of output: 200048 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- src/config.ts relevant range ---'
nl -ba src/config.ts | sed -n '1,180p'
printf '%s\n' '--- Exact change in src/config.ts ---'
git diff --unified=25 1a80b048d392b5a4a2cf21d5fd302017f95152ce^ 1a80b048d392b5a4a2cf21d5fd302017f95152ce -- src/config.ts
printf '%s\n' '--- Exact change in src/dir-config.ts ---'
git diff --unified=25 1a80b048d392b5a4a2cf21d5fd302017f95152ce^ 1a80b048d392b5a4a2cf21d5fd302017f95152ce -- src/dir-config.ts
printf '%s\n' '--- Tests focused on resolveDirConfig alias routing ---'
rg -n -C 10 -P 'alias|workspaceAliases|routed.*org|org.*route|base\.workspaceId' tests/shared/dir-config.test.ts tests/claude-code/config.test.ts 2>/dev/null || trueLength of output: 15910
However, the legacy case remains.
The normal CLI path is safe. The compatibility path is not. Please retain the raw stored reference, or add a regression test that demonstrates the legacy input is intentionally unsupported. ✏️ Learnings added
You are interacting with an AI system. |
||
| const config: Config = { | ||
| ...base, | ||
| orgId: orgLocked ? base.orgId : (found.raw.orgId ?? base.orgId), | ||
| orgId, | ||
| orgName: orgLocked ? base.orgName : (found.raw.orgName ?? found.raw.orgId ?? base.orgName), | ||
| workspaceId: wsLocked ? base.workspaceId : (found.raw.workspaceId ?? base.workspaceId), | ||
| workspaceId: resolveWorkspaceRef(base.workspaceAliases, orgId, wsRef), | ||
| }; | ||
| return { config, collect: found.raw.collect !== false, found }; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Known pattern, not a new trust boundary: the "file data" is the org id from the nearest
.hivemind(existing per-directory routing feature, README "Per-directory routing") and the token/API URL from~/.deeplake/credentials.json, which every authenticated request already carries. A.hivemindcan only route to orgs the logged-in token is authorized for; a wrong org just yields the 403 this PR now reports. Same class as the open alerts onharnesses/pi/extension-source/hivemind.tsandsrc/notifications/sources/balance.ts. No change.