Skip to content

feat(ai): LLM tool/function calling — ai.agent() + built-in tools (#714)#1372

Merged
chhoumann merged 8 commits into
masterfrom
chhoumann/714-llm-function-calling
Jun 17, 2026
Merged

feat(ai): LLM tool/function calling — ai.agent() + built-in tools (#714)#1372
chhoumann merged 8 commits into
masterfrom
chhoumann/714-llm-function-calling

Conversation

@chhoumann

@chhoumann chhoumann commented Jun 17, 2026

Copy link
Copy Markdown
Owner

Closes #714.

Adds LLM tool / function calling to QuickAdd's AI Assistant, exposed through the script API as an AI-SDK-shaped agent primitive. You give a model a prompt plus a set of tools (JS functions); it calls them in a bounded multi-step loop until it has an answer. Optional JSON-schema structured output is included.

module.exports = async ({ quickAddApi }) => {
  const agent = quickAddApi.ai.agent({
    model: "gpt-5",
    system: "You are a vault librarian. Ground every claim in the user's notes.",
    tools: { ...quickAddApi.ai.tools.vault({ only: ["read_note", "search_notes"] }) },
    stopWhen: quickAddApi.ai.stepCountIs(20),
  });
  const { text } = await agent.generate({ prompt: "What do my notes say about gardening?" });
  return text;
};

Surface (quickAddApi.ai)

  • ai.agent(config).generate({ prompt, schema?, assignToVariable? }) — bounded tool loop; with schema, returns a validated object (one bounded repair re-ask).
  • ai.tool({ description, inputSchema, execute, needsApproval?, readOnly?, strict? }) — declare your own tool.
  • ai.tools.{vault,workspace,system}(options) — opt-in built-in tools (only/exclude/prefix/allowedRoots).
  • ai.stepCountIs, ai.hasToolCall; assignToVariable alias added to ai.prompt/ai.chunkedPrompt.

Built-in tools: reads (read_note, list_notes, search_notes, get_property_values, get_active_note, get_selection, get_date) auto-run; writes (create_note, append_to_note, insert_under_heading) ask for approval. High-risk tools (run_choice/apply_template/set_frontmatter/delete_note) are intentionally not shipped in v1.

Design

  • One pure, provider-neutral layer (src/ai/tools/, no Obsidian imports) maps to/from the three wire shapes — OpenAI-compatible, Anthropic Messages, Gemini generateContent — so the loop + mappers are fully unit-testable. The Obsidian-bound Agent is a thin shell over it.
  • Invariant: a turn sends EITHER tools OR a response-format schema, never both (avoids OpenAI suppressing tool calls; the loop always lands on a forced-final text turn).
  • No new sandbox needed — tool handlers are author JS (already fully privileged). The new risk is model-chosen tool + args + loop runaway + indirect prompt injection, controlled by: opt-in registry, per-tool needsApproval + global Confirm AI tool calls setting (default destructive only), step / per-result-byte / cumulative-transcript-byte caps, per-iteration online-disabled recheck, JSON-schema arg validation, path sanitisation + symlink/realpath vault containment, and never running model args through format(). Package-preview discloses an AI TOOLS capability for any bundled script that uses this.

Validation

  • Live e2e — tool loop + structured output both pass on: OpenAI gpt-5-mini & gpt-5.5, Anthropic claude-opus-4-8 & claude-sonnet-4-6, Gemini gemini-3-flash-preview (incl. the Gemini-3 thoughtSignature echo round-trip). e2e is env-gated, so CI never hits the network.
  • Found + fixed two wire bugs for current models in the process: GPT-5.x/o-series need max_completion_tokens (not max_tokens); Anthropic output_config.format requires additionalProperties:false injected into the schema.
  • Full unit suite: 2521 passing; tsc + eslint clean; bundle 1.1 mb.

Notes

  • Docs target the unreleased (Next) version only.
  • Confirmation requires an interactive session; for unattended automation use read-only tools or set the global setting (documented).

Summary by CodeRabbit

Release Notes

  • New Features

    • Added AI Agent framework supporting multi-step tool calling with built-in vault, workspace, and system tools
    • Added tool confirmation settings with options for "Destructive tools only," "Always confirm," or "Never"
    • Added support for Gemini 3 models (gemini-3-pro, gemini-3-flash)
    • Added structured output support with JSON schema validation
  • Documentation

    • Updated AI Assistant settings documentation
    • Expanded QuickAdd API reference with agent and tool-calling examples

Add the internal layer for LLM tool/function calling, shared across QuickAdd's
three provider wire shapes (OpenAI-compatible, Anthropic Messages, Gemini
generateContent):

- NormalizedTools: provider-neutral request/message/tool-call types.
- providerToolMapping: buildChatBody/parseChatResponse per provider, incl.
  OpenAI JSON-string arg parsing, Anthropic tool_result turns + native
  output_config.format (additionalProperties injected, as Anthropic requires),
  Gemini functionCall/Response + thoughtSignature echo, and
  max_completion_tokens for GPT-5.x/o-series reasoning models.
- runToolLoop: pure bounded multi-step loop (forced-final text turn, one result
  per call id, step/result-byte/cumulative-transcript-byte caps, graceful
  context-overflow + online-disabled stops).
- jsonSchemaValidator: registration-time keyword allowlist + value validation.
- sanitizeVaultPath: every-segment dot-floor + traversal/absolute rejection.
- OpenAIRequest: chatRequest() dispatch + Anthropic request-path fix; legacy
  single-shot routing unified on provider kind.
- Provider: ProviderKind discriminator + getProviderKind() + migration.
)

Expose the AI-SDK-shaped surface on quickAddApi.ai and ship opt-in built-in
tools:

- ai.agent(config).generate({ prompt, schema?, assignToVariable? }) — bounded
  tool loop + optional JSON-schema structured output (one bounded repair).
- ai.tool(), ai.stepCountIs, ai.hasToolCall; assignToVariable alias on
  ai.prompt/ai.chunkedPrompt (reserved-name guarded, trimmed at write).
- ai.tools.{vault,workspace,system}(options): reads auto-run; writes ask for
  approval, sanitise paths, fail-rather-than-overwrite, frontmatter-aware,
  symlink/realpath-contained. High-risk tools deferred.
- Per-tool needsApproval + global ai.confirmToolCalls setting (default
  destructive; undefined→destructive for pre-existing settings) via
  AIToolConfirmModal (approve / approve-all-this-run / deny / abort).
- EITHER-tools-OR-responseFormat per turn; per-run reentrancy guard.
A bundled script that wires up AI tool calling can let a model read/write the
vault with model-chosen arguments. Scan the same decoded code the loader runs
(including a js fence hidden in a .md note) and surface a distinct critical
"AI TOOLS" capability row.
…714)

Env-gated (skipped without each provider's API key, so CI never hits the
network). Each exercises the real pure modules end-to-end against a current
model: tool loop + schema-constrained structured output. Validated live on
gpt-5-mini/gpt-5.5, claude-sonnet-4-6/claude-opus-4-8, gemini-3-flash-preview.
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5d045792-a156-4706-8b6f-f6deb5ba489b

📥 Commits

Reviewing files that changed from the base of the PR and between 236528a and 2612f25.

📒 Files selected for processing (3)
  • src/ai/OpenAIRequest.test.ts
  • src/ai/OpenAIRequest.ts
  • src/settings.ts
💤 Files with no reviewable changes (1)
  • src/settings.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/ai/OpenAIRequest.test.ts
  • src/ai/OpenAIRequest.ts

📝 Walkthrough

Walkthrough

Adds a multi-provider AI agent/tool-calling system to QuickAdd. Introduces ProviderKind discrimination, normalized wire contracts, per-provider chat body builders and response parsers for OpenAI/Anthropic/Gemini, a bounded agentic tool execution loop, built-in vault/workspace/system tool groups with path safety guards, an Agent orchestrator with human-in-the-loop confirmation, public API exposure, package preview scanning, and documentation updates.

Changes

AI Agent Tool Calling

Layer / File(s) Summary
ProviderKind type, inference, and provider constant backfill
src/ai/Provider.ts, src/ai/Provider.test.ts
Adds ProviderKind union, getProviderKind resolver with name/endpoint hostname inference and spoofing-resistant matching, extends AIProvider with optional kind, stamps built-in provider constants, and tests all inference paths.
Normalized tool/chat contracts and JSON schema validator
src/ai/tools/NormalizedTools.ts, src/ai/tools/aiToolTypes.ts, src/ai/tools/assignableVariable.ts, src/ai/tools/jsonSchemaValidator.ts, src/ai/tools/jsonSchemaValidator.test.ts
Defines provider-neutral type contracts for tool definitions, calls, results, messages, and chat requests; public API types for agent config and generate results; assertAssignableVariableName guard; a JSON-Schema subset validator with registration-time and runtime validation; and full unit test coverage.
Per-provider chat body builder, response parser, and chatRequest
src/ai/tools/providerToolMapping.ts, src/ai/tools/providerToolMapping.test.ts, src/ai/OpenAIRequest.ts, src/ai/OpenAIRequest.test.ts
Adds injectStrictObjectSchema, OpenAI/Anthropic/Gemini message formatting, tool declaration wiring, and response parsing dispatchers; extends CommonResponse with tool-calling fields, fixes Anthropic content-block concatenation, adds anthropicMaxTokens, routes by getProviderKind, and exports chatRequest for multi-turn loops.
Bounded agentic tool execution loop
src/ai/tools/runToolLoop.ts, src/ai/tools/runToolLoop.test.ts
Implements the provider-agnostic tool loop with step/transcript byte budgets, validate→confirm→execute→stringify pipeline per tool call, abort/context-overflow classification, forced-final text-only step, and comprehensive test coverage.
Vault path sanitization and write guards
src/ai/tools/sanitizeVaultPath.ts, src/ai/tools/sanitizeVaultPath.test.ts, src/ai/tools/builtins/vaultWriteGuards.ts
Pure path sanitization rejecting traversal, dot-segments, absolute paths, illegal characters, Windows device names, and allowedRoots scoping; runtime symlink/escape guard using Node fs.realpath on desktop, lazy-loaded to avoid mobile impact; and full test suites.
Built-in vault, workspace, and system tool groups
src/ai/tools/builtins/shared.ts, src/ai/tools/builtins/vaultTools.ts, src/ai/tools/builtins/workspaceTools.ts, src/ai/tools/builtins/systemTools.ts, src/ai/tools/builtins/builtins.test.ts
Read-only vault tools (list/search/read/property aggregation) and approval-gated write tools (create/append/insert-under-heading) with path safety; workspace tools (active note, selection); system tools (get_date); shared group filtering/prefixing helpers; and comprehensive test coverage.
Agent orchestrator, confirmation UI, and settings
src/gui/AIToolConfirmModal.ts, src/gui/AIAssistantSettingsModal.ts, src/settings.ts, src/ai/tools/Agent.ts, src/ai/tools/Agent.test.ts
Promise-based per-tool confirmation modal with allow/deny/abort/allow-all, arrow-key navigation; confirmToolCalls setting and UI dropdown; Agent class with constructor validation, single-flight concurrency guard, tool loop orchestration, per-tool and global confirmation logic with read-only exemption, structured output with optional repair re-ask, and variable assignment.
Public API surface, package preview scanning, and documentation
src/quickAddApi.ts, src/services/packagePreview.ts, src/services/packagePreview.test.ts, docs/docs/AIAssistant.md, docs/docs/QuickAddAPI.md, src/ai/tools/...e2e.test.ts
Exposes ai.agent, ai.tool, ai.tools.{vault,workspace,system}, stop-condition helpers, and assignToVariable under quickAddApi; adds ai-tools critical capability flag with static scan heuristic to package preview; updates docs with agent config, tool definition, built-ins, and structured output; adds live e2e tests for all three providers.

Sequence Diagram(s)

sequenceDiagram
  participant Script
  participant Agent
  participant runToolLoop
  participant chatRequest
  participant AIToolConfirmModal

  Script->>Agent: generate({ prompt, schema })
  Agent->>Agent: buildSeedMessages + buildRegistry
  Agent->>runToolLoop: RunToolLoopDeps

  loop each turn
    runToolLoop->>chatRequest: NormalizedChatRequest (tools or schema)
    chatRequest-->>runToolLoop: ParsedChatResult
    opt tool calls present
      runToolLoop->>Agent: confirm(toolName, args)
      Agent->>AIToolConfirmModal: Prompt(app, toolName, args)
      AIToolConfirmModal-->>Agent: allow / deny / abort / allow-all
      Agent-->>runToolLoop: outcome
      runToolLoop->>runToolLoop: execute + stringifyToolResult
    end
    runToolLoop->>runToolLoop: append messages, check budget/stop
  end

  runToolLoop-->>Agent: RunToolLoopResult
  opt schema provided
    Agent->>Agent: parseAndValidate + optional repair re-ask
  end
  Agent-->>Script: GenerateResult
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • chhoumann/quickadd#1245: Touches src/ai/OpenAIRequest.test.ts expectations around Anthropic/OpenAI provider request construction and response mapping, which this PR updates directly.
  • chhoumann/quickadd#1287: Modifies the package-import capability review preview pipeline (src/services/packagePreview.ts), the same file this PR extends with the new ai-tools critical flag and bundled-script scanning.
  • chhoumann/quickadd#1305: Changes buildPackagePreview capability-gating logic and PreviewFlag/FLAG_META in src/services/packagePreview.ts, directly affecting the same review predicate machinery this PR's ai-tools scanning hooks into.

Poem

🐇 Hop hop, the rabbit calls a tool,
With schemas checked and vaults kept cool.
"Confirm this write?" the modal asks,
The agent loops through all its tasks.
From Gemini to Claude to GPT—
Function calling, wild and free! 🌟

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chhoumann/714-llm-function-calling

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 17, 2026

Copy link
Copy Markdown

Deploying quickadd with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2612f25
Status: ✅  Deploy successful!
Preview URL: https://aa226cd9.quickadd.pages.dev
Branch Preview URL: https://chhoumann-714-llm-function-c.quickadd.pages.dev

View logs

Comment thread src/ai/Provider.ts Fixed
Comment thread src/ai/Provider.ts Fixed
CodeQL js/incomplete-url-substring-sanitization: endpoint.includes(host) could
mis-route a URL with the known host in its path/query or as a fake subdomain
prefix. Parse the hostname and match exactly or on a real subdomain instead.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🧹 Nitpick comments (2)
src/ai/tools/providerToolMapping.test.ts (1)

68-90: ⚡ Quick win

Add a regression case for non-object JSON tool arguments.

Current OpenAI parse tests cover valid-object JSON and malformed JSON, but not valid JSON that is the wrong shape (e.g. "[]" or "42"). Adding this guards the normalized-call object-args contract.

Suggested test addition
 it("flags malformed JSON arguments with parseError (no throw)", () => {
 	// ...
 });
+
+it("flags non-object JSON arguments with parseError", () => {
+	const res = parseChatResponse("openai", {
+		choices: [{
+			finish_reason: "tool_calls",
+			message: { tool_calls: [{ id: "c1", function: { name: "create_note", arguments: "[]" } }] },
+		}],
+	});
+	expect(res.toolCalls[0]).toMatchObject({ id: "c1", args: null, parseError: true });
+});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ai/tools/providerToolMapping.test.ts` around lines 68 - 90, Add a new
test case to the providerToolMapping.test.ts file that tests the
parseChatResponse function with valid JSON arguments that are not object-shaped
(such as a string containing "[]" or "42"). The test should verify that when
tool arguments are valid JSON but represent a non-object type, the tool call is
handled appropriately by checking that either args is null or parseError is set
to true, ensuring the normalized call object maintains its contract that args
must be an object or null.
src/ai/tools/runToolLoop.test.ts (1)

227-231: ⚡ Quick win

Assert truncation against bytes, not characters.

This test currently checks character length only, so it won’t catch byte-cap regressions on multibyte output.

Suggested patch
 	it("truncates oversized results with a marker", () => {
 		const r = stringifyToolResult("y".repeat(5000), 100);
 		expect(r.content.endsWith("…[truncated]")).toBe(true);
 		expect(r.content.length).toBeLessThan(5000);
+		expect(new TextEncoder().encode(r.content).length).toBeLessThanOrEqual(100);
 	});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ai/tools/runToolLoop.test.ts` around lines 227 - 231, The test "truncates
oversized results with a marker" currently asserts character length using
r.content.length instead of byte length, which won't catch regressions when
multibyte characters are involved. Replace the character length assertion in the
expect statement with a byte length check using Buffer.byteLength(r.content,
'utf-8') to properly validate that the truncation respects the byte limit and
handles multibyte characters correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/docs/QuickAddAPI.md`:
- Around line 686-690: The blockquote starting with "Confirmation needs an
interactive Obsidian session" contains a blank line that violates the MD028
markdown linting rule. Remove the blank line entirely to keep the blockquote
contiguous, or if intentional spacing is needed, prefix the blank line with `>`
to maintain it as part of the blockquote structure.

In `@src/ai/Provider.ts`:
- Around line 41-47: The provider-kind inference in the condition checks at
lines 41 and 45 uses substring matching with endpoint.includes() which can
incorrectly match domain names that appear in URL paths or query parameters
rather than the actual hostname. To fix this, parse the endpoint URL to extract
the actual host/hostname (e.g., using URL constructor or similar parsing method)
and then perform exact comparisons against the target hostnames like
"api.anthropic.com" and "generativelanguage.googleapis.com". Apply this same
host-based matching approach to all similar provider detection conditions in the
setProviderKind method to ensure consistent and accurate provider
classification.

In `@src/ai/tools/Agent.ts`:
- Around line 222-231: Add a guard condition before calling
resolveStructuredObject() to ensure the loop completed successfully. The issue
is that resolveStructuredObject() is invoked even when the loop ended in failure
states (aborted or context-overflow), allowing the repair logic to make
unintended API calls that bypass safety guards. Before the if statement that
checks options.schema, add a condition to verify the loop did not abort or
encounter context-overflow, ensuring structured object resolution only proceeds
on successful loop completion. Apply this guard to all instances where
resolveStructuredObject() is called as noted in the diff range and the "Also
applies to" reference.

In `@src/ai/tools/builtins/vaultTools.ts`:
- Around line 164-165: The note writer paths are not consistently enforcing
markdown extension because the ensureMd function preserves existing non-.md
extensions instead of strictly enforcing them. Locate the ensureMd function
implementation (around line 252) and modify it to strip any existing file
extension before appending .md, ensuring all paths are strictly forced to
markdown format. Then apply this consistent enforcement across all note writer
path sanitization calls at lines 164, 184, and 211 where sanitizeVaultPath and
ensureMd are used together.

In `@src/ai/tools/jsonSchemaValidator.ts`:
- Around line 105-113: The assertRegisterableSchema function currently accepts
tuple-style items schemas (where schema.items is an array), but the runtime
validation logic only handles the case where items is a single schema, causing
tuple constraints to be registered but then ignored at validation time. Fix this
by either rejecting tuple-style items arrays in the registration check (the else
branch that currently calls assertRegisterableSchema recursively) or by adding
complete validation handling for arrays at the corresponding validation location
around line 188-194. The simplest fix is to throw an error in
assertRegisterableSchema when schema.items is an array, since tuple-style
validation is not supported in the codebase.
- Around line 170-175: The required property validation in the schema validation
loop uses the `in` operator with `key in obj`, which checks the entire prototype
chain including inherited properties. This causes false positives when checking
for required fields that exist on the prototype (like "toString"). Replace the
`key in obj` check with `Object.prototype.hasOwnProperty.call(obj, key)` to only
check for own properties actually present on the object, ensuring accurate
validation of required fields.

In `@src/ai/tools/providerToolMapping.ts`:
- Around line 201-226: The parseOpenAIToolCall function does not validate that
the args field is always an object, violating the normalized call contract.
After JSON.parse() on line 208, check if the parsed result is a plain object; if
not, return with args: null and parseError: true instead. Similarly, in the
final return statement on line 225, validate that the cast value is an actual
object before returning it, otherwise return with args: null and parseError:
true. This ensures args is consistently either a Record/object, null with
parseError flag, or explicitly null.

In `@src/ai/tools/runToolLoop.ts`:
- Around line 151-159: The truncation logic in the if block reduces the string
to fit within maxBytes, but then appends the " …[truncated]" marker which can
cause the final string to exceed the configured limit. Modify the truncation
logic to account for the byte length of the truncation marker: subtract the byte
length of " …[truncated]" from maxBytes before entering the while loop that
calculates the cut position, so that when the marker is appended to the sliced
string, the total result remains within the original maxBytes constraint.

In `@src/gui/AIToolConfirmModal.ts`:
- Around line 23-29: The static method `Prompt` in the AIToolConfirmModal class
violates the repository's camelCase naming convention for functions. Rename the
static method from `Prompt` to `prompt` (lowercase), and then search the entire
codebase for all call sites that invoke AIToolConfirmModal.Prompt and update
them to use AIToolConfirmModal.prompt instead.

In `@src/quickAddApi.ts`:
- Around line 415-417: The validation logic for assignToVariable uses a truthy
check which allows empty strings to bypass validation since empty strings are
falsy, but these invalid names can still be assigned later. Replace the truthy
check with an explicit check for undefined (such as `!== undefined` or checking
property existence) when calling assertAssignableVariableName so that all
provided values including empty strings are validated. Apply this change to all
instances where assignToVariable is validated: the ones near line 415-417, line
517-519, and any other similar patterns in the file where assignToVariable
validation occurs before it is used as outputVariableName.

---

Nitpick comments:
In `@src/ai/tools/providerToolMapping.test.ts`:
- Around line 68-90: Add a new test case to the providerToolMapping.test.ts file
that tests the parseChatResponse function with valid JSON arguments that are not
object-shaped (such as a string containing "[]" or "42"). The test should verify
that when tool arguments are valid JSON but represent a non-object type, the
tool call is handled appropriately by checking that either args is null or
parseError is set to true, ensuring the normalized call object maintains its
contract that args must be an object or null.

In `@src/ai/tools/runToolLoop.test.ts`:
- Around line 227-231: The test "truncates oversized results with a marker"
currently asserts character length using r.content.length instead of byte
length, which won't catch regressions when multibyte characters are involved.
Replace the character length assertion in the expect statement with a byte
length check using Buffer.byteLength(r.content, 'utf-8') to properly validate
that the truncation respects the byte limit and handles multibyte characters
correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e7e91f0c-119d-4cef-96b6-e2e79d53a2e7

📥 Commits

Reviewing files that changed from the base of the PR and between d98904e and 4dd9b8a.

📒 Files selected for processing (36)
  • docs/docs/AIAssistant.md
  • docs/docs/QuickAddAPI.md
  • src/ai/OpenAIRequest.test.ts
  • src/ai/OpenAIRequest.ts
  • src/ai/Provider.test.ts
  • src/ai/Provider.ts
  • src/ai/tools/Agent.test.ts
  • src/ai/tools/Agent.ts
  • src/ai/tools/NormalizedTools.ts
  • src/ai/tools/aiToolTypes.ts
  • src/ai/tools/anthropic.e2e.test.ts
  • src/ai/tools/assignableVariable.ts
  • src/ai/tools/builtins/builtins.test.ts
  • src/ai/tools/builtins/shared.ts
  • src/ai/tools/builtins/systemTools.ts
  • src/ai/tools/builtins/vaultTools.ts
  • src/ai/tools/builtins/vaultWriteGuards.ts
  • src/ai/tools/builtins/workspaceTools.ts
  • src/ai/tools/gemini.e2e.test.ts
  • src/ai/tools/jsonSchemaValidator.test.ts
  • src/ai/tools/jsonSchemaValidator.ts
  • src/ai/tools/openai.e2e.test.ts
  • src/ai/tools/providerToolMapping.test.ts
  • src/ai/tools/providerToolMapping.ts
  • src/ai/tools/runToolLoop.test.ts
  • src/ai/tools/runToolLoop.ts
  • src/ai/tools/sanitizeVaultPath.test.ts
  • src/ai/tools/sanitizeVaultPath.ts
  • src/gui/AIAssistantSettingsModal.ts
  • src/gui/AIToolConfirmModal.ts
  • src/migrations/migrate.ts
  • src/migrations/setProviderKind.ts
  • src/quickAddApi.ts
  • src/services/packagePreview.test.ts
  • src/services/packagePreview.ts
  • src/settings.ts

Comment thread docs/docs/QuickAddAPI.md Outdated
Comment thread src/ai/Provider.ts Outdated
Comment thread src/ai/tools/Agent.ts
Comment thread src/ai/tools/builtins/vaultTools.ts Outdated
Comment thread src/ai/tools/jsonSchemaValidator.ts
Comment thread src/ai/tools/jsonSchemaValidator.ts
Comment thread src/ai/tools/providerToolMapping.ts Outdated
Comment thread src/ai/tools/runToolLoop.ts
Comment thread src/gui/AIToolConfirmModal.ts
Comment thread src/quickAddApi.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4dd9b8a5af

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/migrations/setProviderKind.ts Outdated
Comment thread src/ai/OpenAIRequest.ts
…714)

Address CodeRabbit review on #1372:
- jsonSchemaValidator: reject tuple-style `items` arrays at registration (they
  were accepted but skipped at runtime); use own-property check for `required`
  so an inherited name (e.g. "toString") can't satisfy it.
- providerToolMapping: normalize OpenAI tool args — a parsed array/primitive now
  becomes parseError instead of a non-object `args`.
- runToolLoop: reserve the truncation marker's bytes so a capped tool result
  (content + marker) still fits maxResultBytes.
- vaultTools: note writers enforce .md — refuse a non-markdown extension so a
  model can't create/modify a .js (runnable) or other arbitrary file type.
- Agent: don't run the structured-output network repair when the loop ended
  aborted/context-overflow or online features are disabled (keep the free first
  parse); the first parse never makes a call.
- quickAddApi: an empty assignToVariable ("") now falls back instead of leaking
  as the output variable name, matching ai.agent's length>0 behavior.
- docs: convert the confirmation note to a :::note admonition (markdownlint MD028).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/ai/tools/builtins/builtins.test.ts (1)

78-88: ⚡ Quick win

Strengthen the append writer safety assertion to match the test intent.

Line 87 only verifies vault.create wasn’t called, which doesn’t cover append_to_note. Add explicit assertions that append-path write/read APIs were not touched.

Suggested test hardening
 	it("note writers refuse a non-markdown extension (no .js/.css etc.)", async () => {
 		const app = makeApp();
 		const tools = createVaultTools(app);
 		await expect(
 			tools.create_note.execute({ path: "Notes/evil.js" }, { toolCallId: "c", toolName: "create_note" }),
 		).rejects.toThrow(/markdown/i);
 		await expect(
 			tools.append_to_note.execute({ path: "Notes/style.css", content: "x" }, { toolCallId: "c", toolName: "append_to_note" }),
 		).rejects.toThrow(/markdown/i);
 		expect((app.vault.create as ReturnType<typeof vi.fn>)).not.toHaveBeenCalled();
+		expect((app.vault.modify as ReturnType<typeof vi.fn>)).not.toHaveBeenCalled();
+		expect((app.vault.read as ReturnType<typeof vi.fn>)).not.toHaveBeenCalled();
 	});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ai/tools/builtins/builtins.test.ts` around lines 78 - 88, The test is
only asserting that app.vault.create was not called, but it does not verify that
the vault append/write methods were not called when tools.append_to_note.execute
is invoked with an invalid CSS extension. Add explicit assertions after the
expect block for append_to_note to verify that the vault append and read methods
(such as app.vault.append or app.vault.modify) were not called, ensuring the
test properly validates that append_to_note refused to write non-markdown files.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/ai/tools/builtins/builtins.test.ts`:
- Around line 78-88: The test is only asserting that app.vault.create was not
called, but it does not verify that the vault append/write methods were not
called when tools.append_to_note.execute is invoked with an invalid CSS
extension. Add explicit assertions after the expect block for append_to_note to
verify that the vault append and read methods (such as app.vault.append or
app.vault.modify) were not called, ensuring the test properly validates that
append_to_note refused to write non-markdown files.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3ed8ed5b-4aef-4a66-b476-025abb14c114

📥 Commits

Reviewing files that changed from the base of the PR and between 461874e and 236528a.

📒 Files selected for processing (10)
  • docs/docs/QuickAddAPI.md
  • src/ai/tools/Agent.ts
  • src/ai/tools/builtins/builtins.test.ts
  • src/ai/tools/builtins/vaultTools.ts
  • src/ai/tools/jsonSchemaValidator.test.ts
  • src/ai/tools/jsonSchemaValidator.ts
  • src/ai/tools/providerToolMapping.test.ts
  • src/ai/tools/providerToolMapping.ts
  • src/ai/tools/runToolLoop.ts
  • src/quickAddApi.ts
🚧 Files skipped from review as they are similar to previous changes (9)
  • src/ai/tools/providerToolMapping.ts
  • src/ai/tools/jsonSchemaValidator.test.ts
  • src/ai/tools/builtins/vaultTools.ts
  • src/ai/tools/jsonSchemaValidator.ts
  • src/ai/tools/providerToolMapping.test.ts
  • src/quickAddApi.ts
  • docs/docs/QuickAddAPI.md
  • src/ai/tools/Agent.ts
  • src/ai/tools/runToolLoop.ts

chhoumann added a commit that referenced this pull request Jun 17, 2026
…tion (#714)

Address Codex review on #1372:
- anthropicMaxTokens: default back to 4096 (the long-standing pre-#714 value).
  Anthropic 400s when max_tokens exceeds a model's OUTPUT cap (verified live),
  and 8192 exceeds Claude 3 Haiku's 4096 — so the bump could reject small-input
  requests. Explicit maxOutputTokens is still honored uncapped for 4.x models.
- Remove the setProviderKind migration: it persisted an inferred wire kind that
  the provider editor can't change, so repointing a provider's endpoint left a
  stale kind and the wrong wire shape. getProviderKind already infers at request
  time when kind is absent; the explicit field stays for known/preset providers
  (e.g. a custom-named Anthropic proxy).
…tion (#714)

Address Codex review on #1372:
- anthropicMaxTokens: default back to 4096 (the long-standing pre-#714 value).
  Anthropic 400s when max_tokens exceeds a model's OUTPUT cap (verified live),
  and 8192 exceeds Claude 3 Haiku's 4096 — so the bump could reject small-input
  requests. Explicit maxOutputTokens is still honored uncapped for 4.x models.
- Remove the setProviderKind migration: it persisted an inferred wire kind that
  the provider editor can't change, so repointing a provider's endpoint left a
  stale kind and the wrong wire shape. getProviderKind already infers at request
  time when kind is absent; the explicit field stays for known/preset providers
  (e.g. a custom-named Anthropic proxy).
@chhoumann chhoumann force-pushed the chhoumann/714-llm-function-calling branch from 47a4afb to 2612f25 Compare June 17, 2026 07:09
@chhoumann chhoumann merged commit 3d97a16 into master Jun 17, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE REQUEST] llm function calling for macros or scripts

2 participants