mirror of
https://github.com/aaronsb/obsidian-mcp-plugin.git
synced 2026-07-22 06:45:14 +00:00
feat(vault): return combined content inline when destination is omitted
`vault combine` without a `destination` now returns the combined content
inline in the response instead of erroring. Read-only consumers can use
combine for multi-file retrieval with no side effects.
- router.ts: no-destination path returns { inline, content, ... }; the
destination-exists guard is skipped when there is no destination.
- formatters/vault.ts: FileCombineResponse gains optional destination +
inline/content; formatFileCombine renders an inline block.
- semantic-tools.ts: combine counts as a write-op (blocked in read-only
mode) only when a destination is given; param description updated.
- tests: formatter inline/destination/fallback coverage.
Reimplemented from PR #146 against current `main` (code only); supersedes
and closes out #146. Dropped the original PR's
`path: destination || '_inline_'` validator workaround — the batch.combine
validators (BatchLimitValidator, PathArrayValidator) never inspect `path`,
so the hack was unnecessary. The README/troubleshooting cert-trust docs
that #146 also carried are covered by the #143 reimplementation instead, to
avoid duplicate divergent copies.
Co-authored-by: Earl Plak <5597016+laplaque@users.noreply.github.com>
This commit is contained in:
parent
c46d5e13b2
commit
f10aea7d54
4 changed files with 117 additions and 19 deletions
|
|
@ -407,7 +407,9 @@ export function formatFileSplit(response: FileSplitResponse): string {
|
|||
*/
|
||||
export interface FileCombineResponse {
|
||||
success: boolean;
|
||||
destination: string;
|
||||
destination?: string;
|
||||
inline?: boolean;
|
||||
content?: string;
|
||||
filesCombined: number;
|
||||
totalSize?: number;
|
||||
sourceFiles?: string[];
|
||||
|
|
@ -417,11 +419,27 @@ export function formatFileCombine(response: FileCombineResponse): string {
|
|||
const lines: string[] = [];
|
||||
|
||||
const icon = response.success ? '✓' : '✗';
|
||||
|
||||
// Inline mode: no file was written — return the combined content directly
|
||||
if (response.inline && response.content !== undefined) {
|
||||
lines.push(header(1, `${icon} Combined ${response.filesCombined} files (inline)`));
|
||||
lines.push('');
|
||||
if (response.totalSize !== undefined) {
|
||||
lines.push(property('Total size', formatFileSize(response.totalSize), 0));
|
||||
lines.push('');
|
||||
}
|
||||
lines.push(response.content);
|
||||
lines.push('');
|
||||
lines.push(divider());
|
||||
lines.push(summaryFooter());
|
||||
return joinLines(lines);
|
||||
}
|
||||
|
||||
lines.push(header(1, `${icon} Combined: ${response.destination}`));
|
||||
lines.push('');
|
||||
|
||||
if (response.success) {
|
||||
lines.push(property('Destination', response.destination, 0));
|
||||
lines.push(property('Destination', response.destination ?? '(none)', 0));
|
||||
lines.push(property('Files combined', response.filesCombined.toString(), 0));
|
||||
if (response.totalSize !== undefined) {
|
||||
lines.push(property('Total size', formatFileSize(response.totalSize), 0));
|
||||
|
|
|
|||
|
|
@ -653,20 +653,20 @@ export class SemanticRouter {
|
|||
throw new Error('paths array is required for combine operation');
|
||||
}
|
||||
|
||||
if (!destination) {
|
||||
throw new Error('destination is required for combine operation');
|
||||
}
|
||||
|
||||
// Check if destination exists
|
||||
try {
|
||||
const destFile = await this.api.getFile(destination);
|
||||
if (destFile && !overwrite) {
|
||||
throw new Error(`Destination already exists: ${destination}. Set overwrite=true to replace.`);
|
||||
// When a destination is given, refuse to clobber it unless overwrite.
|
||||
// When omitted, the combined content is returned inline (no write) —
|
||||
// see the inline branch below.
|
||||
if (destination) {
|
||||
try {
|
||||
const destFile = await this.api.getFile(destination);
|
||||
if (destFile && !overwrite) {
|
||||
throw new Error(`Destination already exists: ${destination}. Set overwrite=true to replace.`);
|
||||
}
|
||||
} catch {
|
||||
// File doesn't exist, which is what we want
|
||||
}
|
||||
} catch {
|
||||
// File doesn't exist, which is what we want
|
||||
}
|
||||
|
||||
|
||||
// Validate and get all source files
|
||||
const sourceFiles = [];
|
||||
for (const path of paths) {
|
||||
|
|
@ -697,14 +697,37 @@ export class SemanticRouter {
|
|||
}
|
||||
|
||||
const finalContent = combinedContent.join(separator);
|
||||
|
||||
|
||||
// No destination → return the combined content inline without writing
|
||||
// to the vault. Lets read-only consumers use combine for multi-file
|
||||
// retrieval with no side effects.
|
||||
if (!destination) {
|
||||
return {
|
||||
success: true,
|
||||
inline: true,
|
||||
content: finalContent,
|
||||
filesCombined: paths.length,
|
||||
totalSize: finalContent.length,
|
||||
sourceFiles: paths,
|
||||
workflow: {
|
||||
message: `Combined ${paths.length} files inline (no file written)`,
|
||||
suggested_next: [
|
||||
{
|
||||
description: 'Save the combined content to a file',
|
||||
command: `vault(action='combine', paths=${JSON.stringify(paths)}, destination='combined.md')`
|
||||
}
|
||||
]
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
// Create or update destination file
|
||||
if (overwrite) {
|
||||
await this.api.updateFile(destination, finalContent);
|
||||
} else {
|
||||
await this.api.createFile(destination, finalContent);
|
||||
}
|
||||
|
||||
|
||||
return {
|
||||
success: true,
|
||||
destination,
|
||||
|
|
|
|||
|
|
@ -140,8 +140,12 @@ const createSemanticTool = (operation: string, visibility?: ToolVisibility): Sem
|
|||
// Check for read-only mode before processing write operations
|
||||
const plugin = (api as unknown as { plugin?: PluginWithSettings }).plugin;
|
||||
if (plugin?.settings?.readOnlyMode && operation === 'vault') {
|
||||
const writeOperations = ['create', 'update', 'delete', 'move', 'rename', 'copy', 'split', 'combine', 'concatenate'];
|
||||
if (writeOperations.includes(args.action)) {
|
||||
const writeOperations = ['create', 'update', 'delete', 'move', 'rename', 'copy', 'split', 'concatenate'];
|
||||
// combine writes a file only when a destination is given; without one it
|
||||
// returns content inline (no side effects) and is safe in read-only mode.
|
||||
const isWriteOp = writeOperations.includes(args.action) ||
|
||||
(args.action === 'combine' && Boolean(args.destination));
|
||||
if (isWriteOp) {
|
||||
return {
|
||||
content: [{
|
||||
type: 'text' as const,
|
||||
|
|
@ -454,7 +458,7 @@ function getParametersForOperation(operation: string): Record<string, unknown> {
|
|||
},
|
||||
destination: {
|
||||
type: 'string',
|
||||
description: 'Destination path for move/copy operations'
|
||||
description: 'Destination path for move/copy/combine operations. For combine: omit to return the combined content inline without writing a file'
|
||||
},
|
||||
newName: {
|
||||
type: 'string',
|
||||
|
|
|
|||
53
tests/combine-inline.test.ts
Normal file
53
tests/combine-inline.test.ts
Normal file
|
|
@ -0,0 +1,53 @@
|
|||
import { formatFileCombine, FileCombineResponse } from '../src/formatters/vault';
|
||||
|
||||
describe('formatFileCombine — inline (no destination) mode', () => {
|
||||
test('renders combined content inline without a Destination line', () => {
|
||||
const response: FileCombineResponse = {
|
||||
success: true,
|
||||
inline: true,
|
||||
content: 'alpha\n\n---\n\nbeta',
|
||||
filesCombined: 2,
|
||||
totalSize: 16,
|
||||
sourceFiles: ['a.md', 'b.md'],
|
||||
};
|
||||
|
||||
const out = formatFileCombine(response);
|
||||
|
||||
expect(out).toContain('Combined 2 files (inline)');
|
||||
expect(out).toContain('alpha');
|
||||
expect(out).toContain('beta');
|
||||
// Inline mode wrote no file, so it must not claim a destination
|
||||
expect(out).not.toContain('Destination');
|
||||
});
|
||||
|
||||
test('still renders destination mode when a destination is present', () => {
|
||||
const response: FileCombineResponse = {
|
||||
success: true,
|
||||
destination: 'combined.md',
|
||||
filesCombined: 2,
|
||||
totalSize: 16,
|
||||
sourceFiles: ['a.md', 'b.md'],
|
||||
};
|
||||
|
||||
const out = formatFileCombine(response);
|
||||
|
||||
expect(out).toContain('Combined: combined.md');
|
||||
expect(out).toContain('Destination');
|
||||
expect(out).not.toContain('(inline)');
|
||||
});
|
||||
|
||||
test('inline branch requires content to be defined', () => {
|
||||
// inline:true but content undefined falls through to destination mode
|
||||
const response: FileCombineResponse = {
|
||||
success: true,
|
||||
inline: true,
|
||||
destination: 'fallback.md',
|
||||
filesCombined: 1,
|
||||
};
|
||||
|
||||
const out = formatFileCombine(response);
|
||||
|
||||
expect(out).toContain('Combined: fallback.md');
|
||||
expect(out).not.toContain('(inline)');
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue