Answer the turn when binding the tools fails

[CNSeniorious000]

Closes #14. All three reproduce at 5518478, and one of them is worse than the issue recorded.

1 — a failure before the cell hangs the turn

Building the bindings ran in _handle, outside _exec's handler — the one whose stated purpose is that an exec is answered however it went wrong. So a spec that raised was logged by serve()'s frame guard and answered by nobody:

first exec  -> {"timeout": 4000}                      the promise never settles
shell after -> {"ok": true, "repr": "4"}              the very next cell is fine

A host waiting forever on a shell that is demonstrably healthy is the hardest shape to diagnose.

init is the same bug and strictly more reachable, which #14 did not record: start() awaits ready, nothing else resolves it, and the first start() carries the same specs.

第一次 start(坏 spec) -> TIMEOUT — start() 永不返回

The fix is structural rather than per-input, because the issue's own point is that every pre-exec failure is an unanswerable turn — not just this KeyError. Binding moves inside the try that already guarantees an answer, and the handshake is completed even when nothing binds:

dsh-py-codeact/py/kernel.py

Lines 724 to 727 in a9e64ed

	async def _exec(self, exec_id, shell, code: str, specs=None) -> None:
	try:
	# Binding happens HERE, not in `_handle`: it is the last pre-exec step that can fail, and out there it failed outside this handler — `serve()` logged it and sent nothing, so the host waited forever on a shell that was demonstrably healthy. Every pre-exec failure is an unanswerable turn unless it is raised inside this try.
	session = self._session_for(shell, specs)

dsh-py-codeact/py/kernel.py

Lines 766 to 770 in a9e64ed

	# The handshake must be answered for the same reason an exec must: `start()` awaits `ready` and nothing else resolves it, so a spec that raises here hangs the session before its first cell — and this is the MORE reachable path, since the first `start()` carries the same specs. A shell that binds nothing is recoverable; a spawn that never returns is not.
	try:
	self._session_for(shell, frame.get("tools") or [])
	except Exception: # noqa: BLE001 — see above: never at the cost of the handshake
	print(f"[dsh-py-codeact] {shell}: tools failed to bind: {traceback.format_exc()}", file=REAL_STDERR)

exec   ->  ok=false  KernelError: the kernel failed while running this cell.   同 shell 之后 -> 4
init   ->  start() returned                                                    之后跑 cell -> 21

Two edge cases improve as a side effect: a dispose racing an exec used to rebuild the shell with no bindings, and an init arriving mid-flight used to win over the exec's own catalogue. Both now resolve to the exec's specs. Nothing else calls _exec.

2 — a tool with its own kwargs loses its entire signature

The overflow parameter is hardcoded kwargs, so a tool declaring one collided, inspect.Signature raised, and the blanket contextlib.suppress discarded everything — return annotation and every renderable parameter — which is precisely the regression the comment above it says it fixed.

3 — an unrenderable parameter vanishes instead of degrading

The except kept only a boolean, so name, type and required flag were thrown away. Meanwhile the block renders # ... see notion_patch? and lib/index.js promises "name? still shows the real one" — so a model follows the pointer, sees only limit, and the host rejects its call for an argument it was never shown.

dsh-py-codeact/py/kernel.py

Lines 234 to 242 in a9e64ed

	# The overflow parameter must not collide with a real one. A tool declaring its own `kwargs` made `inspect.Signature` raise, and the suppress below then discarded the WHOLE signature — return annotation and every renderable parameter with it — so `weird?` showed `(**kwargs)` while the prompt showed the full list: exactly the regression the comment above says was fixed.
	overflow = "kwargs"
	while any(p.name == overflow for p in params):
	overflow = f"_{overflow}"
	params.append(inspect.Parameter(overflow, inspect.Parameter.VAR_KEYWORD))
	# Name what was folded, because the prompt block points HERE for it — it renders `# ... see <tool>?` and nothing else lists these. A required parameter appearing in neither place is one the model calls without and the host then rejects it for, with no way to find out why.
	spelled = ", ".join(f"{p.get('name')!r}: {p.get('type') or 'Any'}{'' if p.get('required') else ' = ...'}" for p in dropped)
	note = f"Pass via **{overflow} — these parameter names are not Python identifiers: {spelled}."
	call.__doc__ = f"{call.__doc__}\n\n{note}" if call.__doc__ else note

# before
weird?         (**kwargs)                                          # docstring, return type, params — all gone
notion_patch?  (*, limit: 'int' = Ellipsis, **kwargs) -> 'Any'     # the REQUIRED 'file-path' is nowhere

# after
weird?         (*, kwargs: 'str' = Ellipsis, **_kwargs) -> 'str'
               Pass via **_kwargs — these parameter names are not Python identifiers: 'file-path': str.
notion_patch?  (*, limit: 'int' = Ellipsis, **kwargs) -> 'Any'
               Pass via **kwargs — these parameter names are not Python identifiers: 'file-path': str.

Required is signalled the way the signature signals it: no = ....

Verification

Five assertions added (115 total, all pass), each mutation-checked by reverting the exact line it guards — every mutation fails its own assertion and no other:

mutation	fails
bind back in _handle	a spec that raises answers the exec rather than hanging it
drop the init guard	the same spec at init still completes the handshake
hardcode the overflow name	a tool with its own \kwargs` keeps its whole signature`
skip the docstring note	a parameter it cannot spell is still named where the block points

Note

One existing assertion moved: a same-named tool with a changed schema is rebound compared the whole docstring to 'REVISED', and that spec carries a file-path, so it now also names what the signature could not spell. It reads the first line instead, keeping its subject — the rebind — intact.

The hang tests race an 8 s timeout, so a regression fails the suite rather than wedging it.

Gates: node test/smoke.js, uvx ruff check py/, TY_UV=scripts uvx ty check py/kernel.py — all clean. Merges cleanly onto 8a2d963; the full stack with #22 and #24 runs 130 assertions green.

[coderabbitai]

Warning

Review limit reached

Next included review available in 54 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 803b9864-c931-45b6-a3a2-718c3ab1bea5

📥 Commits

Reviewing files that changed from the base of the PR and between 158e485 and 423298b.

📒 Files selected for processing (3)

• lib/index.js
• py/kernel.py
• test/smoke.js

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0ec3dcee-aa40-4389-9b32-2d353ea73abb

📥 Commits

Reviewing files that changed from the base of the PR and between 5518478 and 158e485.

📒 Files selected for processing (3)

• lib/index.js
• py/kernel.py
• test/smoke.js

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

变更摘要

• 将工具绑定移入 _exec 错误处理范围。畸形工具规格现在返回 KERNEL_UNBOUND 错误，不再导致 exec 或 start() 握手挂起。
• _make_binding 现在保留无法渲染参数的名称、类型和必填状态，并写入生成的文档字符串。
• 避免工具参数 kwargs 与兜底 **kwargs 冲突，确保返回注解和可渲染参数不会丢失。
• 新增 KERNEL_UNBOUND 导出，并避免将绑定失败记录为已完成绑定。
• 新增 5 项断言，115 项断言、Node smoke tests、Ruff 和类型检查均通过。

Walkthrough

本次变更处理工具目录绑定失败，并扩展参数自省结果。内核会返回可识别的失败应答，初始化仍完成握手，外壳不会缓存未成功发送的绑定。

Changes

工具目录绑定与自省

Layer / File(s)	Summary
参数自省与绑定构造 py/kernel.py, test/smoke.js	_make_binding 会记录不可渲染参数，处理更多异常类型，生成不冲突的 **kwargs 参数，并将可描述参数追加到文档字符串。
绑定失败应答与重试 py/kernel.py, lib/index.js, test/smoke.js	_exec 在受保护范围内执行绑定；绑定失败时返回 UNBOUND 错误。init 仍发送 ready。外壳不会记忆未成功发送的绑定。测试覆盖执行、握手和后续调用。

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to 158e4

The change prevents failed tool binding from hanging startup or execution and avoids losing tool signature details. A bounded lifecycle risk remains if session disposal overlaps an already accepted execution, because the disposed shell could briefly be recreated with its tool access; the PR is mergeable with explicit owner awareness or follow-up.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	标题准确概括了工具绑定失败时保证请求得到应答这一主要变更。
Description check	✅ Passed	描述与变更相关，并说明了三个故障模式、修复方式和验证结果。
Linked Issues check	✅ Passed	PR 满足 Issue #14 的全部编码目标：保持 exec 和 init 的活性，避免 kwargs 名称冲突，并保留不可渲染参数的信息。[#14]
Out of Scope Changes check	✅ Passed	代码和测试变更均围绕 Issue #14 的工具绑定失败、签名渲染和请求活性问题，没有发现无关改动。

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[sourcery-ai]

你好——我已经审阅了你的更改，整体看起来很棒！

Sourcery 评估

已批准。

---

Sourcery 对开源项目免费——如果你喜欢我们的评审，请考虑分享给他人 ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{请帮助我变得更有用！请在每条评论上点击 👍 或 👎，我会利用反馈来改进评审。}

Original comment in English

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Approved.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

Sourcery withdrew this approval because the latest commits introduced blocking findings.

[CNSeniorious000]

Follow-up worth doing once this and #33 are both in: a cross-half assertion on the fold note.

Both halves name the folded parameters from the same spec, in their own words — this PR gives <tool>? its note, #33 gives the block's comment the = ... marker — so they can disagree about which parameter the tool cannot run without, and neither half's tests would notice, because each reads only its own output.

The test drives ONE schema through the real pipeline (toolSpecs → both renderToolsSection and a live PythonKernel) and compares on content rather than wording, since the block spells JSON strings and the kernel spells repr:

const folds = (text) => [...text.matchAll(/["']([^"']+)["']: (\w+)( = \.\.\.)?/g)].map((m) => `${m[1]}: ${m[2]}${m[3] ? ' optional' : ' REQUIRED'}`).sort()
assert.deepEqual(folds(kernelNote), folds(blockNote))

It cannot live in either PR alone: against main it fails because folded.__doc__ is only the description (no note at all), and against either branch on its own it fails because the other half has not been updated yet. Whichever of the two merges second is where it belongs.