Support streaming tool output and deduplication - #343
Conversation
timvisher-dd
changed the title
|
I appreciate the contribution, but please be mindful there's a lot being sent my way here, from long issue descriptions to chunky PRs. When I ask folks to file a feature request before sending a PR, I don't mean it to be just a checkbox item so folks send the PR soon after that. From CONTRIBUTING.org
There's a span of minutes between the feature request/issue and the PRs submitted for review. According to commit logs, I see this feature was all built today. Being a larger contribution, please use it for some time before sending it my way. Let it settle for some time. Use it. It takes a lot of concentration for me, energy, and time to parse of all this. I am maintaining this package, so I need to understand the contributions sent my way and that takes a significant effort. Please be mindful. |
36ba031 to
5496a0d
Compare
So my intent was to be respectful of your wish to use an Issue to discuss the feature and the PR is opened in Draft to indicate that you shouldn't review it yet unless you wish. It's meant to be a both/and offering rather than to indicate that you should feel the need to read both. If you'd prefer I can keep the draft changeset entirely in my local dev integration branches until you tell me explicitly that I may open a PR. But from my perspective the 'this code might answer this issue if we both agree that the issue is real and make sense' is a useful thing especially when I'm mostly opening the issue because I've fixed it for myself.
The commit logs show that just because I rebase and push every time I work to be sure that I'm not in conflict with anything happening on trunk. I assure you that I'm not opening Issues or PRs without using the changes I'm making for at least a few days.
I wonder if there's some way to set a 'max in flight Issues/PRs' setting on the repo? To me I'm opening issues as I see and solve them for myself and I have zero expectation that you'll spend any more time than is reasonable for you on it. If that means my Issues/PRs sit unwatched for months then so be it. That's the nature of open source. Glob knows my own open source projects often will go that long with zero attention paid by me. That said I'm unsure how better to indicate to you that something's available for you to look at. Like if we had a discord somewhere I'd still be sending you a message 'Hey this thing maybe could use your attention' and then you'd have to decide whether you have any to spare at that time or not. Again it's just kind of the nature of an open source community. I guess finally if I'm pushing too much into your attention we could go where I just work on my fork and integrate your work and you can check my fork every now and then to see if anything interesting has landed. That's a reasonable stance for you to take as well. LMK how you'd like to proceed! Really appreciate all the effort you've put into this so far. :) |
16a7ad0 to
ae7e165
Compare
f46d7c1 to
8fd241e
Compare
CI workflow runs byte-compilation and ERT tests on push/PR using GitHub Actions with deps checked out from timvisher-dd/acp.el-plus and xenodium/shell-maker. bin/test parses ci.yml with yq so local runs stay in sync with CI automatically. It symlinks local dependency checkouts into deps/ to match the CI layout. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
New library for sending desktop notifications from Emacs. In GUI mode on macOS, uses native UNUserNotificationCenter via a dynamic module (agent-shell-alert-mac.dylib) compiled JIT on first use (inspired by vterm). When compilation fails (e.g. missing Xcode CLI tools), a message recommends `xcode-select --install`. In terminal mode, auto-detects the host terminal emulator and sends the appropriate OSC escape sequence: - OSC 9: iTerm2, Ghostty, WezTerm, foot, mintty, ConEmu - OSC 99: kitty - OSC 777: urxvt, VTE-based terminals Inside tmux, wraps in DCS passthrough (checking allow-passthrough first). Falls back to osascript on macOS when the terminal is unknown or tmux passthrough is not enabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Follows the same pattern as acp.el: a boolean toggle (agent-shell-logging-enabled, off by default), a per-shell log buffer stored in state, and label+format-string logging. Adds log calls to idle notification start/cancel/fire for observability. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After each agent turn completes, a 30s timer starts. Any user input in the buffer cancels it; otherwise it fires a desktop notification via agent-shell-alert. The echo area message is only shown when the shell buffer is not the active buffer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Validate that agent-shell-buffers and agent-shell-project-buffers reflect (buffer-list) ordering correctly: switch-to-buffer and select-window promote, with-current-buffer does not, bury-buffer demotes, and project filtering preserves order. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fails PRs that modify .el files or tests/ without also updating README.org, ensuring the soft-fork features list stays current. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add comprehensive ERT tests for agent-shell-usage.el covering notification updates, context indicator scaling/colors, compaction replay, token saving, and number formatting. The ACP server has a bug where model switches cause used to exceed size in session/update notifications. Rather than clamping, signal unreliable data: indicator shows ? with warning face, format shows (?) instead of a bogus percentage. A regression test replays real observed traffic from the Opus 1M -> Sonnet 200k switch scenario. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mirror the CI readme-updated job locally so bin/test catches missing README.org updates before pushing. Also fix the copy-paste error where both dep-missing messages said shell_maker_root. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Document the two key requirements for contributing: run bin/test and keep the README features list current. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
668cbfe to
5dea5d0
Compare
f16181f to
083af54
Compare
773099b to
ee1e040
Compare
31c4d9c to
c7ee08b
Compare
c7ee08b to
53d1260
Compare
Resolves a conflict in agent-shell.el where:
- Our fork added an idle-notification feature
(agent-shell-idle-notification-delay,
agent-shell--idle-notification-{cancel,fire,start,subscribe}).
- Upstream added an idle-timer feature
(agent-shell-idle-timeout, agent-shell--{start,cancel}-idle-timer).
Both implementations are preserved — they have distinct names and
different responsibilities, so they can coexist. Also preserves the
fork's (require 'agent-shell-alert) alongside upstream's new
(require 'agent-shell-kimi).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The README-update check used `-C "${root}" diff "${base}" HEAD`,
which the git wrapper rejects: it sees the relative `bin/..`
path passed to `-C` and the raw `${base}` SHA as suspicious
ref expressions.
Drop `-C` (the script already runs inside the worktree) and
switch to the canonical `@{u}...` three-dot range. This matches
the @{u} pattern documented in the git skill and fires only on
committed changes — uncommitted edits don't block running tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Upstream's agent-shell--{start,cancel}-idle-timer assume two
preconditions that don't hold in many existing test setups:
- agent-shell--state is callable (it errors with "Processed
outside shell" when not in agent-shell-mode).
- The state alist already contains :idle-timer (otherwise
map-put! signals map-not-inplace).
Both are guaranteed by agent-shell--make-state in production but
not by tests that hand-build smaller state alists or run inside
fundamental-mode temp buffers.
Wrap each access in (ignore-errors (agent-shell--state)) and
guard map-put! with map-contains-key. When state is unavailable
or :idle-timer absent, the functions become a no-op rather than
erroring — production behaviour is unchanged because
agent-shell--make-state always provides both.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Upstream made agent-shell--state a function with a strict
derived-mode-p check, and agent-shell--send-command now calls
into agent-shell--{cancel-idle-timer,emit-event} which both
invoke that function.
The send-command-integration and send-command-error-fallback
tests run in fundamental-mode temp buffers and only let-bind
the variable form of agent-shell--state. Override the function
form via cl-letf so it returns the test's let-bound alist.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The test calls (make-frame '((visibility . nil))) to drive an interactive restart flow, which fails with "Unknown terminal type" under emacs --batch. Guard with skip-unless so the suite stays green in non-interactive runs (CI, dev_ci hook). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…26-07+0000 Restore green CI after upstream merge: idle-timer guards + test fixes
2ee10cd to
1913d2e
Compare
Resolves conflict in agent-shell.el by keeping both fork's idle notification subscription and upstream's session title refresh subscriptions (init-finished, turn-complete). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2e3462d to
e45d0f5
Compare
e45d0f5 to
0676213
Compare
0676213 to
5a28f60
Compare
GitHub Actions workflow with four jobs: readme-updated (PR-only,
guards the soft-fork features list), agent-symlinks (verifies the
multi-IDE plumbing), dependency-dag (require graph must be acyclic),
and test (byte-compile + ERT under emacs 29.4 with acp.el and
shell-maker as checkout deps).
bin/test parses ci.yml with yq and dispatches each step locally, so
CI changes are picked up automatically. adapt_for_local rewrites
GitHub PR sha context to a single @{u}... three-dot range that the
git wrapper accepts. CONTRIBUTING.org documents the runner and the
acp_root / shell_maker_root overrides.
.claude / .codex / .gemini and CODEX.md are symlinks pointing at
.agents and AGENTS.md so the same config works across Claude Code,
Codex, and Gemini CLI. .agents/commands/live-validate.md describes
the live rendering-validation workflow.
README.org gets a "Features on top of agent-shell" section
enumerating the streaming-dedup work and follow-on polish.
agent-shell-devcontainer.el declares agent-shell-text-file-capabilities
to suppress a byte-compile warning for the cross-file reference.
Three send-command tests in tests/agent-shell-tests.el get :title
and :last-activity-time pre-seeded on their hand-rolled state alists
so the local runner produces a clean baseline. Without the
placeholders, agent-shell--set-session-title's map-put! call fails
with map-not-inplace because the alists lack the keys.
Quote the keymap argument to shell-maker-define-major-mode. Under
shell-maker 0.91.2 the macro expects a symbol it can resolve at mode
activation; passing the unquoted variable evaluates to the keymap
value before the macro can use it, and agent-shell-mode signals
(void-function keymap) when any test creates a fresh buffer.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three tests cover the markdown table overlay pipeline end-to-end: overlay structure for a static buffer, mid-stream cleanup so stale overlays disappear when a row is rewritten, and a regression that guards against table rows being split across visual lines. The helpers inject ACP traffic via agent-shell--on-notification and fire pending debounce timers when present, so the tests reflect the real streaming path rather than direct markdown-overlays-put calls. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lpers Three new modules and the agent-shell.el integration that wires them together: - agent-shell-meta.el extracts toolResponse and terminal_output from the ACP meta envelope so streaming code can fold mixed-source tool output (terminal stream + final meta.toolResponse) without showing duplicate content. - agent-shell-invariants.el is a runtime tracing and assertion library: a ring of recent ACP/UI events, process-mark and fragment-update guard wrappers, and a long-buffer head/tail snapshot included in violation reports. - agent-shell-streaming.el is the streaming tool_call_update handler with dedup, including a label cache cleared on completion and the generalized title upgrade path that survives buffer-kill races. agent-shell.el wires these in (require, on-notification dispatch, process-mark/fragment guards, insert-cursor reset, defcustom for the markdown-overlay debounce delay, and dropping session/update handlers when the shell buffer has been killed). agent-shell-ui.el gains the invariants require and the UI plumbing the streaming handler relies on. Tests cover the dedup logic across mixed sources, the invariants library's event ring and guard wrappers, and additional regression coverage in agent-shell-tests.el (cancel with nil transcript-file, markdown-overlay debounce buffer-kill race, "Thinking" label restoration on agent_thought_chunk). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ebb0848 to
25a3b75
Compare
3 participants
Fixes #342
See that issue for the full problem description, root cause analysis, and perf measurements.
Depends on xenodium/acp.el#15 (adds
:terminal-capabilityand:meta-capabilitiestoacp-make-initialize-request).Checklist
M-x checkdocandM-x byte-compile-file.Implementation
New files
agent-shell-meta.el— extractors for ACP_metapayloads:agent-shell--meta-lookup— key lookup handling both symbol and string keys in alists.agent-shell--meta-find-tool-response— walks any_metanamespace to find atoolResponsevalue (used by claude-agent-acp).agent-shell--tool-call-meta-response-text— extracts stdout text from_meta.*.toolResponsein its various shapes (string, alist withstdoutkey, vector of content blocks).agent-shell--tool-call-terminal-output-data— extracts_meta.terminal_output.data(used by codex-acp for incremental streaming chunks).agent-shell-streaming.el— streaming tool call update handler:agent-shell--tool-call-normalize-output— strips markdown fences, strips<persisted-output>XML tags (rendering the preview withfont-lock-comment-face), and ensures trailing newlines.agent-shell--append-tool-call-output— accumulates streamed output in the state's:tool-callshash under an:accumulatedkey per tool call ID.agent-shell--handle-tool-call-update-streaming— the main handler, replacing the inlinetool_call_updateblock inagent-shell.el. Three branches:_meta.terminal_output.data): normalize the chunk, accumulate it, and immediately append it to the fragment body for live streaming._meta.*.toolResponse): normalize and accumulate silently (rendered only on final update to avoid duplication)."completed"or"failed"): render accumulated output (or fall back tocontenttext), log to transcript, clean up permission dialogs, and apply title/label updates.agent-shell--mark-tool-calls-cancelled— marks all in-progress tool calls as cancelled (called fromagent-shell-interrupt).Changes to
agent-shell.el(require 'agent-shell-streaming)added.tool_call_updaterendering block (lines 1290-1346 on main) is replaced by a single call toagent-shell--handle-tool-call-update-streaming. The metadata save (title/description/command/raw-input/diff) remains inline before the handler call.initializerequest now passes:terminal-capability tand:meta-capabilities '((terminal_output . t))toacp-make-initialize-request.agent-shell-interruptcallsagent-shell--mark-tool-calls-cancelledafter sending the cancel notification.shell-maker-define-major-modecall passes'agent-shell-mode-map(quoted symbol) instead of the bare variable.Tests
7 new tests in
tests/agent-shell-streaming-tests.el:agent-shell--tool-call-meta-response-text-test— extracts text from_meta.claudeCode.toolResponse.stdout.agent-shell--tool-call-normalize-output-test— strips fences and ensures trailing newline.agent-shell--tool-call-normalize-output-persisted-output-test— strips<persisted-output>tags.agent-shell--tool-call-update-writes-output-test— verifies accumulated output is written to the fragment body.agent-shell--tool-call-meta-response-no-duplication-test— meta response text is rendered once, not duplicated with content.agent-shell-initialize-request-meta-capabilities-test— the initialize request includes_meta.terminal_output.agent-shell--tool-call-terminal-output-data-streaming-test— codex-style_meta.terminal_output.datachunks are accumulated and rendered incrementally.