Skip to content

feat(platform): add request user location agent tool#841

Merged
larryro merged 3 commits into
mainfrom
feat/request-user-location-tool
Mar 24, 2026
Merged

feat(platform): add request user location agent tool#841
larryro merged 3 commits into
mainfrom
feat/request-user-location-tool

Conversation

@larryro
Copy link
Copy Markdown
Collaborator

@larryro larryro commented Mar 24, 2026

Summary

  • Add a new agent tool (request_user_location) that allows agents to request the user's geolocation during chat conversations
  • Implement location request card UI with approval flow integration (approve/deny with timeout)
  • Update template variable resolution to remove static location injection in favor of the new on-demand tool
  • Remove unused product recommendation workflow examples and update OpenAPI spec

Test plan

  • Verify agent can invoke the location request tool during chat
  • Confirm location request card renders correctly and handles approve/deny/timeout states
  • Test that existing approval flows still work with the updated schema
  • Verify template variable resolution works without the removed location fields

Summary by CodeRabbit

  • New Features

    • Location request functionality enabling agents to request user location with an intuitive share/deny interface
    • Location reverse geocoding showing detailed address information based on user coordinates
    • Location response status tracking with shared/denied indicators and metadata display
  • Removed

    • Removed product recommendation workflow examples

Add a new agent tool that allows agents to request the user's location
during chat. Includes the tool definition, approval flow integration,
location request card UI component, and template variable resolution
updates. Also removes unused product recommendation workflow examples
and updates the OpenAPI spec.
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Reformat openapi.json and simplify isThreadGenerating tests to match
the streamlined implementation that only checks generationStatus.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 24, 2026

📝 Walkthrough

Walkthrough

This PR introduces a new location-request approval workflow feature that allows agents to request user geolocation through a browser-based UI component, while concurrently removing product-recommendation workflow examples and refactoring how user geolocation is handled. Key additions include a new LocationRequestCard React component, a request_user_location tool for triggering location approval flows, backend mutations and queries for managing location requests, and schema updates to support location_request as an approval resource type. Simultaneously, the PR removes geolocation acquisition and caching logic from useUserContext, eliminates coordinates/location from user context passed to agents, simplifies thread generation checking, and updates example workflows and tests to use integration_operation instead of product_recommendation.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(platform): add request user location agent tool' accurately and concisely summarizes the main change: adding a new agent tool for requesting user location.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 feat/request-user-location-tool

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
services/platform/convex/workflows/processing_records/integration.test.ts (1)

197-199: ⚠️ Potential issue | 🟡 Minor

Avoid hard-coding status as the only valid selected index key in this test.

With both status and category indexed, this assertion can fail after benign index-priority/tie-break changes even when selection is still correct. Assert stable invariants instead (e.g., selected.indexableConditions includes both parsed equalities, and selected index key is one of status/category).

Proposed test hardening
-      // Products table has both status and category indexes
-      expect(selected.indexValues.status).toBe('active');
+      // Products table has both status and category indexes;
+      // selector may choose either depending on scoring/tie-breaks.
+      expect(selected.indexableConditions).toHaveLength(2);
+      expect(
+        selected.indexValues.status === 'active' ||
+          selected.indexValues.category === 'electronics',
+      ).toBe(true);

Based on learnings: in Convex index selection, values and indexableConditions serve different purposes and should be validated accordingly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/convex/workflows/processing_records/integration.test.ts`
around lines 197 - 199, The test currently hard-codes that
selected.indexValues.status === 'active', which can break if index tie-breaks
choose 'category' instead; update the assertion to validate invariants: assert
that selected.indexableConditions includes both parsed equality conditions for
status and category (e.g., both equality entries are present) and assert that
selected.indexKey (or whatever field indicates which index was chosen) is one of
'status' or 'category'; optionally assert selected.indexValues contains the
expected value(s) for whichever index was chosen rather than assuming status
specifically.
services/platform/convex/threads/queries.ts (1)

37-42: ⚠️ Potential issue | 🟠 Major

Restore stale-generation recovery before using metadata as the only signal.

Lines 37-42 now return true solely from threadMetadata.generationStatus. startAgentChat still sets that flag before scheduling the action and explicitly notes that crashes can leave it stuck, so removing the previous fallback means one failed generation can disable the composer indefinitely for that thread.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/convex/threads/queries.ts` around lines 37 - 42, The
current check in the query that returns metadata?.generationStatus ===
'generating' is unsafe because startAgentChat sets generationStatus before
scheduling and crashes can leave it stuck; restore the previous stale-generation
recovery fallback by also checking the thread's last generation timestamp or
related fields (as used before) when querying threadMetadata via
ctx.db.query('threadMetadata').withIndex('by_threadId', (q) => q.eq('threadId',
args.threadId')).first(), and only return true if generationStatus ===
'generating' AND the timestamp indicates the generation is recent (or,
conversely, treat stale generationStatus as false), mirroring the earlier logic
used alongside startAgentChat to prevent a single failed generation from
permanently disabling the composer for that thread.
services/platform/app/features/chat/hooks/__tests__/use-merged-chat-items.test.ts (1)

49-58: 🧹 Nitpick | 🔵 Trivial

Add a positive merge test for locationRequests.

This fixture update keeps the suite compiling, but nothing in the file asserts that a pending or executing location request is surfaced as activeApproval. Since the new card path depends on that merge behavior, please cover at least one happy path here.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@services/platform/app/features/chat/hooks/__tests__/use-merged-chat-items.test.ts`
around lines 49 - 58, Add a positive test case in use-merged-chat-items.test.ts
that supplies a non-empty locationRequests entry (with status 'PENDING' or
'EXECUTING') into the emptyParams fixture and calls the hook/utility under test
(useMergedChatItems or the exported merge function used in the file), then
assert that the returned merged items include an activeApproval for that
location request (e.g., check mergedItems.some(item => item.activeApproval?.type
=== 'locationRequest' && item.activeApproval?.id === <fixture id>) and/or that
the approval status is present). Use the existing test harness in the file (same
pattern as other approval tests) and create a minimal locationRequest object
with id, status, and necessary fields to mimic a happy-path pending/executing
approval.
services/platform/app/features/chat/components/approval-card-renderer.tsx (1)

16-17: 🧹 Nitpick | 🔵 Trivial

Consider renaming the callback prop to be approval-generic.

onHumanInputResponseSubmitted now also handles location responses; a neutral name (e.g., onApprovalResponseSubmitted) would reduce semantic drift.

Also applies to: 84-85

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/app/features/chat/components/approval-card-renderer.tsx`
around lines 16 - 17, Rename the prop onHumanInputResponseSubmitted to a neutral
name like onApprovalResponseSubmitted across the ApprovalCardRenderer component:
update the prop definition in the interface and the prop destructuring inside
the component (references to onHumanInputResponseSubmitted), update any internal
calls that invoke it (e.g., where onHumanInputResponseSubmitted() is called),
and update all callers of ApprovalCardRenderer to pass the new prop name
(including the usages noted around the earlier referenced block). Keep the
function signature the same and run a quick search/replace for
onHumanInputResponseSubmitted to ensure all references (props, handlers, tests)
are updated to onApprovalResponseSubmitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@services/platform/app/features/chat/components/location-request-card.tsx`:
- Around line 19-26: The current LOCATION_CACHE_KEY/CachedLocation stores exact
lat/lng in localStorage indefinitely; replace this with a non-origin-wide,
short-lived approach: either keep the cache in-memory (module-level variable) or
use sessionStorage (scoped to the browser session) and do not persist exact
coordinates — store a coarse/rounded location (e.g., reduced precision) plus a
timestamp/TTL and check COORD_THRESHOLD as before; update any code that
reads/writes LOCATION_CACHE_KEY (and uses CachedLocation) to respect the new
storage strategy and TTL so location is not reused across users or long after
logout.
- Around line 147-159: The code currently falls back to raw coordinates when
reverseGeocode(lat, lng) returns no address; change this so that if
reverseGeocode returns null/undefined/empty you treat it as a failure (fail
closed) instead of using `${lat.toFixed(2)}, ${lng.toFixed(2)}`: after calling
reverseGeocode in the LocationRequestCard logic, check the address result and if
falsy abort the approval flow (return/throw or mark as denied) and do NOT write
a cache entry with raw coords to LOCATION_CACHE_KEY; ensure any downstream code
that reads the local variable location (the approval response path) never
receives raw coordinates and handles the failure case appropriately.

In `@services/platform/app/features/chat/hooks/use-merged-chat-items.ts`:
- Around line 129-137: The loop in useMergedChatItems only promotes
locationRequests when isActiveStatus(status) is true, which prevents
completed/denied states from reaching the chat surface; update the logic in
useMergedChatItems so that any location request with a messageId that exists in
loadedMessageIds is pushed into activeApprovals (or a new approvals array used
by useLocationRequests) regardless of isActiveStatus, so LocationRequestCard can
render non-pending states (completed/denied); ensure references to
locationRequests, activeApprovals, isActiveStatus, and
useLocationRequests/useActiveApprovals are updated consistently so
completed/rejected requests remain available to the UI.

In `@services/platform/convex/agent_tools/database/helpers/schema_definitions.ts`:
- Around line 137-140: Update the schema documentation for the approval
resourceType example to include the new flow: modify the note string on the
field named 'resourceType' in schema_definitions.ts to add "location_request"
alongside the existing examples (e.g., "integration_operation", "conversations",
"workflow_creation") so the example values reflect the introduced approval flow.

In `@services/platform/convex/agent_tools/location/mutations.ts`:
- Around line 89-92: The code is inserting client-controlled args.location
directly into privileged prompt text (see responseMessage, the saved role:
'system' message, and promptMessage in this file), which allows prompt
injection; instead, keep any system messages static and do not interpolate
args.location into them—store the location as structured data (e.g., a separate
"location" field or metadata in the message object) and pass that structured
field to downstream logic that consumes location, and update the places
referenced (responseMessage generation, the system message creation around the
prompt, and the promptMessage usage at the other locations noted) to read the
location from the structured payload rather than embedding it in the system
prompt string.

In `@services/platform/convex/agent_tools/location/request_user_location_tool.ts`:
- Around line 43-48: The tool's prompt is waiting for the marker
"<location_response>" but the mutation function submitLocationResponse emits a
system message starting with "[LOCATION_RESPONSE]"; update the
request_user_location_tool text to use the exact emitted marker
"[LOCATION_RESPONSE]" (and similarly update the duplicated block at lines
105-117) so the model waits for the same token that submitLocationResponse
actually writes; reference the request_user_location_tool messaging text and
submitLocationResponse to ensure both sides match exactly.
- Around line 87-97: When calling
internal.agent_tools.location.internal_mutations.createLocationRequest in
request_user_location_tool.ts, include the originating message id so the
approval is visible to useMergedChatItems; add the message id field (e.g.,
messageId: args.messageId or the correct prop name from the tool args) to the
mutation input alongside organizationId, threadId, reason, wfExecutionId, and
stepSlug so the created request is attached to the original message.

---

Outside diff comments:
In `@services/platform/app/features/chat/components/approval-card-renderer.tsx`:
- Around line 16-17: Rename the prop onHumanInputResponseSubmitted to a neutral
name like onApprovalResponseSubmitted across the ApprovalCardRenderer component:
update the prop definition in the interface and the prop destructuring inside
the component (references to onHumanInputResponseSubmitted), update any internal
calls that invoke it (e.g., where onHumanInputResponseSubmitted() is called),
and update all callers of ApprovalCardRenderer to pass the new prop name
(including the usages noted around the earlier referenced block). Keep the
function signature the same and run a quick search/replace for
onHumanInputResponseSubmitted to ensure all references (props, handlers, tests)
are updated to onApprovalResponseSubmitted.

In
`@services/platform/app/features/chat/hooks/__tests__/use-merged-chat-items.test.ts`:
- Around line 49-58: Add a positive test case in use-merged-chat-items.test.ts
that supplies a non-empty locationRequests entry (with status 'PENDING' or
'EXECUTING') into the emptyParams fixture and calls the hook/utility under test
(useMergedChatItems or the exported merge function used in the file), then
assert that the returned merged items include an activeApproval for that
location request (e.g., check mergedItems.some(item => item.activeApproval?.type
=== 'locationRequest' && item.activeApproval?.id === <fixture id>) and/or that
the approval status is present). Use the existing test harness in the file (same
pattern as other approval tests) and create a minimal locationRequest object
with id, status, and necessary fields to mimic a happy-path pending/executing
approval.

In `@services/platform/convex/threads/queries.ts`:
- Around line 37-42: The current check in the query that returns
metadata?.generationStatus === 'generating' is unsafe because startAgentChat
sets generationStatus before scheduling and crashes can leave it stuck; restore
the previous stale-generation recovery fallback by also checking the thread's
last generation timestamp or related fields (as used before) when querying
threadMetadata via ctx.db.query('threadMetadata').withIndex('by_threadId', (q)
=> q.eq('threadId', args.threadId')).first(), and only return true if
generationStatus === 'generating' AND the timestamp indicates the generation is
recent (or, conversely, treat stale generationStatus as false), mirroring the
earlier logic used alongside startAgentChat to prevent a single failed
generation from permanently disabling the composer for that thread.

In `@services/platform/convex/workflows/processing_records/integration.test.ts`:
- Around line 197-199: The test currently hard-codes that
selected.indexValues.status === 'active', which can break if index tie-breaks
choose 'category' instead; update the assertion to validate invariants: assert
that selected.indexableConditions includes both parsed equality conditions for
status and category (e.g., both equality entries are present) and assert that
selected.indexKey (or whatever field indicates which index was chosen) is one of
'status' or 'category'; optionally assert selected.indexValues contains the
expected value(s) for whichever index was chosen rather than assuming status
specifically.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: b08409c3-6fd6-4f78-8c13-2e6b6bb3f2fb

📥 Commits

Reviewing files that changed from the base of the PR and between f82a46a and f5dec55.

⛔ Files ignored due to path filters (1)
  • services/platform/convex/_generated/api.d.ts is excluded by !**/_generated/**
📒 Files selected for processing (39)
  • examples/workflows/general/product-recommendation-email.json
  • examples/workflows/general/product-recommendation.json
  • examples/workflows/loopi/product-recommendation.json
  • services/platform/app/features/chat/components/approval-card-renderer.tsx
  • services/platform/app/features/chat/components/chat-interface.tsx
  • services/platform/app/features/chat/components/location-request-card.tsx
  • services/platform/app/features/chat/hooks/__tests__/use-merged-chat-items.test.ts
  • services/platform/app/features/chat/hooks/mutations.ts
  • services/platform/app/features/chat/hooks/queries.ts
  • services/platform/app/features/chat/hooks/use-merged-chat-items.ts
  • services/platform/app/features/chat/hooks/use-send-message.ts
  • services/platform/app/features/chat/hooks/use-user-context.ts
  • services/platform/app/features/custom-agents/hooks/use-test-chat.ts
  • services/platform/convex/agent_tools/database/helpers/schema_definitions.ts
  • services/platform/convex/agent_tools/location/internal_mutations.ts
  • services/platform/convex/agent_tools/location/mutations.ts
  • services/platform/convex/agent_tools/location/request_user_location_tool.ts
  • services/platform/convex/agent_tools/tool_names.ts
  • services/platform/convex/agent_tools/tool_registry.ts
  • services/platform/convex/approvals/helpers.ts
  • services/platform/convex/approvals/list_approvals_paginated.test.ts
  • services/platform/convex/approvals/schema.ts
  • services/platform/convex/approvals/validators.ts
  • services/platform/convex/custom_agents/unified_chat.ts
  • services/platform/convex/lib/agent_chat/internal_actions.ts
  • services/platform/convex/lib/agent_chat/start_agent_chat.ts
  • services/platform/convex/lib/agent_response/__tests__/resolve_template_variables_no_location.test.ts
  • services/platform/convex/lib/agent_response/generate_response.ts
  • services/platform/convex/lib/agent_response/resolve_template_variables.ts
  • services/platform/convex/threads/queries.ts
  • services/platform/convex/workflow_engine/action_defs/conversation/conversation_action.ts
  • services/platform/convex/workflow_engine/action_defs/workflow_processing_records/workflow_processing_records_action.ts
  • services/platform/convex/workflow_engine/helpers/engine/shard.test.ts
  • services/platform/convex/workflows/processing_records/integration.test.ts
  • services/platform/lib/shared/constants/system-message-tags.ts
  • services/platform/lib/shared/schemas/approvals.ts
  • services/platform/messages/en.json
  • services/platform/public/openapi.json
  • services/platform/stress-tests/scenarios/scheduler-overlap.ts
💤 Files with no reviewable changes (8)
  • services/platform/app/features/chat/hooks/use-send-message.ts
  • services/platform/convex/custom_agents/unified_chat.ts
  • services/platform/convex/lib/agent_response/generate_response.ts
  • services/platform/convex/lib/agent_chat/internal_actions.ts
  • services/platform/convex/lib/agent_response/resolve_template_variables.ts
  • examples/workflows/general/product-recommendation-email.json
  • examples/workflows/loopi/product-recommendation.json
  • examples/workflows/general/product-recommendation.json

Comment on lines +19 to +26
const LOCATION_CACHE_KEY = 'tale:user-location';
const COORD_THRESHOLD = 0.01;

interface CachedLocation {
lat: number;
lng: number;
address?: string;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Do not persist exact location in origin-wide localStorage.

This cache stores precise lat/lng indefinitely under a single key, so it survives logout/account switching and can be reused across users on the same browser. An in-memory cache or a short-lived coarse cache is safer.

Also applies to: 38-47, 149-153

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/app/features/chat/components/location-request-card.tsx`
around lines 19 - 26, The current LOCATION_CACHE_KEY/CachedLocation stores exact
lat/lng in localStorage indefinitely; replace this with a non-origin-wide,
short-lived approach: either keep the cache in-memory (module-level variable) or
use sessionStorage (scoped to the browser session) and do not persist exact
coordinates — store a coarse/rounded location (e.g., reduced precision) plus a
timestamp/TTL and check COORD_THRESHOLD as before; update any code that
reads/writes LOCATION_CACHE_KEY (and uses CachedLocation) to respect the new
storage strategy and TTL so location is not reused across users or long after
logout.

Comment on lines +147 to +159
address = await reverseGeocode(lat, lng);
// Update cache
try {
localStorage.setItem(
LOCATION_CACHE_KEY,
JSON.stringify({ lat, lng, address }),
);
} catch {
// localStorage full — not critical
}
}

const location = address ?? `${lat.toFixed(2)}, ${lng.toFixed(2)}`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fail closed if reverse geocoding cannot produce an address.

Falling back to ${lat.toFixed(2)}, ${lng.toFixed(2)} sends raw coordinates through the approval response path. That breaks the tool’s “city-level only / never raw coordinates” contract and still reveals roughly neighborhood-level location.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/app/features/chat/components/location-request-card.tsx`
around lines 147 - 159, The code currently falls back to raw coordinates when
reverseGeocode(lat, lng) returns no address; change this so that if
reverseGeocode returns null/undefined/empty you treat it as a failure (fail
closed) instead of using `${lat.toFixed(2)}, ${lng.toFixed(2)}`: after calling
reverseGeocode in the LocationRequestCard logic, check the address result and if
falsy abort the approval flow (return/throw or mark as denied) and do NOT write
a cache entry with raw coords to LOCATION_CACHE_KEY; ensure any downstream code
that reads the local variable location (the approval response path) never
receives raw coordinates and handles the failure case appropriately.

Comment on lines +129 to +137
for (const a of locationRequests ?? []) {
if (
a.messageId &&
loadedMessageIds.has(a.messageId) &&
isActiveStatus(a.status)
) {
activeApprovals.push({ type: 'location_request', data: a });
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

The completed/rejected location card states are unreachable.

LocationRequestCard implements non-pending UI, but this hook only ever promotes pending/executing location requests into the chat surface. After approve/deny, the card drops out instead of showing the shared/denied result. useLocationRequests currently comes from useActiveApprovals too, so both layers are treating location requests as active-only.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/app/features/chat/hooks/use-merged-chat-items.ts` around
lines 129 - 137, The loop in useMergedChatItems only promotes locationRequests
when isActiveStatus(status) is true, which prevents completed/denied states from
reaching the chat surface; update the logic in useMergedChatItems so that any
location request with a messageId that exists in loadedMessageIds is pushed into
activeApprovals (or a new approvals array used by useLocationRequests)
regardless of isActiveStatus, so LocationRequestCard can render non-pending
states (completed/denied); ensure references to locationRequests,
activeApprovals, isActiveStatus, and useLocationRequests/useActiveApprovals are
updated consistently so completed/rejected requests remain available to the UI.

Comment on lines 137 to 140
field: 'resourceType',
type: 'string',
note: 'Type of resource being approved (e.g., "product_recommendation", "conversations", "email")',
note: 'Type of resource being approved (e.g., "integration_operation", "conversations", "workflow_creation")',
},
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 | 🔵 Trivial

Consider adding location_request to the resourceType examples.

Since this PR introduces the location_request approval flow, consider including it in the example values for documentation completeness. This helps the AI understand all available resource types when constructing filter expressions.

📝 Suggested documentation update
       {
         field: 'resourceType',
         type: 'string',
-        note: 'Type of resource being approved (e.g., "integration_operation", "conversations", "workflow_creation")',
+        note: 'Type of resource being approved (e.g., "integration_operation", "location_request", "conversations", "workflow_creation")',
       },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
field: 'resourceType',
type: 'string',
note: 'Type of resource being approved (e.g., "product_recommendation", "conversations", "email")',
note: 'Type of resource being approved (e.g., "integration_operation", "conversations", "workflow_creation")',
},
field: 'resourceType',
type: 'string',
note: 'Type of resource being approved (e.g., "integration_operation", "location_request", "conversations", "workflow_creation")',
},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/convex/agent_tools/database/helpers/schema_definitions.ts`
around lines 137 - 140, Update the schema documentation for the approval
resourceType example to include the new flow: modify the note string on the
field named 'resourceType' in schema_definitions.ts to add "location_request"
alongside the existing examples (e.g., "integration_operation", "conversations",
"workflow_creation") so the example values reflect the introduced approval flow.

Comment on lines +89 to +92
const responseMessage = isDenied
? '[LOCATION_RESPONSE] User denied location access. Proceed without location data.'
: `[LOCATION_RESPONSE] User location: ${args.location}`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Do not splice client-controlled location text into system prompts.

args.location comes from a public mutation, but it is interpolated into both a saved role: 'system' message and the next promptMessage. A crafted payload can become privileged prompt content. Keep the system text static and pass the location as structured data instead.

Also applies to: 118-133, 189-190

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/convex/agent_tools/location/mutations.ts` around lines 89 -
92, The code is inserting client-controlled args.location directly into
privileged prompt text (see responseMessage, the saved role: 'system' message,
and promptMessage in this file), which allows prompt injection; instead, keep
any system messages static and do not interpolate args.location into them—store
the location as structured data (e.g., a separate "location" field or metadata
in the message object) and pass that structured field to downstream logic that
consumes location, and update the places referenced (responseMessage generation,
the system message creation around the prompt, and the promptMessage usage at
the other locations noted) to read the location from the structured payload
rather than embedding it in the system prompt string.

Comment on lines +43 to +48
**AFTER CALLING - CRITICAL:**
• You MUST STOP and produce your final response immediately
• Do NOT call any more tools or continue with any operation
• Do NOT assume or guess the user's location
• The user's response will appear in a FUTURE turn as <location_response>
• Simply acknowledge you're waiting for their location`,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use the same location-response marker that the mutation emits.

This tool tells the model to wait for <location_response>, but submitLocationResponse writes [LOCATION_RESPONSE] ... system messages. The follow-up turn is brittle because the model is watching for a marker that never arrives.

Also applies to: 105-117

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/convex/agent_tools/location/request_user_location_tool.ts`
around lines 43 - 48, The tool's prompt is waiting for the marker
"<location_response>" but the mutation function submitLocationResponse emits a
system message starting with "[LOCATION_RESPONSE]"; update the
request_user_location_tool text to use the exact emitted marker
"[LOCATION_RESPONSE]" (and similarly update the duplicated block at lines
105-117) so the model waits for the same token that submitLocationResponse
actually writes; reference the request_user_location_tool messaging text and
submitLocationResponse to ensure both sides match exactly.

Comment on lines +87 to +97
try {
const requestId = await ctx.runMutation(
internal.agent_tools.location.internal_mutations
.createLocationRequest,
{
organizationId,
threadId,
reason: args.reason,
wfExecutionId,
stepSlug,
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Attach the originating message id when creating the request.

useMergedChatItems only renders approvals whose messageId is attached to a loaded message, and createLocationRequest already accepts that field. This call omits it, so the new location approval never becomes visible in chat.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/convex/agent_tools/location/request_user_location_tool.ts`
around lines 87 - 97, When calling
internal.agent_tools.location.internal_mutations.createLocationRequest in
request_user_location_tool.ts, include the originating message id so the
approval is visible to useMergedChatItems; add the message id field (e.g.,
messageId: args.messageId or the correct prop name from the tool args) to the
mutation input alongside organizationId, threadId, reason, wfExecutionId, and
stepSlug so the created request is attached to the original message.

…y retry text

Short (≤2 line) warning/error system messages now render inline with an
icon instead of inside a collapsible, and the response-interrupted
message is shortened to "Retrying…".
@larryro larryro merged commit f0e034e into main Mar 24, 2026
17 checks passed
@larryro larryro deleted the feat/request-user-location-tool branch March 24, 2026 00:29
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.

1 participant