Conversation
conversation.chat.turnTail is a list slot since DSH 0.1.6-alpha.2: every entry needs an id and the host injects the owner props instead of a chain matched value. Registering with select and no id throws 'list slot "conversation.chat.turnTail" requires options.id' and the whole plugin client entry fails to activate on 0.1.6/0.1.7. Move the routing decision into an exported selectTurnTail(store) and a thin entry component that renders nothing when the selector declines; register with id and no select. Spec updated for the list contract. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Hengne Li <Hengne.Li@cern.ch>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The list-slot entry may duplicate the default produced-files row; documentation and suspended-state test coverage also need updates.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
Updates turn-tail interception to support the host’s list-slot contract and restore plugin activation.
Changes:
- Adds a list-slot ID and moves routing into
selectTurnTail. - Conditionally renders produced-file entries.
- Expands registration and selector tests.
| File | Summary |
|---|---|
tests/turn-tail-intercept.spec.ts |
Tests list-slot registration and selector behavior. |
src/client/intercept.tsx |
Implements list-slot registration and routing. Critical (1 vote): may render duplicate produced-files rows. Nit (2 votes): stale chain-slot documentation. Nit (1 vote): missing suspended-state selector test. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const SidebarProducedFilesEntry = (props: { openInSidebar: (path: string) => void, onShowInFolder: (files: readonly string[]) => void }) => { | ||
| const matched = select(props) | ||
| if (matched === null) return null | ||
| return <SidebarProducedFiles {...props} matched={matched} /> |
There was a problem hiding this comment.
Agreed — a list entry sits next to the deliverables row and duplicates it, and shadowing the deliverables cell would also hide its changed-files card. Reworked: the chain takeover stays for hosts ≤ 0.1.6-alpha.1, and on list-slot hosts the registration is skipped (only the requires options.id refusal is swallowed; anything else rethrows). No second row there; the default row's chips already open in the sidebar because this plugin's editor/files tab types are in the extension band, which takes over the builtin kind's address claims. Spec covers the skip and the rethrow.
| // `conversation.chat.turnTail` is a LIST slot since DSH 0.1.6-alpha.2 (it was | ||
| // a chain slot before): every entry needs an `id`, the host renders all | ||
| // entries and injects the owner props instead of a chain `matched`, so the | ||
| // routing decision moves into the entry component, which renders nothing | ||
| // when the selector declines. Registering with `select` and no `id` throws |
There was a problem hiding this comment.
Rewrote the leading JSDoc: it now documents both host contracts (chain ≤ 0.1.6-alpha.1, list ≥ 0.1.6-alpha.2) and why the list-slot hosts skip the takeover.
Review follow-up: on a list-slot host (DSH >= 0.1.6-alpha.2) a second
turnTail entry does not replace the deliverables row, it is rendered next
to it and duplicates the chips. The takeover is not needed there: the
host's openFile funnel goes through sidebarRight.openResource, and this
plugin's editor/files tab types sit in the extension band, which takes
over the builtin kind's address claims, so the default row's chips
already open in the sidebar.
Keep the chain registration (select, priority -1) for older hosts and
swallow exactly the list-slot refusal ('requires options.id') so the
plugin client still activates; any other registration error is
rethrown. JSDoc rewritten for both contracts; spec covers the skip and
the rethrow.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Hengne Li <Hengne.Li@cern.ch>
|
Superseded — |
Problem
On DeepSeek Harness 0.1.6-alpha.2 / 0.1.7-alpha.2 the plugin's client entry fails to activate:
conversation.chat.turnTailbecame a list slot in 0.1.6-alpha.2 (packages/client/ui-chat/src/client/contract/slots.ts); it was a chain slot before.registerTurnTailInterceptionregisters the chain shape (select, noid), soui-slotsthrows at registration and the whole plugin client goes down with it — sidebar tabs included.Why not simply switch to a list entry
(First revision of this PR did that; Copilot's review was right.) A list host renders every entry, so a sidebar entry would sit next to the default
ui-deliverablesrow and duplicate its chips. Shadowing the deliverables cell (sameid, lowerpriority) would replace the whole tail, including its changed-files card, for turns the sidebar declines.And the takeover is not needed on those hosts: the host's
openFilefunnel goes throughsidebarRight.openResource(fileAddress), and this plugin registers itseditor/filestab types in theextensionband, which pertab-registry.tstakes over the builtin kind's address claims — so the default row's chips already open in the sidebar.Fix
src/client/intercept.tsxselect,priority: -1) for hosts ≤ 0.1.6-alpha.1 — behavior there is unchanged.requires options.id) and return a working no-op disposer, so the plugin client activates on newer hosts; any other registration error is rethrown.selectTurnTail(store); JSDoc rewritten to describe both host contracts.tests/turn-tail-intercept.spec.ts: two new cases — list host → nothing registered, disposer idempotent; unrelated registration error → rethrown. Existing chain-host cases unchanged.Verification
pnpm typecheck,pnpm lint,pnpm test(127 files / 1348 tests) pass.🤖 Generated with Claude Code