[opieter-aws]

Human Overview

Description

Agent state and interrupt state can be restored from a session, and HumanInTheLoop let that restored data outrank the current configuration and the human answering the prompt.

It skipped the prompt for any tool listed under hitl:trustedTools / hitl:trusted_tools, even with enableTrust / enable_trust off. The list is now read only when trust is enabled, and anything other than a list of strings counts as empty.

In inline ask mode the human's answer was passed to Confirm(response=…), which returns a stored response for the same tool call when one exists, so a stored "yes" could outrank a live "no". HITL now decides on the answer it just received (Proceed / Deny). Interrupt and middleware semantics are unchanged from main.

Related Issues

N/A

Documentation PR

Included

Type of Change

Bug fix

Testing

• I ran hatch run prepare

Checklist

• I have read the CONTRIBUTING document
• I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
• My change is focused and reasonably small; I have split unrelated work into separate PRs
• I have added any necessary tests that prove my fix is effective or my feature works
• I have updated the documentation accordingly
• I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
• My changes generate no new warnings
• Any dependent changes have been merged and published

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[github-actions]

Assessment: Comment (approve-leaning)

Solid, well-tested bugfix. The three fixes — gating the trusted-tools read behind enable_trust/enableTrust, validating restored state as a list-of-strings before trusting it, and deciding inline ask answers directly (Proceed/Deny) instead of routing through Confirm(response=…) which could surface a stored interrupt response — each cleanly address a real precedence/restored-state bug. Tests cover the malformed-state and restored-interrupt precedence cases in both SDKs.

Review notes

• Correctness: Core logic changes look correct and match the PR description; interrupt/middleware paths are preserved.
• Cross-SDK parity: One behavioral difference between Python and TS on the "inline ask returns no answer" path (fail-closed deny vs. interrupt/resume) — see inline comment. Pre-existing, but worth reconciling or documenting given this PR reorganizes that path.
• Tests: Comprehensive and parametrized; coverage reported at 100% for modified lines. No duplicate-functionality concerns.
• API: No public API surface change; no api/needs-review needed.

Nice defensive handling of restored session state — validating the trusted-tools value before using it is exactly right.

[mkmeral]

@strandly-the-agent review pls

[strandly-the-agent]

Comment — approve-leaning. Both fixes are real and the new tests genuinely pin them: run against origin/main's SDK source, 7 of the 8 new Python tests and 7 of the 9 new TS tests fail; all pass on 6480fdf91. No blocker from me.

One 🟡, inline: the new Deny reason is the approval prompt, so the model now reads DENIED: Tool "x" denied: Approve "x"? Input: {…} — tool name + "denied" three times and the stated reason is a question. On main it was CONFIRMATION_FAILED: Approve "x"?…, and nothing in either SDK pins that text.

✅ What I verified by running (head 6480fdf91, base 409cb540d)

Check	Result
pytest tests/strands/vended_interventions/test_hitl.py	73 passed
pytest tests/strands/{interventions,vended_interventions} (cedar skipped — cedarpy not installed in my sandbox)	152 passed
vitest run src/vended-interventions/hitl	82 passed (41 × node + chromium), no type errors
vitest run src/interventions src/vended-interventions	267 passed, no type errors
ruff check / ruff format --check / mypy on hitl/	clean
eslint on both changed TS files	clean
Baseline: PR's test_hitl.py against origin/main source	7 failed, 66 passed
Baseline: PR's hitl.test.ts against origin/main hitl.ts	14 failed (7 tests × 2 projects), 68 passed

The two tests that pass on main in each SDK (test_applies_restored_answer_when_no_ask_callback_is_configured, TS pauses and applies the resume answer / applies a restored answer…) are regression guards for the untouched interrupt path — correct that they pass both ways. No echo-the-mock tests: every new case drives a real Agent invocation and asserts on whether the tool body actually ran.

Logs + repro script uploaded as review artifacts (pytest.log, baseline-main-pytest.log, vitest.log, baseline-main-vitest.log, repro-deny-text.py, repro-deny-text.out).

Questions (non-blocking)

1. Release note for the trust gate? Anyone who pre-seeded hitl:trusted_tools / hitl:trustedTools without enable_trust was getting a (never-documented) declarative allowlist, and it stops working here. That's exactly the fix, but it's a silent behaviour change — worth a line in the release notes?
2. mkmeral's "fail closed" call on the TS no-answer branch isn't in this head yet (hitl.ts:234-236 still returns confirm(...)). Following up in this PR, or a separate one? Not re-raising the divergence itself — just tracking the decision so it doesn't get lost on merge.

Reading order for human reviewers

1. strands-py/src/strands/vended_interventions/hitl/hitl.py:312 + :364-379 — the trust gate and the new validating read (the security-relevant half).
2. hitl.py:272-287 — inline answers now decided locally instead of via Confirm(response=…).
3. strands-ts/src/vended-interventions/hitl/hitl.ts:228-237, :294-301 — the TS mirror; naming, deny string and validation are identical to Python.
4. Tests: TestRestoredInterruptResponses / describe('restored interrupt responses') — both build the interrupt id by raising a real interrupt first, which is why they actually collide with the restored entry.

Appendix — checked and cleared (5)

• No aliasing from _trusted_tools returning state's list. Both stores deep-copy on read: strands-py/src/strands/types/json_dict.py:56 (return copy.deepcopy(self._data.get(key))) and strands-ts/src/state-store.ts:9 ("All values are deep copied on get/set"). The returned list can't be mutated into state.
• Dropping the inline event.interrupt(...) write loses nothing. An answered record is filtered out everywhere it could be observed — AgentResult.interrupts (only raised interrupts reach it), background tasks (background_tasks/in_process/_manager.py:409, if interrupt.response is None) — and _InterruptState.end_tool_cycle() (interrupt.py:96-102) drops tool-scoped records at the end of the cycle regardless. setdefault doesn't bump _version, so no extra session write happened on main either. My repro prints interrupts == {} after the turn on both main and this branch. Bonus: it also stops an answered record being revived by _InterruptState.from_dict (interrupt.py:232-237 keeps non-agent-stream ids even when activated=False) — which is the same class of bug as fix #2.
• Nothing depends on the old CONFIRMATION_FAILED string for this path. Only interventions/registry.{py,ts} produce it and only their own tests assert it; vended-interventions/steering/handlers/llm.ts:217 returns its own confirm and never routes through HITL. The harness does use the inline path (harness-py/src/strands_harness/interventions.py:56, harness-ts/src/interventions.ts:57) so it gets the new text, but asserts nothing on it.
• No stale tests left behind despite the +153/-0 and +215/-0 test diffs: no test in either file ever asserted a Confirm from the inline-ask path, or asserted the cancel text, and all pre-existing cases pass unchanged.
• ⚪ _decide_ask_response is a two-line body under a ten-line docstring, called once — inlining it would be shorter, but keeping it mirrors _decideAskResponse, so leaving it is defensible. The mdx line reads slightly better as "only read when trust is enabled"; the fact users most need is that pre-seeding the key no longer skips approval.

Process note: this review is the correctness pass, run as an independent fresh-context subagent. A separate adversarial pass ran afterwards (the first attempt timed out and was re-run on a lighter tier, plus in-context probe reruns of stale-vs-live answers, trust gating, malformed state, a self-interrupting tool restart and evaluate edge cases on main vs 6480fdf91): no new findings — every probe matches the PR's claims, outputs uploaded as adversarial-main-vs-pr.out and hitl-probe-output-{main,pr-6480fdf91}.txt.

[github-actions]

Assessment: Approve

Thanks for addressing the earlier feedback — this looks great. The TS path now fails closed (deny) when the inline ask returns no answer, matching Python's None → Deny, so the cross-SDK divergence I flagged is resolved. Dropping the now-unused prompt param from _decideAskResponse/_decide_ask_response and surfacing distinct, human-readable rejection reasons ("the human rejected the call" vs. "approval callback returned no response") is a nice improvement, and the parametrized tests in both SDKs (plus the sync/async no-answer coverage on the Python side) lock the behavior in. No further concerns from me.

[github-actions]

Documentation Preview Ready

Your documentation preview has been successfully deployed!

Changed pages:

• user-guide/sdk/agents/interventions/human-in-the-loop

Updated at: 2026-10-28T22:47:38.006Z

[mehtarac]

LGTM