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.

[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.