Analyzed and merged GitHub PR with changelog updatePublic

Agent

Model

Claude Opus 4.5 (latest)

Started

Jan 25, 6:15 PM

Messages45

Downloads0

Summary

Closed, Completed

Understand
Reviewed linked issue and PR details.
Understand
Understand
Analyzed code changes in PR.
Understand
Understand
Checked for changelog entry.
Understand
Edit
Added changelog entry for fix.
Edit
Cleanup
Merged PR and updated main branch.
Cleanup

You are given one or more GitHub PR URLs: https://github.com/badlogic/pi-mono/pull/947

For each PR URL, do the following in order:

Read the PR page in full. Include description, all comments, all commits, and all changed files.
Identify any linked issues referenced in the PR body, comments, commit messages, or cross links. Read each issue in full, including all comments.
Analyze the PR diff. Read all relevant code files in full with no truncation from the current main branch and compare against the diff. Do not fetch PR file blobs unless a file is missing on main or the diff context is insufficient. Include related code paths that are not in the diff but are required to validate behavior.
Check for a changelog entry in the relevant packages/*/CHANGELOG.md files. Report whether an entry exists. If missing, state that a changelog entry is required before merge and that you will add it if the user decides to merge. Follow the changelog format rules in AGENTS.md. Verify:
- Entry uses correct section (### Breaking Changes, ### Added, ### Fixed, etc.)
- External contributions include PR link and author: Fixed foo ([#123](https://github.com/badlogic/pi-mono/pull/123) by [@user](https://github.com/user))
- Breaking changes are in ### Breaking Changes, not just ### Fixed
Check if packages/coding-agent/README.md, packages/coding-agent/docs/.md, packages/coding-agent/examples/**/.md require modification. This is usually the case when existing features have been changed, or new features have been added.
Provide a structured review with these sections:
- Good: solid choices or improvements
- Bad: concrete issues, regressions, missing tests, or risks
- Ugly: subtle or high impact problems
Add Questions or Assumptions if anything is unclear.
Add Change summary and Tests.

Output format per PR: PR: Changelog:

... Good:
... Bad:
... Ugly:
... Questions or Assumptions:
... Change summary:
... Tests:
...

If no issues are found, say so under Bad and Ugly.

Let me read the linked issue #645 and the relevant code file:

Let me continue reading the rest of the file, specifically around line 1575-1580 where the change is:

Let me check if there's a changelog entry for this specific fix in the Unreleased section and also check the related code more closely:

Now let me check if the README.md or docs need updates for this fix:

Now I have enough context. Let me provide the review:

PR: https://github.com/badlogic/pi-mono/pull/947

Changelog:

Missing. No changelog entry exists for this fix. A changelog entry is required before merge and will be added under ### Fixed if user decides to merge.

Good:

Correct diagnosis of the bug: the PR description clearly explains the timing issue where setEditorComponent() copied onExtensionShortcut from defaultEditor at creation time, capturing undefined because setupExtensionShortcuts() hadn't run yet.
The fix is minimal and elegant: changing from direct property copy to a delegating closure (data: string) => this.defaultEditor.onExtensionShortcut?.(data) ensures the call happens at runtime rather than copy time.
The optional chaining (?.) is appropriate since onExtensionShortcut may still be undefined and should just be a no-op in that case.
The PR body provides excellent traceability back to the regression introduced in commit b846a4bf / issue #645.

Bad:

No automated test for this regression. The behavior involves extensions using both registerShortcut() and setEditorComponent(), which would benefit from a test to prevent future regressions.

Ugly:

None.

Questions or Assumptions:

The same copy-at-creation pattern exists for onEscape, onCtrlD, and onPasteImage on lines 1573-1575. Those are set earlier in the initialization flow (via setupKeyHandlers() and setupEditorSubmitHandler() which run in init() before initExtensions()), so they are already defined when setEditorComponent() is called during session_start. However, if the order ever changes, similar bugs could appear. Worth considering whether those should also be delegated for consistency, though this is out of scope for this PR.

Change summary:

packages/coding-agent/src/modes/interactive/interactive-mode.ts: Line 1578 changed from copying this.defaultEditor.onExtensionShortcut to a closure that delegates to this.defaultEditor.onExtensionShortcut?.(data) at call time. This ensures that when a custom editor component calls onExtensionShortcut, it correctly invokes the handler that was set up later by setupExtensionShortcuts().

Tests:

No tests added. Manual testing required: create an extension that uses both registerShortcut() and setEditorComponent() in the session_start handler, then verify the shortcut fires correctly.

merge via gh cli, pull from orogin

Now let me add the changelog entry for this fix:

Done. PR #947 merged, pulled to main, and changelog entry added:

Fixed extension shortcuts not firing when extension also uses setEditorComponent() (#947 by @Perlence)