Skip to content

Fix the three bugs and six design gaps found in review - #1

Merged
liaowang11 merged 6 commits into
mainfrom
review-fixes
Sep 6, 2026
Merged

liaowang11 merged 6 commits into
mainfrom
review-fixes

Conversation

@liaowang11

Copy link
Copy Markdown
Owner

A review of the package against agent-shell 0.75.2, Codex /side at 2df67054, Claude Code's /btw, and ChatGPT's branch feature. Three bugs and six design gaps, each decided before it was fixed. DESIGN.md section 5 records the reasoning.

Bugs

  • Handback into a busy parent lost the summary. agent-shell-insert refuses while a turn runs and the subscriber wrapper swallowed the error. The stub never refused, so no test caught it.
  • A resumed side conversation was not marked as one. No lighter, no keys, absent from the listing, and it could nest.
  • Kept link records were never removed. agent-shell-side-links-remove had no callers.

Decisions

  1. Busy parent. Never send on the user's behalf and never use the minibuffer, since a conclusion is long. Findings go where the user writes. With agent-shell-prefer-viewport-interaction that is the parent's compose buffer, opened in edit mode so a busy parent still takes it. Otherwise the shell prompt, at once when idle or as soon as the turn ends. The side conversation stays open and marked pending until then.
  2. Resumed side conversations are marked as sides with no parent, the shape an orphan already has.
  3. Kept records are consumed on resume.
  4. The parent's systemPrompt.append is kept ahead of the side text, as Codex keeps its developer instructions. No boundary-version bump: the side text is unchanged.
  5. agent-shell-side-on-dismiss defaults to delete, matching Codex's ephemeral thread.
  6. Viewport is honoured for display and for the handback.

Without a decision

Preconditions run before the question is read. The parent-status echo needs the side buffer on screen. The dismiss prompt no longer names a /side command Emacs does not have. Three checkdoc nits. The test stub's agent-shell-insert now refuses a busy target like the real one, which is what had hidden the first bug.

Verification

make check passes 92 tests, up from 78. Byte-compile is clean against the stubs and against the real agent-shell 0.75.2 stack. checkdoc is clean.

Not verified live. The viewport compose path and the wait-for-idle handback are covered by stub tests that mirror upstream's argument shapes and event ordering, but no real agent has run them.

@liaowang11
liaowang11 merged commit 6d822ad into main Sep 6, 2026
1 check passed
@liaowang11
liaowang11 deleted the review-fixes branch September 6, 2026 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant