Skip to content

fix(desktop): offer composer Resume after a manual Stop, drop the turn-footer resume button - #5927

Merged
me2seeks merged 1 commit into
apache:mainfrom
me2seeks:feat/5923-resume-after-manual-stop
Oct 3, 2026
Merged

me2seeks merged 1 commit into
apache:mainfrom
me2seeks:feat/5923-resume-after-manual-stop

Conversation

@me2seeks

@me2seeks me2seeks commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Summary

#5904's review round added a suppression: after the user stops a Turn from the composer (Stop button or Escape), the send slot never offered Resume for that Turn — resume was relegated to a small ghost button in the stopped Turn's footer (a path with no test or story coverage).

This restores the original #5903 behavior: after a manual Stop, with an empty draft, the composer send slot offers Resume (play glyph) exactly as for any other resumable interruption — and removes the turn-footer resume button, so the slot is the single visible affordance.

Why the suppression can go: the hazard it guarded against — a repeated or late click on Stop restarting the Turn the user just stopped — is narrow now. The slot renders Resume only after a fresh ready answer from the planner (a re-read retracts the previous offer while in flight), and ready requires the stop's terminal transition to have completed, so a click during the settle window lands on a disabled Send. A deliberate click on a visible Resume is the feature working, and a resumed Turn can simply be stopped again.

The tracker keeps the review-round wins that are unconditional: stale-offer retraction while a re-read is in flight, refresh on Host rebound, latest-wins answer ordering, and fail-closed parked/error mapping.

Empty draft after a manual Stop — Resume owns the send slot:

Resume owns the send slot while the draft is empty

Typing anything switches the same slot back to Send:

Typing restores the Send button

The click in flight through the safe-boundary admission:

Resume pending state

Fixes #5923

Verification

  • @maka/ui build + test:dist: 699/699 (the one dropped test pinned the removed footer button).
  • Desktop build:test + four-config typecheck: pass. resume-availability tracker suite 6/6 (suppression cases removed; retraction, latest-wins, settle, burst coalescing and per-session keying stay).
  • Full @maka/desktop test:dist: 3133 pass, 5 fail — the 5 reproduce identically on a clean origin/main checkout on this machine (git-environment and skills-IPC fixtures, unrelated to this PR).
  • check:app-shell-hooks, renderer-architecture --strict-base (base c7fa6bb), biome lint: pass.
  • Storybook: the three resume stories render and their play functions assert the slot swap (screenshots above are those stories with the composer form given the conversation column's width, as the app shell lays it out).
  • Not run: the Electron app end-to-end against a live interrupted session.

AI use

  • Generative tooling made a substantive contribution

Tool(s) and scope: Maka (k3-256k) — issue analysis, implementation, tests, and this PR description under human direction. Generated-by trailer on the commit.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above

…n-footer resume button

The review-round suppression on apache#5904 hid the send-slot Resume offer for a
Turn the user just stopped and relegated resume to a small ghost button in
the stopped Turn's footer. That kills the primary use case: stopping a Turn
mid-reply and then deciding to continue it is exactly when the user looks at
the send slot, and the footer button was never covered by a test or story.

Restore the original apache#5903 behavior: after a manual Stop, with an empty
draft, the composer send slot offers Resume like any other resumable
interruption. The accidental-restart hazard the suppression guarded against
is narrow now: the slot only renders Resume after a fresh ready answer from
the planner, which requires the stop's terminal transition to have
completed, so a click during the settle window lands on a disabled Send; a
resumed Turn can simply be stopped again.

The tracker keeps the unconditional wins from the review rounds: stale-offer
retraction while a re-read is in flight, and refresh on Host rebound.

Fixes apache#5923

Generated-by: Maka (k3-256k)
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Oct 2, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 474ab5c202833a02a3ef7e4129359c8d76723375 (7 files, +21/−282, a single commit).

No P0–P3 findings.

What the change does. It reverses the earlier fix for this exact surface: the "suppress the first offer after a user stop" machinery is deleted — noteUserStopped, the suppression set, the stopOwningItsTarget wrapper in app-shell.tsx, and the tests for them (−131 lines) — and a Turn the user just stopped is once again offered like any other resumable interruption. The stated reasoning is recorded in the contract's own comment, citing the issue that drove it: the slot only renders Resume after a fresh ready answer, i.e. after the stop's terminal transition has completed, so a click during the settle window lands on a disabled Send, and a resumed Turn can simply be stopped again.

I checked what that safety argument rests on, because a reversal like this is only sound if the remaining guards are real:

  • The retract-on-request guard is retained and still does the work the argument needs: a fresh read retracts an already-visible offer, so nothing clickable survives while the authority is being re-asked (resume-availability.ts:52-89). That is the piece that makes "Resume only appears after a fresh ready" true rather than hopeful.
  • The removal is complete: nothing in the tree still references the deleted suppression (noteUserStopped, offerSuppressedAfterUserStop return no hits), and no document still describes the old behaviour.
  • The trimmed test file still covers everything that remains: ready versus parked, fail-closed on a rejected read, a stale answer landing after a click, burst coalescing, per-session keying, and "retracts a visible offer while a fresh read is in flight". So the −131 lines removed tests for behaviour that no longer exists, not coverage of behaviour that does.

One residual I want on the record rather than re-graded. The claim is precise about the settle window, and it holds there. It does not eliminate the narrower case: a late repeated click, arriving after Resume has appeared, still resumes the Turn the user just stopped. That is the deliberate trade this change makes — the cost is one more Stop, weighed against not being able to resume a stop you meant — and it is a policy call, not a defect, so I am noting it rather than objecting to it.

Gate on this head: test and label are green; mergeable is true and the state is blocked, i.e. review pending.

What I could not judge

  • No Electron run: the click-timing behaviour described above is taken from the send-policy gating and the tracker, plus the tests, not from clicking through it.
  • I did not run the workspace suites myself.

I did not approve, request changes, or merge.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[kabi-sol]

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

No P0–P2 findings on 474ab5c202833a02a3ef7e4129359c8d76723375 in the reviewed scope. I support the explicit #5923 tradeoff: offer continuation in the composer after manual Stop and remove the redundant stopped-turn footer action. This deliberately accepts a second click after Resume becomes visible; fresh-plan gating protects the pending-query interval, not every accidental second click. The old suppression assertions therefore describe the superseded design rather than a regression criterion.

Validated with freshly built production useShellResume, Composer, and TurnView mounted against controlled services: Stop callback; disabled Send while the fresh query is pending; Resume after a ready result for the stopped candidate; duplicate/rebound refresh retracting the previous offer; click target and pending state; cross-session late results; quote/streaming precedence; parked/error outcomes; removal of the manual-stop footer action while the status row and failed-turn banner action remain. The Host's active-execution admission guard was source-inspected, not exercised end to end.

89 focused tests, 121 architecture fixtures plus the architecture gate, Desktop dependency/main builds, and all four Desktop typechecks passed. CI test is successful on this head. I did not run Electron, a real Host stop/restart, Storybook, or the complete repository suite.

One description correction: the removed footer did have a test—the deleted offers Safe resume in the Desktop Stop outcome notice test rendered and clicked it. Its removal follows the product decision, but “no test coverage” is inaccurate.

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[kabi-sol]

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

Approving at the human requester’s explicit direction after rechecking 474ab5c202833a02a3ef7e4129359c8d76723375. The head is unchanged from my completed review, CI test is successful, and there are no new unresolved review threads or blocking findings. The earlier review’s evidence and limits remain unchanged.

The synthetic merge with current main (ab5996bdb) is conflict-free. I inspected the intervening UI changes; the resume tracker, hook, composer, and Host admission guard are unchanged, and the footer removal remains intact. I did not rerun the full suite on that synthetic merge or perform a real Electron/Host stop-resume test. The accepted possibility of a second click after Resume is visible remains the explicit #5923 product tradeoff.

@me2seeks
me2seeks merged commit 8ad836c into apache:main Oct 3, 2026
2 checks passed
chihumyum added a commit to chihumyum/maka that referenced this pull request Oct 3, 2026
apache#5927 removed the composer's stopped-Turn note so the send slot offers
Resume after a manual Stop. The Stop path now lives in the Composer
submission owner, so the owner drops its noteUserStoppedTurn shell
port, and the Composer's Stop button, Escape and question prompt share
one stop action. The owner test now checks that Stop removes the rows
the Host retracts.

The renderer architecture ledger starts from main's copy, gets the
hand-maintained controllerOwners, private modules and retired ownership
entries back, and is regenerated.

Generated-by: Claude Opus 5.5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(desktop): offer composer Resume after a manual Stop, drop the turn-footer resume button

3 participants