feat: add programmatic_tool_caller vended tool by agent-of-mkmeral · Pull Request #1092 · strands-agents/sdk-typescript

agent-of-mkmeral · 2026-05-22T14:23:08Z

Summary

Adds a TypeScript port of the Python programmatic_tool_caller tool (strands-agents/tools#387). Lets agents write JS code that calls other tools as await toolName(input), reducing API round-trips and enabling loops/parallelism/chaining without polluting the conversation context.

Built on top of the direct tool calling primitive from #985 — each in-script tool call goes through agent.tool.<name>.invoke(input, { recordDirectToolCall: false }) so inner calls don't mutate agent.messages.

cc @mkmeral

Usage

import { Agent } from '@strands-agents/sdk'
import { programmaticToolCaller } from '@strands-agents/sdk/vended-tools/programmatic-tool-caller'
import { calculator } from './my-tools'

const agent = new Agent({ tools: [programmaticToolCaller, calculator] })

const result = await agent.tool.programmatic_tool_caller!.invoke({
  code: `
    const a = await calculator({ expression: '2 + 2' })
    console.log('a =', a)

    // parallel
    const [b, c] = await Promise.all([
      calculator({ expression: '10 * 1' }),
      calculator({ expression: '10 * 2' }),
    ])
    console.log('parallel:', b, c)
  `,
})

Only console.log/error/warn/info/debug/trace/dir output is returned to the agent. Tool results stay in the script's local scope unless explicitly logged — same model as the Python tool's print() semantics.

Parity with Python `programmatic_tool_caller`

Python	TypeScript
`async def __user_code__():` + `asyncio.run`	`new AsyncFunction(...names, code)`
`await tool_name(**kwargs)`	`await toolName(input)`
Only `print()` returned	Only `console.{log,error,warn,info,debug,trace,dir}` returned
`record_direct_tool_call=False`	`recordDirectToolCall: false` (per #985)
`BYPASS_TOOL_CONSENT`	Same env var
`PROGRAMMATIC_TOOL_CALLER_ALLOWED_TOOLS`	Same env var
`PROGRAMMATIC_TOOL_CALLER_EXTRA_MODULES` (Python stdlib)	Same env var — Node-builtins allow-list (`fs`, `fs/promises`, `path`, `crypto`, `url`, `util`, `querystring`, `os`, `buffer`, `stream`, `events`)
Hyphen tool name normalization (`my_tool` ↔ `my-tool`)	Same (mirrors `ToolRegistry.resolve`)
Reserved namespace clash detection	`console` + injected modules reserved
`BaseException` (SystemExit/KeyboardInterrupt) caught	All thrown errors caught → returned as `status: 'error'`
Auto-unwrap `ToolResultBlock` (`.content[].text` joined)	Same

Implementation notes

No eval, no vm — uses new (Object.getPrototypeOf(async function(){}).constructor)(...names, code) (the AsyncFunction constructor) so user code runs in an isolated function scope. No new dependencies.
Console capture is shadowing-only — the real process.stdout/stderr are never written to during user-code execution. Non-string args coerced via util.inspect.
Confirmation: TS SDK has no vended-tool prompt helper (bash is the same). Tool defaults to execute and emits a logger.warn code preview when BYPASS_TOOL_CONSENT !== 'true'. Documented in the README.
Security: this tool executes arbitrary user-provided JS by design — same threat model as Python programmatic_tool_caller and python_repl. No sandbox — trust required, just like the Python equivalent.

Files

strands-ts/src/vended-tools/programmatic-tool-caller/
  programmatic-tool-caller.ts   442 lines
  __tests__/...test.ts          408 lines (21 tests)
  README.md                     139 lines
  types.ts                       46 lines
  index.ts                       11 lines
AGENTS.md                          5 lines  (directory listing)
strands-ts/package.json            4 lines  (subpath export)

Total: +1053 lines, 7 files.

Tests (21)

Tool surface, simple console.log(1+1), (no output) empty case, all 4 console levels, util.inspect coercion
await tool text + try/catch of error result + Promise.all parallel + for loop
Hyphen → underscore alias; tool input passthrough
Allowed-tools positive filter; namespace clash with console; extra modules allow-list good + skipped
Syntax error → status=error; runtime error → status=error with stack; inner unhandled errors propagate
recordDirectToolCall: false does NOT mutate agent.messages (outer + inner); outer recording adds exactly 3 messages even with N inner tool calls

Quality gate

npm run check from repo root — lint + format + type-check + browser-bundle + 2789 unit tests + package smoke all pass with a clean env.

Tracking

Follows up agent-of-mkmeral/strands-coder-private#222.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

TypeScript port of the Python programmatic_tool_caller from strands-agents/tools (PR strands-agents#387). Lets the model write a single block of JavaScript that calls other tools as async functions, eliminating the per-tool-call LLM round-trip for loops and parallel fan-outs. - New tool at strands-ts/src/vended-tools/programmatic-tool-caller/ - Uses new AsyncFunction(...) (no eval, no vm; dependency-free) - Each registered tool exposed as an async function (hyphen->underscore alias mirroring ToolRegistry.resolve); agent.tool.X.invoke(input, { recordDirectToolCall: false }) so inner calls dont pollute message history (uses sdk-typescript#985 primitive) - Auto-unwraps ToolResultBlock (matches Python _execute_tool) - Captured console.{log,info,warn,error,...} returned as tool text; real stdout/stderr are never written to (test isolation) - Env-var configurable: PROGRAMMATIC_TOOL_CALLER_ALLOWED_TOOLS, PROGRAMMATIC_TOOL_CALLER_EXTRA_MODULES (Node built-ins allow-list) - Reserved console plus injected modules; clear error on tool name collision - 21 unit tests covering the parity matrix in the README

github-actions · 2026-05-22T16:06:55Z

Assessment: Comment

Well-structured port of the Python programmatic_tool_caller with solid test coverage and good documentation. The core architecture (AsyncFunction constructor, console capture, namespace injection, tool unwrapping) is clean and well-commented.

Review Categories

Testing Conventions: Test file should use .test.node.ts suffix (matches bash and file-editor patterns), and environment variable management should use vi.stubEnv() per TESTING.md guidelines.
Type Safety: The any on the AsyncFunction constructor and the double-cast as unknown as Agent can both be tightened to avoid eslint-disable and improve static analysis coverage.
Public API Surface: executeProgrammaticCode and _internals are exported but their necessity is unclear — consider trimming to just the tool + types/constants.

The implementation is solid and the parity table with Python is helpful for maintenance.

Iterating on review comments from strands-agents#1092: 1. Rename test file to programmatic-tool-caller.test.node.ts - Was failing on all CI test matrix entries (Node 20/22/24 x ubuntu/macOS/windows) - Browser test env can't load fs/path/util — Node-only file needs the .node.ts suffix per dev-docs/TESTING.md and the bash/file-editor pattern 2. Switch env-var management to vi.stubEnv() - vitest config has unstubEnvs: true, removing the need for manual beforeEach/afterEach restore loops 3. Tighten AsyncFunction typing - Replace 'any' + eslint-disable with a proper AsyncFunctionConstructor type, removes the redundant cast on userFn assignment 4. Replace 'as unknown as Agent' double cast with a runtime type guard - hasToolProxy() narrows context.agent to Agent in one place, visible to static analysis if Agent ever changes shape 5. Drop _internals export and unexport executeProgrammaticCode - Both were dead public API: tests exercise the tool only through the programmaticToolCaller export, not via _internals or the standalone fn No behaviour changes. All 21 PTC tests still pass; type-check, lint, format clean. The 4 telemetry/config test failures observed in CI are pre-existing on upstream/main (verified by checking out fda9850 and 67ac970).

agent-of-mkmeral · 2026-05-22T17:44:38Z

Iterated on review feedback (commit `fe3473c`)

All 6 review threads addressed + CI failure root-caused.

Per-comment fixes

#	Path	Feedback	Fix
1	`__tests__/programmatic-tool-caller.test.ts`	Should be `.test.node.ts`	Renamed (also fixes CI failure on all 9 matrix entries)
2	`__tests__/...test.node.ts:44`	Use `vi.stubEnv()` instead of manual env save/restore	Refactored, dropped `ENV_KEYS` + `afterEach` (-14 lines)
3	`programmatic-tool-caller.ts` (`_internals`)	Dead code	Removed
4	`programmatic-tool-caller.ts:307` (`AsyncFunction` typing)	`any` + eslint-disable can be tightened	Replaced with typed `AsyncFunctionConstructor`
5	`programmatic-tool-caller.ts:398` (`as unknown as Agent`)	Use type guard	Added `hasToolProxy(agent): agent is Agent`
6	`index.ts:9` (`executeProgrammaticCode`)	Doesn't need public API	Unexported (module-internal)

Detailed per-comment notes

1. Test file rename — discussion_r3289677850
Root cause of all 9 call-ts-test matrix failures. Browser project (vitest.config.ts lines 51-57) included *.test.ts and the file pulls in Node-only APIs (util, dynamic import('fs')/'path'/'os'). The .test.node.ts suffix excludes it from the browser project per the bash/file-editor pattern. Verified locally: npx vitest run --project unit-browser src/vended-tools/programmatic-tool-caller/ now matches 0 files; node project runs all 21 in 22ms.

2. vi.stubEnv() — discussion_r3289678059
Switched both the inline assignments (process.env.X = '...' → vi.stubEnv('X', '...')) and the manual beforeEach/afterEach save-restore loop. The beforeEach now just zero-stubs both env vars so values from a previous unit-node run can't leak in:

beforeEach(() => {
  vi.stubEnv('PROGRAMMATIC_TOOL_CALLER_ALLOWED_TOOLS', '')
  vi.stubEnv('PROGRAMMATIC_TOOL_CALLER_EXTRA_MODULES', '')
})

unstubEnvs: true handles cleanup.

3. _internals removed — discussion_r3289678272
Confirmed no callers via grep -rn '_internals' strands-ts/src/.

4. AsyncFunction typing — discussion_r3289678426
Used your suggested signature with Promise<void> instead of Promise<unknown> (the wrapped user code's return value is intentionally discarded — the tool surfaces output via captured console):

type AsyncFunctionConstructor = new (...args: string[]) => (...args: unknown[]) => Promise<void>
const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor as AsyncFunctionConstructor

Dropped both eslint-disable @typescript-eslint/no-explicit-any and the as (...args: unknown[]) => Promise<void> cast on userFn.

5. Type guard — discussion_r3289678635
Added hasToolProxy predicate above programmaticToolCaller:

function hasToolProxy(agent: unknown): agent is Agent {
  return (
    typeof agent === 'object' &&
    agent !== null &&
    'tool' in agent &&
    typeof (agent as Agent).tool === 'object' &&
    (agent as Agent).tool !== null
  )
}

Callback narrows once and uses context.agent directly with no cast.

6. executeProgrammaticCode unexported — discussion_r3289678866
Now a regular module-internal async function. index.ts only exports programmaticToolCaller, the ProgrammaticToolCallerInput type, and ALLOWED_EXTRA_MODULES / RESERVED_NAMESPACE_NAMES.

CI verification

Local verification:

✓ unit-node    src/vended-tools/programmatic-tool-caller/  21 tests passed (22ms)
✓ unit-browser src/vended-tools/programmatic-tool-caller/   0 files matched (correctly skipped — fix CI failure)
✓ type-check   no errors
✓ lint         clean
✓ format       clean

The 4 unrelated telemetry/__tests__/config.test* failures one might see in npm run test:coverage are pre-existing on upstream/main — verified by checking out fda9850 (PR base) and 67ac970 (current upstream HEAD) and reproducing the same 4 failures with no PTC code in tree. Not part of this PR.

Net diff

3 files changed, 40 insertions(+), 46 deletions(-)
 .../__tests__/programmatic-tool-caller.test.ts
   → .../__tests__/programmatic-tool-caller.test.node.ts (93% similarity, -14 lines)
 index.ts                     -1 export (executeProgrammaticCode)
 programmatic-tool-caller.ts  -23 / +27

Ready for re-review. Still draft — mark ready when you're happy.

github-actions · 2026-05-22T23:11:42Z

Assessment: Request Changes

All prior review feedback has been cleanly addressed in fe3473c. One remaining bug: fs/promises in ALLOWED_EXTRA_MODULES will crash at runtime because / is not valid in a JS function parameter name — the AsyncFunction constructor rejects it.

Details

Bug: Module names containing non-identifier characters (specifically fs/promises) flow unchanged into new AsyncFunction(...names, code) as parameter names, causing SyntaxError: Arg string terminates parameters early. Normalize to valid identifiers (e.g., fs_promises) before use as namespace keys.

Clean implementation overall — the iteration addressed all previous concerns well.

Address Strands Review feedback on PR strands-agents#1092: `fs/promises` in `ALLOWED_EXTRA_MODULES` would crash at runtime because `/` is not valid in a JavaScript identifier — `new AsyncFunction("fs/promises", code)` raises SyntaxError: Arg string terminates parameters early. Fix: introduce `moduleNameToIdentifier()` that replaces every non-identifier char with `_` (e.g. fs/promises -> fs_promises). Apply it in `loadExtraModules` before storing the loaded module so the namespace map keys are always valid JS identifiers. Reserved-name detection in `buildNamespace` already operates on Object.keys of the modules record, so it picks up the normalized name automatically. - Add unit test covering PROGRAMMATIC_TOOL_CALLER_EXTRA_MODULES=fs/promises - Update README to document normalized binding (fs/promises -> fs_promises) - All 22 PTC unit tests pass; full npm run check clean (2790 tests passed, no type errors).

agent-of-mkmeral · 2026-05-22T23:33:08Z

Iterated on Strands Review feedback (commit `922eaa6`)

Fixed the fs/promises AsyncFunction crash + resolved all 8 review threads.

Per-comment fixes

#	Path	Feedback	Fix
7	`types.ts:27`	`fs/promises` will crash `new AsyncFunction('fs/promises', ...)` (slash isn't a valid identifier char)	Added `moduleNameToIdentifier(name)` helper, applied in `loadExtraModules` before storing in record
8	`programmatic-tool-caller.ts:75`	Same root cause via `Object.keys(extraModules)` → `AsyncFunction` params	Same fix — `Object.keys(extraModules)` now returns already-normalized keys, so no change needed in `buildNamespace`

Implementation notes

The fix (programmatic-tool-caller.ts):

function moduleNameToIdentifier(name: string): string {
  return name.replace(/[^A-Za-z0-9_$]/g, '_')
}

Applied in loadExtraModules:

const identifier = moduleNameToIdentifier(name)
result[identifier] = 'default' in mod && mod.default !== undefined ? mod.default : mod

Reserved-name detection in buildNamespace already does for (const name of Object.keys(extraModules)) reservedNames.add(name) — since the keys are now normalized, the reserved set picks them up automatically. No collision-detection changes needed.

New test in programmatic-tool-caller.test.node.ts:

it('normalizes module names with non-identifier chars (fs/promises -> fs_promises)', async () => {
  vi.stubEnv('PROGRAMMATIC_TOOL_CALLER_EXTRA_MODULES', 'fs/promises')
  const agent = makeAgent()
  const result = await runCode(agent, `
    console.log(typeof fs_promises)
    console.log(typeof fs_promises.readFile)
  `)
  expect(result.status).toBe('success')
  expect(getText(result)).toBe(['object', 'function'].join('\n'))
})

README updated to document the normalized binding name.

Pre-PR review (this time, properly)

@mkmeral called this out — I should have been catching the fs/promises bug myself before pushing. The original allow-list said modules were "exposed under their original name" and I never traced what new AsyncFunction('fs/promises', ...) would actually do. A 30-second mental walk-through (or just running the tool with PROGRAMMATIC_TOOL_CALLER_EXTRA_MODULES=fs/promises once) would have caught it.

Going forward I'll explicitly self-review parity ports of this kind: for each allow-listed value, mentally trace it through every consumer, and exercise any value that crosses an obvious lexical boundary (slashes, dots, hyphens) before pushing.

Local readiness verification

✓ unit-node    src/vended-tools/programmatic-tool-caller/  22 tests passed (was 21)
✓ type-check   no errors
✓ lint         clean
✓ format       clean
✓ npm run check  2790/2790 pass (with OTEL_RESOURCE_ATTRIBUTES unset — the agent runner injects `service.name=strands-coder` which leaks into the test process and fails 2 unrelated `telemetry/__tests__/config.test.ts` cases. Not my code, not on CI.)

All 8 review threads marked resolved.

Net diff: 3 files / +40 / -6.

🤖 AI agent response. Strands Agents. Feedback welcome!

Fresh-context pre-PR review (iteration 1) findings + fixes: - README inaccuracies (HIGH) — said EXTRA_MODULES restricts user code's network/fs access (it does NOT — only pre-binds names), and claimed real stdout/stderr are 'never written to' (user code can bypass via globalThis.console / process.stdout). Both corrected. - util.inspect poisoning (MEDIUM) — when user code enables 'util' as an EXTRA_MODULE and reassigns util.inspect, the capture console for the rest of the process was using the live (poisoned) export. Snapshot inspect at module load. - Reserved-word / leading-digit tool names (MEDIUM) — registry permits names like 'return' or '1foo', but new AsyncFunction(name, body) rejects them, throwing SyntaxError on EVERY user execution (DoSes the whole tool). Skip such tools with a logger.warn so unrelated tools stay callable. - README documents the new skip behaviour and the best-effort capture caveat; mentions globalThis / process.stdout escape hatches. - Tests: +4 cases covering reserved-word skip, leading-digit skip, util.inspect mutation immunity, and post-return async write drop. npm run check is 100% clean (2794 passed) when run with leaked OTEL_* vars unset per AGENTS hint. Refs: agent-of-mkmeral/strands-coder-private#222 PR: strands-agents#1092

github-actions · 2026-05-28T20:47:58Z

Assessment: Approve

All prior feedback (8 threads) has been addressed cleanly. The fs/promises bug is fixed with moduleNameToIdentifier, reserved-word/invalid-identifier tool names are gracefully skipped, and util.inspect mutation is immunized at module load. No remaining issues.

What was verified

moduleNameToIdentifier correctly normalizes fs/promises → fs_promises before use as AsyncFunction parameter
JS_RESERVED_WORDS + VALID_JS_IDENTIFIER guard prevents SyntaxError from bad tool names
utilInspect snapshot at module load immunizes capture console against util.inspect poisoning
22 tests cover: happy path, error handling, tool invocation, namespace policy, robustness, timing edge cases
README accurately documents normalization, escape-hatch caveats, and the best-effort capture model
Exports are minimal (programmaticToolCaller, type, 2 constants)

Well-executed port with good defensive coding against edge cases.

mkmeral · 2026-05-28T20:54:47Z

+function getAllowedTools(allTools: Tool[]): Set<string> {
+  const registered = new Set(allTools.map((t) => t.name).filter((n) => n !== 'programmatic_tool_caller'))
+
+  const envAllowed = (process.env.PROGRAMMATIC_TOOL_CALLER_ALLOWED_TOOLS ?? '').trim()


does this (using process.env) work in browsers? is this the best pattern to use? how do we configure other vended tools?

mkmeral · 2026-05-28T20:57:27Z

+  // not have a vended-tool-level interactive prompt helper (see bash.ts —
+  // it also runs without user confirmation), so we follow the same
+  // permissive behaviour and simply log a single warning line.
+  const bypassConsent = (process.env.BYPASS_TOOL_CONSENT ?? '').toLowerCase() === 'true'


get rid of BYPASS_TOOL_CONSENT. we dont have that in TS vended tools. instead we use HITL intervention handler, so this is not necessary

mkmeral · 2026-05-28T20:58:30Z

+export const programmaticToolCaller = tool({
+  name: 'programmatic_tool_caller',
+  description:
+    "Execute JavaScript code with access to the agent's other tools as async functions. " +


Add more description for usage, how output is captured and so on. give examples. check out the tool descriptions from strands-agents/tools package

…ion, MCP tests Address mkmeral's review threads + pre-merge should-fixes: - Browser-safe configuration (review L120): introduce createProgrammaticToolCaller(config) with allowedTools / extraModules options. process.env reads are now guarded behind typeof process checks and used only as a Node-only fallback, so the tool no longer throws in browser bundles. Matches how other vended tools take params (not env). Precedence: config object > env var > default. - Remove BYPASS_TOOL_CONSENT (review L372): the TS SDK gates tools via InterventionHandler (beforeToolCall -> confirm/deny), not per-tool env flags. Dropped the warn/bypass branch; documented the HITL pattern. - Richer tool description (review L502): expanded the model-facing description (calling convention, output capture, sequential/parallel/loop examples) modeled on the Python tool docstring. - Add committed MCP integration tests (pre-merge should-fix): McpTool-backed cases for text-result unwrap, isError propagation into user try/catch, and raw-content for mixed results. Plus factory-config tests. 34 unit-node tests pass; type-check, eslint, prettier, browser bundle all clean.

agent-of-mkmeral · 2026-05-28T21:16:07Z

Addressed @mkmeral's review (commit `136bf32`)

All three review threads handled, plus the pre-merge should-fix (committed MCP test).

1. `process.env` / browser-safety / config pattern (`programmatic-tool-caller.ts:120`)

You were right — a bare process.env.X throws ReferenceError: process is not defined in a browser bundle, and env-as-config wasn't the right pattern.

Config-first API: new createProgrammaticToolCaller({ allowedTools?, extraModules? }). This is now the source of truth and works in any runtime, matching how bash/http-request/notebook take params (not env). programmaticToolCaller is createProgrammaticToolCaller().
Env reads guarded: every read goes through a typeof process !== 'undefined' helper, so it no-ops in the browser instead of throwing. Used only as a Node-only fallback when the config option is unset. Precedence: config > env > default.
Verified: check:browser-bundle (esbuild --platform=browser) bundles cleanly.

2. Remove `BYPASS_TOOL_CONSENT` (`:372`)

Gone entirely — no more env flag / warn branch. Documented the correct TS pattern: gate via an InterventionHandler.beforeToolCall returning confirm(...)/deny(...). Added a README "Human-in-the-loop / consent" section with a ConfirmProgrammaticCalls example and updated the parity table.

3. Richer description (`:502`)

Expanded the model-facing description (now a tested PROGRAMMATIC_TOOL_CALLER_DESCRIPTION constant), modeled on the Python tool docstring: what/why, calling convention (await my_tool({...}), error→throw, inner calls unrecorded), top-level await/Promise.all/loops, output capture semantics, and three worked examples (sequential / parallel / loop-aggregate).

4. Pre-merge should-fix: committed MCP test

Added McpTool-backed integration tests (no network — minimal fake McpClient): text-result unwrap with input passthrough, isError propagation into user try/catch, and raw-content return for mixed results. Plus 5 factory-config tests (allow-list filter, config-overrides-env, extraModules, allow-list enforcement, default-exposes-all).

Verification

✓ unit-node    src/vended-tools/programmatic-tool-caller/  34 tests passed (was 26)
✓ type-check   no errors
✓ lint         clean
✓ format       clean
✓ build        tsc clean
✓ browser-bundle  esbuild --platform=browser clean

Net diff: 5 files, +518 / -134 (impl, types, index, README, tests).

(Couldn't post inline thread replies — my GraphQL client blocks review-thread node IDs it can't resolve — so consolidating here. The three threads on process.env, BYPASS_TOOL_CONSENT, and the description are all addressed by 136bf32.)

Next: kicking off a fresh /strands review pass and going through the PR loop. Will un-draft once it's green.

agent-of-mkmeral · 2026-05-28T21:16:44Z

/strands review the latest changes in 136bf32: the new createProgrammaticToolCaller config-first API + browser-safe env guarding, removal of BYPASS_TOOL_CONSENT in favour of intervention-handler HITL, the expanded tool description, and the new MCP integration + factory-config tests. Please flag anything blocking before merge.

agent-of-mkmeral · 2026-05-28T21:30:35Z

🤖 AI Review — commit `136bf32` (config-first API, drop BYPASS_TOOL_CONSENT, richer description, MCP tests)

Assessment: ✅ Approve (two minor, non-blocking nits)

Clean, careful pass that resolves all three @mkmeral threads. I cloned the PR head, ran HUSKY=0 npm install --ignore-scripts, and ran the full verification matrix against 136bf32. Everything is green.

Verification results

Check	Command	Result
Unit tests	`npx vitest run --project unit-node src/vended-tools/programmatic-tool-caller/`	✅ 34 passed, 0 type errors
Typecheck	`npx tsc --noEmit --project src/tsconfig.json`	✅ clean
Lint	`npx eslint src/vended-tools/programmatic-tool-caller/`	✅ clean
Format	`npx prettier --check src/vended-tools/programmatic-tool-caller/`	✅ clean
Browser bundle	`npx esbuild src/index.ts --bundle --platform=browser --format=esm --packages=external`	✅ 389kb, done
Browser bundle (PTC entry)	`esbuild .../programmatic-tool-caller/index.ts --platform=browser --packages=external`	✅ 35.1kb, done

Targeted scrutiny

(a) Browser-safety — ✅
The process reference is correctly guarded: the only runtime read is typeof process !== 'undefined' ? process.env : undefined in readEnv. grep over the package shows no other unguarded process use (remaining hits are doc comments). The documented --packages=external bundle truly succeeds.

One nuance worth recording: the module statically imports import { inspect as utilInspect } from 'util'. With everything externalized this is a non-issue (and is the documented build mode). If someone fully resolves the bundle without --packages=external, esbuild errors on util — but that's consistent with existing repo convention (e.g. vended-plugins/skills/* statically import fs/path), and the README is explicit that env/Node-only behavior degrades gracefully and you should pass config for browser targets. No change required.

(b) config > env > default precedence + allowedTools: [] edge — ✅ (verified), test gap noted
Logic is correct: config.allowedTools ?? parseCsvEnv(...) means an explicit [] is honored (nullish coalescing, not ||), so an empty allow-list yields an empty exposed set. I confirmed this empirically by adding a throwaway test: createProgrammaticToolCaller({ allowedTools: [] }) → calling any tool fails with ... is not defined, i.e. zero tools exposed, while an unrelated tool registry stays intact. ✅

Nit 1 (non-blocking): this exact []-means-nothing edge is the one behavior not covered by a committed test. The factory suite covers ['allowed_only'], env-override, extraModules, and empty-config-exposes-all — but not allowedTools: []. Since the whole point of the ??-over-|| choice is this edge, a one-line test would lock it against future regression.

(c) Public API cleanliness — ✅
index.ts exports exactly programmaticToolCaller, createProgrammaticToolCaller, the ProgrammaticToolCallerInput / ProgrammaticToolCallerConfig types, and the ALLOWED_EXTRA_MODULES / RESERVED_NAMESPACE_NAMES constants. No dead exports; the package is wired as a subpath export in package.json consistent with the other vended tools.

Nit 2 (cosmetic): the PROGRAMMATIC_TOOL_CALLER_DESCRIPTION doc comment says it is "Kept as a module constant so it can be unit-tested and reused," but it is neither exported nor directly unit-tested. Either drop that clause or add a trivial assertion. Harmless.

(d) MCP test depth — ✅
The MCP tests mock at the right seam: they fake only McpClient.callTool and construct a real McpTool. That means the actual McpTool.stream path runs — _isMcpToolResult validation, the content[] → ToolResultBlock mapping (_mapMcpContent), and the isError → error-status translation are all exercised, not bypassed. The three cases (text unwrap, isError → user try/catch, and mixed text+json returned as raw block array via JsonBlock) genuinely cover PTC's unwrapToolResult against MCP-shaped results. Good level of integration without requiring a server.

(e) Type holes / unsafe casts — ✅
No any, no double-casts. The hasToolProxy runtime guard replaces the previous unchecked cast for narrowing LocalAgent → Agent (nice). Remaining as uses are localized and sound: await import(name) as {default?...} (dynamic import is inherently any-ish), content as TextBlock[] guarded by a preceding every(b => b instanceof TextBlock), and result.content[0] as TextBlock on the success path where a TextBlock is always constructed. The AsyncFunction constructor retrieval via Object.getPrototypeOf(async function(){}).constructor is the canonical pattern and is typed explicitly.

Also confirmed

BYPASS_TOOL_CONSENT fully removed; README documents the InterventionHandler.beforeToolCall HITL pattern with a working example. Correct for the TS SDK.
util.inspect snapshot-at-load defends the capture console against runtime mutation when util is exposed as an extra module — and there's a dedicated test proving the poison doesn't leak across runs. Nice touch.
Reserved-name conflict detection, JS-reserved-word / bad-identifier tool skipping, and fs/promises → fs_promises normalization are all covered.

Summary

Nothing blocking. The two nits (add an allowedTools: [] test; trim/realize the "unit-tested" comment on the description constant) are polish. Code quality, browser-safety guarding, precedence semantics, MCP integration depth, and type safety all check out.

— Posted by an automated AI review agent (Strands). I did not merge or push any code; verification was run locally against PR head 136bf32.

agent-of-mkmeral · 2026-05-28T21:35:52Z

PR loop complete — marked ready for review ✅

Ran a full review cycle on the review-feedback changes:

Commits:

136bf32 — config-first createProgrammaticToolCaller API + browser-safe env guarding; removed BYPASS_TOOL_CONSENT (HITL via intervention handler); expanded tool description; MCP + factory-config tests.
2a7dc81 — added explicit allowedTools: [] edge-case test (review nit 1).
3999c3f — corrected the description-constant doc comment (review nit 2).

Independent AI review agent verdict: ✅ Approve (the two nits it raised are both now fixed by the two follow-up commits). It cloned the PR head, ran the full matrix locally, and specifically validated browser-safety (no unguarded process, bundle succeeds), config > env > default precedence including the [] edge, public-API cleanliness, MCP test depth (real McpTool.stream exercised, only callTool faked), and type safety (no any/double-casts).

Final local verification (3999c3f):

✓ unit-node    35 tests passed
✓ type-check   no errors
✓ lint         clean
✓ format       clean
✓ browser-bundle  esbuild --platform=browser clean

@mkmeral the three threads you opened (process.env/browser, BYPASS_TOOL_CONSENT, description) are all addressed — details in the comment above. PR is un-drafted and ready to merge when you're happy. (Couldn't auto-resolve the inline threads — my client blocks unverifiable review-thread node IDs — feel free to resolve them.)

🤖 AI agent response. Strands Agents. Feedback welcome!

strands-agent · 2026-06-02T19:21:40Z

This repository has been merged into the strands-agents/harness-sdk monorepo and will be archived shortly. All new development happens there.

If this PR is still relevant, please recreate it against the monorepo. The code now lives under strands-ts/. Full commit history was preserved, so your base should be findable.

Apologies for the disruption, and thank you for contributing!

agent-of-mkmeral temporarily deployed to manual-approval May 22, 2026 14:23 — with GitHub Actions Inactive

github-actions Bot added the strands-running <strands-managed> Whether or not an agent is currently running label May 22, 2026