Skip to content

Commit 1ea3f2a

Browse files
authored
Merge pull request #209 from activeloopai/fix/skip-missing-table-reads
fix(reads): skip lazy SELECTs for absent tables via listTables pre-check
2 parents 04af7b8 + f89e70e commit 1ea3f2a

17 files changed

Lines changed: 337 additions & 58 deletions

src/commands/context.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,16 @@ export async function runContextCommand(args: string[]): Promise<void> {
6161
cfg.tableName,
6262
);
6363

64+
const known = await api.knownTablesOrNull();
65+
const tableExists = known ? (name: string) => known.includes(name) : undefined;
6466
const block = await renderContextBlock(
6567
(sql: string) => api.query(sql) as Promise<Array<Record<string, unknown>>>,
6668
{
6769
rulesTable: cfg.rulesTableName,
6870
goalsTable: cfg.goalsTableName,
6971
currentUser: cfg.userName,
7072
},
73+
{ tableExists },
7174
);
7275

7376
if (!block) {

src/deeplake-api.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,22 @@ export class DeeplakeApi {
413413
return tables;
414414
}
415415

416+
/**
417+
* Like listTables() but returns null when the list could NOT be trusted
418+
* (the fetch failed / was non-cacheable). Callers gating a read on table
419+
* existence use this to tell a genuinely-empty workspace ([]) apart from a
420+
* failed lookup (null): on [] they can safely skip the read (no table → no
421+
* 42P01), on null they must fall back to SELECT-then-catch so a transient
422+
* lookup blip doesn't drop a read of a table that really exists.
423+
*/
424+
async knownTablesOrNull(): Promise<string[] | null> {
425+
if (this._tablesCache) return [...this._tablesCache];
426+
const { tables, cacheable } = await this._fetchTables();
427+
if (!cacheable) return null;
428+
this._tablesCache = [...tables];
429+
return [...tables];
430+
}
431+
416432
private async _fetchTables(): Promise<{ tables: string[]; cacheable: boolean }> {
417433
for (let attempt = 0; attempt <= MAX_RETRIES; attempt++) {
418434
try {

src/hooks/cursor/session-start.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,14 +184,18 @@ async function main(): Promise<void> {
184184
// to the user (model-only), so the full block is fine. Renderer
185185
// absorbs its own errors and returns "" on any failure (including
186186
// missing rules table — see context-renderer.ts).
187+
// Trusted table list (cached) so the renderer skips the rules/goals
188+
// SELECT when the table isn't there yet — avoids a 42P01 server-side.
189+
const known = await api.knownTablesOrNull();
190+
const tableExists = known ? (name: string) => known.includes(name) : undefined;
187191
rulesBlock = await renderContextBlock(
188192
(sql: string) => api.query(sql) as Promise<Array<Record<string, unknown>>>,
189193
{
190194
rulesTable: config.rulesTableName,
191195
goalsTable: config.goalsTableName,
192196
currentUser: config.userName,
193197
},
194-
{ log },
198+
{ log, tableExists },
195199
);
196200
}
197201
} catch (e: any) {

src/hooks/hermes/session-start.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,14 +154,18 @@ async function main(): Promise<void> {
154154
}
155155
// Read-only renderer. Hermes's context field is invisible to
156156
// the user (model-only). Renderer absorbs its own errors.
157+
// Trusted table list (cached) so the renderer skips the rules/goals
158+
// SELECT when the table isn't there yet — avoids a 42P01 server-side.
159+
const known = await api.knownTablesOrNull();
160+
const tableExists = known ? (name: string) => known.includes(name) : undefined;
157161
rulesBlock = await renderContextBlock(
158162
(sql: string) => api.query(sql) as Promise<Array<Record<string, unknown>>>,
159163
{
160164
rulesTable: config.rulesTableName,
161165
goalsTable: config.goalsTableName,
162166
currentUser: config.userName,
163167
},
164-
{ log },
168+
{ log, tableExists },
165169
);
166170
}
167171
} catch (e: any) {

src/hooks/session-start.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,14 +210,19 @@ async function main(): Promise<void> {
210210
// It absorbs its own errors (missing table, network, etc.)
211211
// and returns "" on any failure — SessionStart MUST NOT fail
212212
// because of a bad rules read.
213+
// Trusted table list (cached — ensureTable above already warmed it)
214+
// so the renderer can skip the rules/goals SELECT when the table
215+
// isn't there yet, avoiding a 42P01 server-side on every SessionStart.
216+
const known = await api.knownTablesOrNull();
217+
const tableExists = known ? (name: string) => known.includes(name) : undefined;
213218
rulesBlock = await renderContextBlock(
214219
(sql: string) => api.query(sql) as Promise<Array<Record<string, unknown>>>,
215220
{
216221
rulesTable: config.rulesTableName,
217222
goalsTable: config.goalsTableName,
218223
currentUser: config.userName,
219224
},
220-
{ log },
225+
{ log, tableExists },
221226
);
222227
}
223228
} catch (e: any) {

src/hooks/shared/context-renderer.ts

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,15 @@ export interface RenderOptions {
4747
maxGoals?: number;
4848
/** Optional logger for debugging — receives line-by-line trace events. */
4949
log?: (msg: string) => void;
50+
/**
51+
* Optional existence predicate built from a trusted table list (see
52+
* DeeplakeApi.knownTablesOrNull). When provided and it reports a table
53+
* absent, we skip the SELECT entirely — a fresh workspace that never
54+
* created hivemind_rules / hivemind_goals would otherwise log a 42P01
55+
* server-side on every SessionStart. When omitted (e.g. the table list
56+
* couldn't be fetched), we fall back to the SELECT-then-catch path below.
57+
*/
58+
tableExists?: (name: string) => boolean;
5059
}
5160

5261
/**
@@ -82,25 +91,35 @@ export async function renderContextBlock(
8291
// Over-fetch so the "X more" truncation hint can give a useful
8392
// count. 4× the display cap balances "this user has a lot" against
8493
// unbounded reads on a busy org.
94+
const tableExists = opts.tableExists;
95+
8596
let rules: RuleRow[] = [];
86-
try {
87-
rules = await listRules(query, input.rulesTable, {
88-
status: "active",
89-
limit: Math.max(maxRules * 4, maxRules + 1),
90-
});
91-
} catch (rulesErr: unknown) {
92-
const rmsg = rulesErr instanceof Error ? rulesErr.message : String(rulesErr);
93-
log(`render-context-block: rules unavailable (continuing): ${rmsg}`);
97+
if (tableExists && !tableExists(input.rulesTable)) {
98+
log(`render-context-block: rules table "${input.rulesTable}" not present — skipping read`);
99+
} else {
100+
try {
101+
rules = await listRules(query, input.rulesTable, {
102+
status: "active",
103+
limit: Math.max(maxRules * 4, maxRules + 1),
104+
});
105+
} catch (rulesErr: unknown) {
106+
const rmsg = rulesErr instanceof Error ? rulesErr.message : String(rulesErr);
107+
log(`render-context-block: rules unavailable (continuing): ${rmsg}`);
108+
}
94109
}
95110

96111
let goals: OpenGoalRow[] = [];
97-
try {
98-
goals = await listOpenGoals(query, input.goalsTable, input.currentUser, {
99-
limit: Math.max(maxGoals * 4, maxGoals + 1),
100-
});
101-
} catch (goalsErr: unknown) {
102-
const gmsg = goalsErr instanceof Error ? goalsErr.message : String(goalsErr);
103-
log(`render-context-block: goals unavailable (continuing): ${gmsg}`);
112+
if (tableExists && !tableExists(input.goalsTable)) {
113+
log(`render-context-block: goals table "${input.goalsTable}" not present — skipping read`);
114+
} else {
115+
try {
116+
goals = await listOpenGoals(query, input.goalsTable, input.currentUser, {
117+
limit: Math.max(maxGoals * 4, maxGoals + 1),
118+
});
119+
} catch (goalsErr: unknown) {
120+
const gmsg = goalsErr instanceof Error ? goalsErr.message : String(goalsErr);
121+
log(`render-context-block: goals unavailable (continuing): ${gmsg}`);
122+
}
104123
}
105124

106125
const rulesShown = rules.slice(0, maxRules);

src/skillify/auto-pull.ts

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,14 @@ export async function autoPullSkills(deps: AutoPullDeps = {}): Promise<AutoPullR
8989

9090
// Build the query function. Tests inject one; real callers get the API client.
9191
let query: QueryFn;
92+
// Deferred table-existence discovery — resolved INSIDE the timeout budget
93+
// below, NOT here, so a slow `GET /tables` can't make SessionStart block
94+
// past timeoutMs. Resolves to a predicate that lets runPull skip the SELECT
95+
// (and the server-side 42P01) when `skills` doesn't exist yet on a fresh
96+
// workspace, or undefined when the list can't be fetched / a test injects
97+
// its own query (runPull then falls back to its isMissingTableError catch).
98+
let discoverTableExists: () => Promise<((name: string) => boolean) | undefined> =
99+
async () => undefined;
92100
if (deps.queryFn) {
93101
query = deps.queryFn;
94102
} else {
@@ -100,22 +108,32 @@ export async function autoPullSkills(deps: AutoPullDeps = {}): Promise<AutoPullR
100108
config.skillsTableName,
101109
);
102110
query = (sql: string) => api.query(sql) as Promise<Record<string, unknown>[]>;
111+
discoverTableExists = async () => {
112+
const known = await api.knownTablesOrNull();
113+
return known ? (name: string) => known.includes(name) : undefined;
114+
};
103115
}
104116

105117
const install = deps.install ?? "global";
106118
const timeoutMs = deps.timeoutMs ?? DEFAULT_TIMEOUT_MS;
107119

108120
try {
109121
const summary = await withTimeout(
110-
runPull({
111-
query,
112-
tableName: config.skillsTableName,
113-
install,
114-
cwd: install === "project" ? (deps.cwd ?? process.cwd()) : undefined,
115-
users: [],
116-
dryRun: false,
117-
force: false,
118-
}),
122+
// Table discovery + pull share one budget: if `GET /tables` hangs the
123+
// whole thing times out and we degrade, instead of blocking startup.
124+
(async () => {
125+
const tableExists = await discoverTableExists();
126+
return runPull({
127+
query,
128+
tableName: config.skillsTableName,
129+
install,
130+
cwd: install === "project" ? (deps.cwd ?? process.cwd()) : undefined,
131+
users: [],
132+
dryRun: false,
133+
force: false,
134+
tableExists,
135+
});
136+
})(),
119137
timeoutMs,
120138
);
121139
log(`pulled scanned=${summary.scanned} wrote=${summary.wrote} skipped=${summary.skipped}`);

src/skillify/pull.ts

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,15 @@ export interface PullOptions {
5959
dryRun?: boolean;
6060
/** Overwrite even when local version >= remote. Backs up the existing file. */
6161
force?: boolean;
62+
/**
63+
* Optional existence predicate built from a trusted table list (see
64+
* DeeplakeApi.knownTablesOrNull). When it reports the skills table absent
65+
* we skip the SELECT and treat it as empty — a fresh workspace lazily
66+
* creates `skills` on first INSERT, so reading it first otherwise logs a
67+
* 42P01 server-side on every SessionStart auto-pull. Omitted (or the list
68+
* couldn't be fetched) falls back to the SELECT-then-catch path below.
69+
*/
70+
tableExists?: (name: string) => boolean;
6271
}
6372

6473
export interface PullResultEntry {
@@ -463,21 +472,29 @@ export async function runPull(opts: PullOptions): Promise<PullSummary> {
463472
// hasn't been lazy-migrated by an INSERT yet) retry once with the
464473
// legacy SELECT shape so the pull keeps working until the next write.
465474
let rows: Record<string, unknown>[] = [];
466-
try {
467-
rows = await opts.query(sql);
468-
} catch (e: any) {
469-
if (isMissingTableError(e?.message)) {
470-
rows = [];
471-
} else if (isMissingContributorsColumnError(e?.message)) {
472-
const legacySql = buildPullSql({
473-
tableName: opts.tableName,
474-
users: opts.users,
475-
skillName: opts.skillName,
476-
includeContributors: false,
477-
});
478-
rows = await opts.query(legacySql);
479-
} else {
480-
throw e;
475+
if (opts.tableExists && !opts.tableExists(opts.tableName)) {
476+
// Known-absent from a trusted table list: skip the SELECT so a fresh
477+
// workspace doesn't log a 42P01 server-side. Leaves rows empty, the same
478+
// outcome as the isMissingTableError catch below, minus the round-trip
479+
// and the error.
480+
rows = [];
481+
} else {
482+
try {
483+
rows = await opts.query(sql);
484+
} catch (e: any) {
485+
if (isMissingTableError(e?.message)) {
486+
rows = [];
487+
} else if (isMissingContributorsColumnError(e?.message)) {
488+
const legacySql = buildPullSql({
489+
tableName: opts.tableName,
490+
users: opts.users,
491+
skillName: opts.skillName,
492+
includeContributors: false,
493+
});
494+
rows = await opts.query(legacySql);
495+
} else {
496+
throw e;
497+
}
481498
}
482499
}
483500
const latest = selectLatestPerName(rows);

tests/claude-code/cli-context.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
88
*/
99

1010
const queryMock = vi.fn();
11+
const knownTablesMock = vi.fn();
1112

1213
vi.mock("../../src/config.js", () => ({
1314
loadConfig: vi.fn(),
@@ -23,6 +24,7 @@ vi.mock("../../src/deeplake-api.js", () => ({
2324
_tableName: string,
2425
) { /* nothing */ }
2526
query(sql: string) { return queryMock(sql); }
27+
async knownTablesOrNull() { return knownTablesMock(); }
2628
},
2729
}));
2830

@@ -55,6 +57,9 @@ beforeEach(() => {
5557
logged = [];
5658
erred = [];
5759
queryMock.mockReset().mockResolvedValue([]);
60+
// Default: untrusted table list (null) → renderer SELECTs unconditionally.
61+
// Individual tests override to return a trusted list and exercise gating.
62+
knownTablesMock.mockReset().mockResolvedValue(null);
5863
loadConfigMock.mockReset().mockReturnValue(VALID_CONFIG);
5964
logSpy = vi.spyOn(console, "log").mockImplementation((...a: any[]) => { logged.push(a.join(" ")); });
6065
errSpy = vi.spyOn(console, "error").mockImplementation((...a: any[]) => { erred.push(a.join(" ")); });
@@ -151,6 +156,32 @@ describe("runContextCommand — output", () => {
151156
expect(erred.some(l => l.includes("(no active rules or open goals)"))).toBe(true);
152157
});
153158

159+
it("gates the SELECTs on knownTablesOrNull when the table list is trusted", async () => {
160+
// A non-null table list lets renderContextBlock skip the SELECT for any
161+
// absent table (avoids a 42P01 server-side). Both present here, so both
162+
// SELECTs still run — and the tableExists predicate is exercised.
163+
knownTablesMock.mockResolvedValue(["hivemind_rules", "hivemind_goals"]);
164+
queryMock.mockResolvedValueOnce([{
165+
id: "row-1", rule_id: "rule-1", text: "no DROP TABLE on prod",
166+
scope: "team", status: "active", assigned_by: "alice@activeloop.ai",
167+
version: 1, created_at: "2026-05-20T10:00:00Z",
168+
agent: "manual", plugin_version: "0.7.99",
169+
}]);
170+
queryMock.mockResolvedValueOnce([]); // goals empty
171+
await runContextCommand([]);
172+
expect(logged.some(l => l.includes("no DROP TABLE on prod"))).toBe(true);
173+
expect(queryMock).toHaveBeenCalledTimes(2);
174+
});
175+
176+
it("skips every SELECT when the trusted table list omits rules + goals", async () => {
177+
// Fresh workspace: knownTablesOrNull returns [] → no SELECT may fire
178+
// (each would log a 42P01 server-side). Empty render → stderr diagnostic.
179+
knownTablesMock.mockResolvedValue([]);
180+
await runContextCommand([]);
181+
expect(queryMock).not.toHaveBeenCalled();
182+
expect(erred.some(l => l.includes("(no active rules or open goals)"))).toBe(true);
183+
});
184+
154185
it("uses the configured table names from cfg (not hardcoded)", async () => {
155186
loadConfigMock.mockReturnValueOnce({
156187
...VALID_CONFIG,

0 commit comments

Comments
 (0)