-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat(tui): add daemon adapter spike #4202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
5784574
docs(tui): draft daemon adapter plan
c9705bd
feat(tui): add daemon adapter spike
aaf45df
fix(tui): harden daemon adapter event handling
c13b42a
fix(tui): report daemon prompt failures
5f9170f
fix(tui): surface daemon terminal failures
0c79afb
fix(tui): harden daemon adapter state handling
4f9658f
fix(tui): harden daemon adapter lifecycle
de6cb66
fix(tui): harden daemon adapter follow-ups
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| # TUI Daemon Adapter Draft | ||
|
|
||
| ## Goal | ||
|
|
||
| Add a flag-gated TUI transport that talks to `qwen serve` through | ||
| `DaemonSessionClient` instead of creating an in-process `Config` + agent | ||
| runtime. | ||
|
|
||
| This is a dogfood path for Mode B client migration. It must not replace the | ||
| default TUI path until output sinks, typed daemon events, session-scoped | ||
| permission, and lifecycle diagnostics are stable. | ||
|
|
||
| ## Proposed Entry Point | ||
|
|
||
| ```bash | ||
| QWEN_DAEMON_URL=http://127.0.0.1:4170 qwen --experimental-daemon-tui | ||
| ``` | ||
|
|
||
| Optional: | ||
|
|
||
| ```bash | ||
| QWEN_DAEMON_TOKEN=... QWEN_DAEMON_WORKSPACE=/repo qwen --experimental-daemon-tui | ||
| ``` | ||
|
|
||
| The CLI should refuse this mode unless both are true: | ||
|
|
||
| - `QWEN_DAEMON_URL` or `--daemon-url` is set. | ||
| - `GET /capabilities` advertises `session_create`, `session_prompt`, and | ||
| `session_events`. | ||
|
|
||
| ## Minimal Flow | ||
|
|
||
| 1. Create `DaemonClient` with daemon URL and token. | ||
| 2. Fetch `/capabilities`. | ||
| 3. Create or attach with `DaemonSessionClient.createOrAttach()`. | ||
| 4. Subscribe to `session.events()`. | ||
| 5. Submit user prompts through `session.prompt()`. | ||
| 6. Route cancel through `session.cancel()`. | ||
| 7. Route model switch through `session.setModel()`. | ||
| 8. Route permission votes through `session.respondToPermission()`. | ||
|
|
||
| ## Rendering Contract | ||
|
|
||
| The first implementation adds `DaemonTuiAdapter`, a locally verifiable reducer | ||
| and transport spike. It maps only these daemon events: | ||
|
|
||
| | Daemon event | TUI handling | | ||
| | ---------------------------------------- | -------------------------------------------- | | ||
| | `session_update` / `agent_message_chunk` | Append assistant text | | ||
| | `session_update` / `agent_thought_chunk` | Append thinking text | | ||
| | `session_update` / `tool_call` | Show tool call lifecycle | | ||
| | `permission_request` | Show existing confirmation UI where possible | | ||
| | `permission_resolved` | Close or update confirmation UI | | ||
| | `model_switched` | Update footer/model display | | ||
| | `session_died` | Show disconnected state and stop streaming | | ||
|
|
||
| Unknown events must be ignored, not fatal. Typed event reducers will land in a | ||
| later protocol PR. | ||
|
|
||
| The adapter is not wired into the default Ink app yet. Existing interactive TUI, | ||
| JSONL, stream-json, and dual-output behavior remains unchanged. | ||
|
|
||
| ## Explicit Non-Goals | ||
|
|
||
| - Do not remove the current TUI in-process runtime. | ||
| - Do not change JSONL, stream-json, or dual-output behavior in this PR. | ||
| - Do not expose file CRUD, MCP management, memory CRUD, or provider/auth | ||
| mutation through TUI yet. | ||
| - Do not make browser/web direct-to-daemon assumptions; this is terminal only. | ||
|
|
||
| ## Merge Safety | ||
|
|
||
| - Default off. | ||
| - Additive code path. | ||
| - No existing CLI flags change behavior. | ||
| - If the daemon is unavailable, the experimental path fails before starting the | ||
| TUI and tells the user to run `qwen serve`. | ||
|
|
||
| ## Validation Plan | ||
|
|
||
| - Unit-test event-to-TUI-state mapping with synthetic daemon events. | ||
| - Unit-test prompt, cancel, model switch, and permission vote forwarding. | ||
| - Unit-test flag/env parsing when the feature flag is wired. | ||
| - Smoke-test against a local `qwen serve`: | ||
| - prompt text streams into the TUI | ||
| - cancel resolves the active prompt | ||
| - permission request can be accepted or rejected | ||
| - reconnect sends the tracked `Last-Event-ID` | ||
|
|
||
| ## Blockers Before Default Migration | ||
|
|
||
| - Typed daemon event schema. | ||
| - Session-scoped permission route. | ||
| - Output sink refactor for JSONL / stream-json / dual-output parity. | ||
| - Session lifecycle close/delete semantics. | ||
| - Runtime diagnostics for MCP, skills, providers, and workspace env. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Low — capability 校验列表与 Minimal Flow 不对齐
这里只要求
session_create/session_prompt/session_events,但 Minimal Flow 第 6-8 步用了session.cancel()/setModel()/respondToPermission()。如果 daemon 不广告这些 capability,这些路径会在运行时静默崩。
要么补进必需 capability,要么在 doc 里说明它们可选(并解释为什么)。