Reviewed and finalized GitHub PR #1039Public

Let me read the linked issue #500 and the current source file:

Let me read more of the file to understand the renderInitialMessages method:

Now let me check the changelog:

Let me check the current setExtensionHeader implementation in the main branch:

Let me check the docs to see if there's anything related to custom headers:

Now I have enough context to provide a complete review.

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

Changelog:

• No changelog entry exists. An entry is required under ### Fixed in packages/coding-agent/CHANGELOG.md before merge, e.g.:
  - Fixed custom header not displaying correctly with `quietStartup` enabled ([#1039](https://github.com/badlogic/pi-mono/pull/1039) by [@tudoroancea](https://github.com/tudoroancea))

Good:

• The root cause is correctly identified: with quietStartup enabled, builtInHeader was created but never added to the UI tree, so setExtensionHeader() couldn't find/replace it via ui.removeChild()
• The headerContainer approach is clean and isolates header management from the overall UI child ordering
• headerContainer.children[index] replacement is more robust than ui.children.splice(1, 0, ...) which depended on magic indices
• The quiet startup branch now correctly adds this.headerContainer.addChild(this.builtInHeader); ensuring the header is always in the container

Bad:

• Missing pkg:coding-agent label on the PR
• The renderInitialMessages() relocation from run() to init() (before ui.start()) is bundled in without clear justification. The comment says "to avoid layout jump" but this is unrelated to the header container fix
• The PR mixes two concerns: (1) fixing header container management and (2) changing initialization order

Ugly:

• The renderInitialMessages() move could have subtle side effects:
  • run() also calls renderInitialMessages() in multiple other places (extension commands like /fork, /tree, /new etc.)
  • If there are any implicit ordering dependencies with subscribeToAgent() or initExtensions() that expect messages not to be rendered yet, this could break
  • Extensions calling setHeader during session_start may now see different state depending on whether messages were already rendered

Questions or Assumptions:

1. Is the renderInitialMessages() move actually necessary for the header fix, or was it discovered during debugging? The PR description doesn't mention it.
2. Has this been tested with:
   • Resumed sessions (which have existing messages)?
   • New sessions with initial messages via -m flag?
   • Extensions that read/modify chat state during session_start?

Change summary:

1. Adds headerContainer: Container field to hold all header-related components
2. Adds headerContainer as first child of ui in init()
3. Changes header building to add components to headerContainer instead of ui directly
4. Fixes quiet startup branch to add builtInHeader to headerContainer (the actual bug fix)
5. Refactors setExtensionHeader to manipulate headerContainer.children by index instead of splicing into ui.children
6. Unrelated: Moves renderInitialMessages() from run() to init() (before ui.start())

Tests:

• No automated tests added
• Manual testing required with quietStartup: true + custom header extension

Recommendation: The headerContainer fix is correct. The renderInitialMessages() move should be either:

1. Removed from this PR and submitted separately with proper justification, or
2. Explained in the PR description with testing evidence

If you want to merge only the header fix without the renderInitialMessages change, the relevant lines to revert are the addition at lines 469-470 and the removal at lines 526-527 in the diff.

The PR is merged but no changelog entry was added. Let me add one:

Done. Merged PR #1039 via squash, added changelog entry, pushed, and left a comment thanking the contributor.