improvement(tools): state every pinned param, not just the credential - #7170
Open
waleedlatif1 wants to merge 5 commits into
Open
improvement(tools): state every pinned param, not just the credential#7170waleedlatif1 wants to merge 5 commits into
waleedlatif1 wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
Greptile SummaryThe PR expands model-visible tool annotations from duplicate resource bindings to all safely stateable pinned parameters.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/providers/tool-binding.ts | Introduces pinned-field collection, resource metadata, scalar sanitization, and guards for sensitive or inapplicable parameters. |
| apps/sim/executor/utils/tool-pinned-params.ts | Resolves resource names and appends bounded pinned-parameter statements and duplicate-selection guidance to tool descriptions. |
| apps/sim/executor/handlers/agent/agent-handler.ts | Registers MCP pinned fields and applies secret-aware annotations to provider tools. |
| apps/sim/executor/handlers/pi/local/sim-tools.ts | Adds per-tool secret-provenance checks before annotating Pi tool descriptions. |
| apps/sim/providers/utils.ts | Registers collected pinned fields during standard block-tool transformation. |
| apps/sim/executor/utils/tool-pinned-params.test.ts | Covers resource resolution, duplicate discrimination, annotation limits, caching, and secret withholding. |
| apps/sim/providers/tool-binding.test.ts | Extensively tests pinned-field selection, canonical parameters, sanitization, and sensitive-field exclusions. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Workflow tool parameters] --> B[Collect pinned fields]
B --> C{Sensitive or unsupported?}
C -->|Yes| D[Exclude literal]
C -->|No| E[Sanitize scalar value]
B --> F[Resolve resource IDs to names]
D --> G[Build provider tool]
E --> G
F --> G
G --> H[Annotate model-visible description]
H --> I{Duplicate values differ?}
I -->|Yes| J[Add copy-selection guidance]
I -->|No| K[Keep pinned-value statement only]
Reviews (6): Last reviewed commit: "improvement(tools): decide pinned fields..." | Re-trigger Greptile
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 9 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Collaborator
Author
1 similar comment
Collaborator
Author
Collaborator
Author
Two agent tools bound to the same Gmail account but different labels reached the model byte-identical: every param a user fills is stripped from the schema, and only the credential was ever named in the description. The model could not tell Inbox from Sent, and with a single tool it could promise a caller it would search a folder that tool can never reach. Each tool now states the values the workflow pinned on it, whether or not it has a duplicate sibling. Opaque resource ids still resolve to a display name; plain values like a label, a row limit or a toggle need no lookup and are stated directly. Secrets never are: a field marked `password`, a hidden field, a secret-named param, and every literal on a tool whose params resolved an environment variable are all withheld.
… the prose Follow-ups from review of the pinned-param descriptions. A block's subblocks span every operation it supports, so a Gmail block switched from Send to Read still holds `to`, `subject` and `body` — and the tool stated them as constraints on a read tool, leaking stale draft content into the prompt. Literals are now filtered to the selected tool's declared params. Resources are exempt: an OAuth credential is a block input that never appears in a tool's param map. MCP tools carry configured params but have no subblocks, so they registered nothing and their pinned values went unstated. They now collect from their configured params directly. The duplicate hint claimed the copies differ whenever a tool had a sibling, even when both rendered identical text. It now compares the rendered statements, so the model is never told to pick between indistinguishable copies. Also: reuse `isPasswordParameter` instead of a second secret regex, applied only to literals since it matches `oauthCredential`; make the field type a real union so a field cannot be both a literal and a resource; and cut the stated-field cap from six to three, since every field costs tokens on every request in the loop.
…tween copies Two copies pinned to identical values, where only one of them resolved an environment variable, rendered different text — one withheld its literal — and both were then told "other copies are pinned to different values". That is the exact assertion the comparison exists to prevent. The duplicate check now compares the un-withheld render, so disclosure differences no longer read as configuration differences. Also drops a redundant copy of the resolved-name cache, returns undefined rather than an empty-string sentinel for an unresolved resource, unexports two internal-only interfaces, and corrects six comments: five overstated or referenced the module this branch renamed, and one described the wrong failure mode for a credential entered in advanced mode. Adds the uncovered branches the review named: canonical-id grouping (the shape production actually sees once wire ids are aliased), a sibling that states nothing, negative-cache reuse, both resource kinds in one pass, an omitted tool param map, empty and oversized titles, and a non-finite number.
The MCP discovery path stripped only `toolName` from a tool entry's stored params, so `serverId` and `serverName` stayed in the values treated as pinned and were stated to the model — an internal server id reaching a provider, and two of the three field slots consumed before any real param. The cached path already stripped all three; the split now lives in one helper so the two cannot drift again. Pi withheld literal values for every tool whenever ANY input in the whole run resolved a secret, which is true of almost any real workflow — the feature was effectively off there. It now asks the registry the same per-input-path question the Agent block asks, so only the tool that actually carries a secret is withheld. A canonical group blocked by one half no longer leaks through the other: a `file-upload` basic half skipped without claiming its param, letting a `short-input` twin state a raw file reference. 78 groups have that shape. Widens the secret-name backstop for params named by a remote MCP schema rather than by Sim — `authorization`, `cookie`, `signature`, `connectionString`, `otp` and friends are not in the Sim-tuned `isPasswordParameter` list.
waleedlatif1
force-pushed
the
feat/discriminate-prefilled-params
branch
from
August 27, 2026 23:37
ca5e742 to
31a8f8e
Compare
Collaborator
Author
…aram name The membership check asked a proxy question — "does this subblock's id match a declared tool param?" — when the real one is "is this field part of the operation the tool was selected for". 41 blocks rename a field on its way to the tool inside `tools.config.params`, so their pinned values failed the name match and were never stated. Datadog's `listMonitorName` feeds the tool param `name`; it is now stated as Filter by Name "CPU" instead of dropped. `evaluateSubBlockCondition` answers the real question directly and survives a rename, because it never looks at tool param names. It also still excludes the stale-field case the name match was introduced for: Gmail's to/subject/body are gated to the send operations, so a block switched to Read drops them. The operation selector and trigger-mode subblocks are excluded explicitly — they carry values but do not constrain the call. Trigger mode is skipped per subblock rather than blocking its canonical group. Gmail puts `triggerCredentials` in the same group as `credential`, and blocking the group dropped the account from every Gmail tool. Only value-level disqualifiers — password, hidden, unstateable type — block a whole group, since a canonical group shares one value.
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
createLLMToolSchema), and improvement(tools): tell the model which duplicate tool instance is which #7079 only ever named the credential — so the two tools arrived byte-identical, and the shared credential was correctly suppressed as non-discriminating, leaving nothing at all.INBOXcan otherwise promise a caller it will searchSENT— the model has no way to know the field is fixed. Duplicates additionally get a sentence saying the copies differ.password, a hidden subblock, a secret-named param, and — for a tool whose params resolved an environment variable — every literal value. Resource names are still stated there, since a looked-up name cannot carry the secret.Renamed
tool-binding-labels.tstotool-pinned-params.tsto match what it now does.Type of Change
Testing
31 unit tests, including the exact reported case (same credential, different folder). Verified the guards fail when removed: deleting the
password/hiddenskip, and deleting the secret-withholding branch, each turn the relevant tests red. Full suite green: 3,953 tests,tscclean, all 36 audits pass, lint clean.Behaviour change worth noting for review: descriptions now change for every existing agent that pins any param, not just ones with duplicate tools, and credential lookups now run for every OAuth tool rather than only duplicates. Lookups are deduped per request and memoized for the run, so a credential shared across tools costs one read.
Checklist