mirror of
https://github.com/aaronsb/obsidian-mcp-plugin.git
synced 2026-07-22 06:45:14 +00:00
fix(dataview): serialize Luxon dates and propagate Dataview-internal errors
Closes #115. Closes #123. The Dataview API returns Luxon DateTime objects (which expose toISO(), not toISOString()) for file.ctime/mtime, so listPages() and getPageMetadata() crashed with "toISOString is not a function" the moment any real Dataview result reached them. The existing tests passed only because the mocks used native Date, which exposes toISOString(). The query path had a separate envelope bug: executeQuery() hard-coded success: true whenever dataviewAPI.query() didn't throw, while Dataview itself returns { successful: false, error: "..." } for malformed queries without throwing. The formatter then read response.successful (undefined), hit the failure branch, and rendered every result as "❌ Query failed: Unknown error" — even when the query had succeeded. - Add toIsoOptional() that prefers Luxon's toISO() and falls back to Date.toISOString(); apply at every page.file.ctime/mtime serialization site and inside convertDataviewValue(). - Propagate result.successful and result.error from the inner Dataview response to the outer tool envelope. - Add a dataview.query normalizer case in the formatter dispatcher that flattens result.{type,values,headers} to the top level and maps success → successful, matching the formatter's contract. - Regression tests for both bugs, including a Luxon-shaped mock distinct from the existing Date-shaped one.
This commit is contained in:
parent
dfa5affad7
commit
b5b1ebdfd2
3 changed files with 150 additions and 12 deletions
|
|
@ -339,6 +339,30 @@ function normalizeResponse(key: string, response: unknown): NormalizedResponse {
|
|||
return resp;
|
||||
}
|
||||
|
||||
// dataview.query: tool returns
|
||||
// {success, query, format, result: {type, values, headers?} | {type:'unknown', data}, type, error?, ...}
|
||||
// Formatter expects {query, type, values?, headers?, successful, error?} at top level.
|
||||
// Flatten result.{type,values,headers} and rename success → successful so the
|
||||
// formatter renders typed results and surfaces Dataview's own error messages.
|
||||
case 'dataview.query': {
|
||||
const dvResp = resp as {
|
||||
success?: boolean;
|
||||
query?: string;
|
||||
error?: string;
|
||||
type?: string;
|
||||
result?: { type?: string; values?: unknown; headers?: unknown };
|
||||
};
|
||||
const inner = dvResp.result;
|
||||
return {
|
||||
...resp,
|
||||
successful: dvResp.success ?? true,
|
||||
type: inner?.type ?? dvResp.type ?? 'list',
|
||||
values: inner?.values,
|
||||
headers: inner?.headers,
|
||||
error: dvResp.error
|
||||
};
|
||||
}
|
||||
|
||||
// edit.window: router returns {isError, content}, formatter expects {success, path}
|
||||
case 'edit.window':
|
||||
case 'edit.from_buffer': {
|
||||
|
|
|
|||
|
|
@ -14,9 +14,33 @@ interface DataviewArray<T = unknown> {
|
|||
map<U>(fn: (item: T) => U): DataviewArray<U>;
|
||||
}
|
||||
|
||||
/** Dataview date/time value with ISO serialization */
|
||||
/**
|
||||
* Dataview date/time value. Dataview emits Luxon DateTime objects, which
|
||||
* expose `toISO()` (returns `string | null`). Some test fixtures pass native
|
||||
* `Date` objects that only expose `toISOString()`. We accept either shape.
|
||||
*/
|
||||
interface DataviewDateTime {
|
||||
toISOString(): string;
|
||||
toISO?(): string | null;
|
||||
toISOString?(): string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Serialize a Dataview date value to ISO 8601, preferring Luxon's `toISO()`
|
||||
* and falling back to native `Date.toISOString()`. Returns `undefined` for
|
||||
* nullish input, invalid Luxon dates, or values that expose neither method —
|
||||
* which lets `?.` propagate cleanly and keeps the response shape stable.
|
||||
*/
|
||||
function toIsoOptional(value: unknown): string | undefined {
|
||||
if (value == null) return undefined;
|
||||
const dt = value as DataviewDateTime;
|
||||
if (typeof dt.toISO === 'function') {
|
||||
const iso = dt.toISO();
|
||||
if (iso) return iso;
|
||||
}
|
||||
if (typeof dt.toISOString === 'function') {
|
||||
return dt.toISOString();
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/** Dataview link value */
|
||||
|
|
@ -62,6 +86,7 @@ interface DataviewQueryResult {
|
|||
type: string;
|
||||
values?: DataviewArray;
|
||||
headers?: string[];
|
||||
error?: string;
|
||||
}
|
||||
|
||||
/** Row within a table result */
|
||||
|
|
@ -150,14 +175,18 @@ export class DataviewTool {
|
|||
|
||||
try {
|
||||
if (format === 'dql') {
|
||||
// Execute DQL query
|
||||
// Execute DQL query. Dataview returns {successful: false, error}
|
||||
// for syntax/runtime errors rather than throwing — propagate that
|
||||
// status to the outer envelope so the formatter can render it.
|
||||
const result: DataviewQueryResult = await dataviewAPI.query(query);
|
||||
const innerSuccess = result.successful !== false;
|
||||
return {
|
||||
success: true,
|
||||
success: innerSuccess,
|
||||
query,
|
||||
format,
|
||||
result: this.formatQueryResult(result),
|
||||
type: result.type || 'unknown',
|
||||
error: innerSuccess ? undefined : result.error,
|
||||
workflow: this.generateQueryWorkflow(query, result),
|
||||
hints: this.generateQueryHints(query)
|
||||
};
|
||||
|
|
@ -199,8 +228,8 @@ export class DataviewTool {
|
|||
path: page.file.path,
|
||||
name: page.file.name,
|
||||
size: page.file.size,
|
||||
created: page.file.ctime?.toISOString(),
|
||||
modified: page.file.mtime?.toISOString(),
|
||||
created: toIsoOptional(page.file.ctime),
|
||||
modified: toIsoOptional(page.file.mtime),
|
||||
tags: page.file.tags?.array() ?? [],
|
||||
links: page.file.outlinks?.array()?.length ?? 0,
|
||||
aliases: page.aliases?.array() ?? [],
|
||||
|
|
@ -244,8 +273,8 @@ export class DataviewTool {
|
|||
basename: page.file.basename,
|
||||
extension: page.file.extension,
|
||||
size: page.file.size,
|
||||
created: page.file.ctime?.toISOString(),
|
||||
modified: page.file.mtime?.toISOString()
|
||||
created: toIsoOptional(page.file.ctime),
|
||||
modified: toIsoOptional(page.file.mtime)
|
||||
},
|
||||
tags: page.file.tags?.array() ?? [],
|
||||
aliases: page.aliases?.array() ?? [],
|
||||
|
|
@ -387,10 +416,11 @@ export class DataviewTool {
|
|||
return dvArray.array().map((item: unknown) => this.convertDataviewValue(item));
|
||||
}
|
||||
|
||||
// Handle Dataview dates
|
||||
const dvDate = value as { toISOString?: () => string };
|
||||
if (typeof dvDate.toISOString === 'function') {
|
||||
return dvDate.toISOString();
|
||||
// Handle Dataview dates (Luxon DateTime exposes toISO(), native Date toISOString())
|
||||
const dvDate = value as DataviewDateTime;
|
||||
if (typeof dvDate.toISO === 'function' || typeof dvDate.toISOString === 'function') {
|
||||
const iso = toIsoOptional(value);
|
||||
if (iso !== undefined) return iso;
|
||||
}
|
||||
|
||||
// Handle Dataview links
|
||||
|
|
|
|||
|
|
@ -273,6 +273,90 @@ describe('Dataview Integration', () => {
|
|||
expect(result.metadata.custom.priority).toBe('high');
|
||||
});
|
||||
|
||||
test('should serialize Luxon DateTime values from listPages without crashing', async () => {
|
||||
// Regression for #123 bug 2: Dataview emits Luxon DateTime objects
|
||||
// (toISO() returns string|null) and not native Date.toISOString().
|
||||
const luxonDate = (iso: string) => ({ toISO: () => iso });
|
||||
const app = new MockApp(true, true);
|
||||
const api = new MockObsidianAPI(app);
|
||||
const tool = new DataviewTool(api as any);
|
||||
|
||||
const dvApi = (app.plugins.plugins['dataview'] as any).api as MockDataviewAPI;
|
||||
dvApi.pages = (() => ({
|
||||
length: 1,
|
||||
array: () => [{
|
||||
file: {
|
||||
path: 'Luxon.md',
|
||||
name: 'Luxon.md',
|
||||
size: 42,
|
||||
ctime: luxonDate('2026-01-02T03:04:05.000Z'),
|
||||
mtime: luxonDate('2026-01-02T03:04:06.000Z'),
|
||||
tags: { array: () => [] },
|
||||
outlinks: { array: () => [] }
|
||||
},
|
||||
aliases: { array: () => [] }
|
||||
}]
|
||||
})) as any;
|
||||
|
||||
const result = tool.listPages() as any;
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.pages[0].created).toBe('2026-01-02T03:04:05.000Z');
|
||||
expect(result.pages[0].modified).toBe('2026-01-02T03:04:06.000Z');
|
||||
});
|
||||
|
||||
test('should serialize Luxon DateTime values from getPageMetadata without crashing', async () => {
|
||||
// Regression for #123 bug 3
|
||||
const luxonDate = (iso: string) => ({ toISO: () => iso });
|
||||
const app = new MockApp(true, true);
|
||||
const api = new MockObsidianAPI(app);
|
||||
const tool = new DataviewTool(api as any);
|
||||
|
||||
const dvApi = (app.plugins.plugins['dataview'] as any).api as MockDataviewAPI;
|
||||
dvApi.page = (path: string) => path === 'Luxon.md' ? {
|
||||
file: {
|
||||
path: 'Luxon.md',
|
||||
name: 'Luxon.md',
|
||||
basename: 'Luxon',
|
||||
extension: 'md',
|
||||
size: 42,
|
||||
ctime: luxonDate('2026-01-02T03:04:05.000Z'),
|
||||
mtime: luxonDate('2026-01-02T03:04:06.000Z'),
|
||||
tags: { array: () => [] },
|
||||
outlinks: { array: () => [] },
|
||||
inlinks: { array: () => [] },
|
||||
tasks: { array: () => [] },
|
||||
lists: { array: () => [] }
|
||||
},
|
||||
aliases: { array: () => [] }
|
||||
} as any : null;
|
||||
|
||||
const result = tool.getPageMetadata('Luxon.md') as any;
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.metadata.file.created).toBe('2026-01-02T03:04:05.000Z');
|
||||
expect(result.metadata.file.modified).toBe('2026-01-02T03:04:06.000Z');
|
||||
});
|
||||
|
||||
test('should propagate Dataview-internal query failures to outer envelope', async () => {
|
||||
// Regression for #115 / #123 bug 1: Dataview returns
|
||||
// {successful: false, error: "..."}
|
||||
// when a query is malformed instead of throwing. The tool used to
|
||||
// hard-code success:true regardless, hiding the real error.
|
||||
const app = new MockApp(true, true);
|
||||
const api = new MockObsidianAPI(app);
|
||||
const tool = new DataviewTool(api as any);
|
||||
|
||||
const dvApi = (app.plugins.plugins['dataview'] as any).api as MockDataviewAPI;
|
||||
dvApi.query = (() => ({
|
||||
successful: false,
|
||||
type: 'table',
|
||||
error: 'No field "nonexistent" on this page'
|
||||
})) as any;
|
||||
|
||||
const result = await tool.executeQuery('TABLE nonexistent FROM ""') as any;
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error).toBe('No field "nonexistent" on this page');
|
||||
});
|
||||
|
||||
test('should validate queries', async () => {
|
||||
const app = new MockApp(true, true);
|
||||
const api = new MockObsidianAPI(app);
|
||||
|
|
|
|||
Loading…
Reference in a new issue