feat(shell): welcome banner redesign + TUI polish, update orchestrator, release-orchestration plans#37
Merged
Merged
Conversation
Approved design for finishing the GitHub App/OIDC migration, a self-owned release.py single-source-of-truth tool, fail-loud promotion, and broadened distribution (Scoop/GHCR/Nix/WinGet). Phased P0->P2 roadmap. Produced via a multi-agent design workflow, ground-truthed against the live repos.
Write-stage plans recovered from the plan-writing workflow, each with its automated review punch-list embedded. Finalize pass (applying the punch-lists) pending after a session-limit interruption.
Applied the automated review punch-lists (finalize pass): P0 fixes the invalid gh api --jq, the $GITHUB_ENV bottleneck plumbing, semver-ordered readiness-issue auto-close, and the changelog-gate verification; P1 wires the PYTHINKER_MANAGED hook end-to-end; P2 fixes the Scoop extract_dir/bin nesting and the Docker ancestor-check dry-run. Removed draft banners and embedded punch-lists.
… docs Broad checkpoint of the welcome-banner-redesign branch: - Shell/TUI: welcome banner footer-chip layout, design system, glyphs, motion, prompt, slash, tool renderers, MCP status, activity tree, live view, worklog, theme tokens. - Update flow: new update_orchestrator module + CLI wiring; terminal capability detection (terminal_capabilities) with tests. - Feedback: structured feedback repo (feedback_repo) and slash command. - Auth/config/llm: OpenAI auth + platform/config/llm adjustments. - Docs: welcome-banner spec/plan and P0–P2 release-orchestration plans. - Web: subagent-steps component update. - Address CodeRabbit review: -> None on update CLI handlers, drop a duplicate test assertion, correct the backoff verification budget (7 retries / 364s), and de-hardcode uv paths in the P2 plan.
Adjust update_orchestrator, mcp_status, feedback_repo, and live_view alongside their tests; add update CLI test coverage.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds account-scoped ChatGPT model discovery and branded login callback; implements a file-backed async update orchestrator with CLI status/log and slash integration; centralizes terminal capability/glyph/motion handling and plain-theme behavior; updates many TUI renderers (shared markers, hooks, subagent rollups, welcome-panel redesign); improves feedback repo normalization and secret redaction; and adds extensive tests, docs, and CI workflow tweaks. ChangesComplete PR checkpoint
Sequence DiagramsequenceDiagram
participant CLI
participant UpdateOrchestrator
participant Updater as do_update/_run_upgrade_command
participant Filesystem
CLI->>UpdateOrchestrator: run_update_job(print_output, check_only, source)
UpdateOrchestrator->>Updater: invoke do_update with output_callback
Updater->>Filesystem: spawn subprocess, stream stdout/stderr
Filesystem-->>Updater: exit code + streamed output
Updater-->>UpdateOrchestrator: UpdateResult + log lines
UpdateOrchestrator->>Filesystem: write status, append redacted log, release lock
UpdateOrchestrator-->>CLI: return UpdateResult
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-05-31-release-orchestration-p0-quick-wins.md`:
- Around line 22-24: The GH secret creation commands for
PYTHINKER_RELEASE_BOT_APP_ID and PYTHINKER_RELEASE_BOT_APP_PRIVATE_KEY use
--visibility all which exposes the secrets to the entire org; update the gh
secret set invocations that reference PYTHINKER_RELEASE_BOT_APP_ID and
PYTHINKER_RELEASE_BOT_APP_PRIVATE_KEY to use --visibility selected and add
--repos pythinker-code (or the minimal required repos) so only the intended
repo(s) can access the app ID and private key.
In `@docs/superpowers/plans/2026-05-31-release-orchestration-p1-release-tool.md`:
- Around line 159-160: Replace all raw "python ..." command examples in the plan
with the project-managed launcher "uv run python ..." (or a corresponding make
target if present); specifically change the shown sanity-check command "python
scripts/check_pythinker_dependency_versions.py --root-pyproject pyproject.toml
--pythinker-core-pyproject packages/pythinker-core/pyproject.toml
--pythinker-host-pyproject packages/pythinker-host/pyproject.toml
--pythinker-review-pyproject packages/pythinker-review/pyproject.toml" to "uv
run python scripts/check_pythinker_dependency_versions.py ..." and update the
other raw invocations referenced in the comment (around the same file at the
occurrences noted) to use "uv run python ..." or "uv run --directory <package>
python ..." / make targets where appropriate so all Python examples follow the
project guideline.
In
`@docs/superpowers/plans/2026-05-31-release-orchestration-p2-distribution-channels.md`:
- Line 194: The tag filters using the literal regex string
"v[0-9]+.[0-9]+.[0-9]+" must be replaced with a glob-safe pattern so GitHub
Actions will match tags; update every workflow that uses that exact string
(e.g., in windows-installer.yml, linux-installer.yml, release-pythinker-cli.yml,
homebrew-tap.yml, promote-release.yml) to use a glob like "v*.*.*" or
"v[0-9]*.[0-9]*.[0-9]*" instead, and leave the workflow's existing "Resolve
version" validation logic intact so it still enforces strict version formatting;
also update the docs entry that lists the old pattern to reflect the new
glob-safe example.
In `@docs/superpowers/plans/2026-05-31-welcome-banner-redesign.md`:
- Line 99: The markdown file contains fenced code blocks without surrounding
blank lines (notably the triple-backtick markers ``` at the shown locations);
add a blank line immediately before and after each fenced code block (i.e.,
insert an empty line before the opening ``` and after the closing ```) for the
occurrences mentioned (including the ``` markers at the listed locations) to
satisfy markdownlint style.
- Around line 136-206: The plan's _print_welcome_info body diverges from the
real implementation; update the doc to reflect the shipped responsive layout and
helpers by restoring the width calculations and conditional layouts: mention the
use of _welcome_panel_width, _WELCOME_MAX_WIDTH and _WELCOME_PANEL_CHROME_WIDTH
for responsive sizing, and reference the truncation/helpers
_truncate_middle_to_width, _welcome_value, and _welcome_tip_lines used to
compute cell-aware text wrapping; note that the actual code builds tables with
Table.grid(...) and has a width >= 68 branch that places the logo beside the
text (vs stacked) and that columns set explicit width and no_wrap flags (so
include column width/no_wrap behavior in the plan). Ensure the plan either
mirrors this implementation exactly or is marked superseded.
In `@docs/superpowers/specs/2026-05-31-release-orchestration-design.md`:
- Line 287: Add blank lines before and after the subsection heading "P0 — Quick
wins (low risk, high value; fixes F2 + the F1 sub-package race + the real 0.24.0
class)" so the heading is separated from surrounding content; apply the same
change to the other nearby subsection headings mentioned (the headings at the
other two spots referenced) so each Markdown heading has an empty line above and
below for proper rendering and consistency.
- Around line 319-328: The spec currently embeds absolute filesystem paths (e.g.
"/home/ai/Projects/pythinker-code-main/...") — update every referenced path to
repository-relative paths (e.g. "pyproject.toml",
"scripts/check_pythinker_dependency_versions.py",
".github/workflows/promote-release.yml",
".github/workflows/dispatch-pythinker-home-sync.yml",
".github/workflows/changelog-entry-required.yml",
".github/workflows/release-pythinker-core.yml", "flake.nix",
"scripts/install.ps1", "site/scripts/sync-upstream-products.ts") so the document
uses repo-root relative references; scan the entire file for any other absolute
path literals and replace them consistently, preserving the file names and any
parent subpaths while removing the leading machine-specific directory.
In `@docs/superpowers/specs/2026-05-31-welcome-banner-redesign-design.md`:
- Line 28: Update the fenced code blocks containing terminal art mockups in the
welcome-banner-redesign spec by adding the language specifier "text" to each
opening fence (change ``` to ```text) so markdownlint MD040 is satisfied; also
apply the same change to the matching "50-50" mockup sections mentioned in the
document to ensure consistency across the spec.
In `@README.md`:
- Around line 804-807: The README's sentence "This README does not collect
user-level telemetry" is imprecise; update the text in README.md (the centered
paragraph block) to a clearer phrasing such as "Viewing this README does not
trigger telemetry" or "This documentation is telemetry-free" so it accurately
states that reading the file doesn't send telemetry; replace the existing
sentence in the <p align="center"> block with one of these alternatives.
In `@src/pythinker_code/ui/shell/mcp_status.py`:
- Line 16: Replace hardcoded bullet glyphs in mcp_status.py with the ASCII-aware
constants from glyphs.py: remove the _MCP_STARTUP_GLYPH = "•" and any literal
"•" or " • " prefixes used in renderers and instead import and use the
appropriate glyph constant (e.g., TRANSCRIPT_ACTIVE_MARKER) from
src/pythinker_code/ui/shell/glyphs.py; update usages in functions/methods that
render startup/loading lines so they reference TRANSCRIPT_ACTIVE_MARKER (or
another named glyph from glyphs.py if more appropriate) so ASCII fallback
behavior via ascii_glyphs_enabled() is preserved.
In `@src/pythinker_code/ui/shell/slash.py`:
- Around line 567-577: The code assumes provider.api_key is non-null and calls
provider.api_key.get_secret_value(), which will raise if api_key is None; add a
null guard to return None (or otherwise handle missing key) before calling
get_secret_value and before passing it into soul.runtime.oauth.resolve_api_key;
in the block using provider (symbols: provider, provider.api_key,
provider.custom_headers, provider.oauth, soul.runtime.oauth.resolve_api_key)
check if provider.api_key is None and bail out (return None) or handle that case
so no AttributeError occurs, then proceed to extract the secret and apply the
Authorization header and final authorization presence check.
In `@src/pythinker_code/ui/shell/update_orchestrator.py`:
- Around line 377-384: The code currently calls _write_last_success regardless
of the smoke check result which is misleading; change the logic in the
UpdateResult.UPDATED branch so that after run_post_install_smoke_check() and
append_update_log(smoke_message) you only call
_write_last_success(job_id=job_id, message=message) when smoke_ok is True, and
skip (or alternatively write a differently named marker) when smoke_ok is False;
update the block around UpdateResult.UPDATED to conditionally call
_write_last_success based on the smoke_ok boolean (referencing
_write_last_success, run_post_install_smoke_check, append_update_log, and
UpdateResult.UPDATED).
In `@src/pythinker_code/ui/shell/visualize/_blocks.py`:
- Around line 576-578: Replace the explicit loop that builds paths from
result.display with a list comprehension: iterate over getattr(result,
"display", []) or [], filter by isinstance(block, DiffDisplayBlock), and collect
block.path into the paths list (i.e., assign to variable paths rather than
appending). This change touches the block handling code that currently
references result, the DiffDisplayBlock type, and the paths list.
In `@tests/cli/test_update_cli.py`:
- Line 11: The helper function _isolate_update_files is missing a return type
annotation; open its body, determine the actual return value (e.g., a
pathlib.Path, a tuple like Tuple[pathlib.Path, ...], or a callable) and add an
explicit return type annotation (for example -> pathlib.Path or ->
Tuple[pathlib.Path, str] as appropriate) to the function signature, importing
any typing names (Tuple, Optional, Callable) and pathlib if needed so the
annotation is valid; ensure the annotation matches the concrete value returned
by _isolate_update_files.
In `@tests/ui_and_conv/test_update_orchestrator.py`:
- Line 14: The helper function _isolate_update_files is missing a return type
annotation; update its signature to include an explicit return type that matches
what it actually returns (for example, -> tuple[pathlib.Path, pathlib.Path] or
-> pathlib.Path as appropriate), and add any needed imports (typing.Tuple or
pathlib.Path) so the annotation is valid; ensure you modify the def
_isolate_update_files(monkeypatch, tmp_path): line to include the chosen return
type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e50f77c5-aed8-4e41-96e5-5ef887ade838
📒 Files selected for processing (62)
.gitignorePRODUCT.mdREADME.mddocs/superpowers/plans/2026-05-31-release-orchestration-p0-quick-wins.mddocs/superpowers/plans/2026-05-31-release-orchestration-p1-release-tool.mddocs/superpowers/plans/2026-05-31-release-orchestration-p2-distribution-channels.mddocs/superpowers/plans/2026-05-31-welcome-banner-redesign.mddocs/superpowers/specs/2026-05-31-release-orchestration-design.mddocs/superpowers/specs/2026-05-31-welcome-banner-redesign-design.mdsrc/pythinker_code/app.pysrc/pythinker_code/auth/openai.pysrc/pythinker_code/auth/platforms.pysrc/pythinker_code/cli/update.pysrc/pythinker_code/config.pysrc/pythinker_code/feedback.pysrc/pythinker_code/feedback_repo.pysrc/pythinker_code/llm.pysrc/pythinker_code/ui/shell/__init__.pysrc/pythinker_code/ui/shell/components/bash_execution.pysrc/pythinker_code/ui/shell/components/bordered_loader.pysrc/pythinker_code/ui/shell/components/tool_execution.pysrc/pythinker_code/ui/shell/design_system.pysrc/pythinker_code/ui/shell/glyphs.pysrc/pythinker_code/ui/shell/mcp_status.pysrc/pythinker_code/ui/shell/motion.pysrc/pythinker_code/ui/shell/prompt.pysrc/pythinker_code/ui/shell/slash.pysrc/pythinker_code/ui/shell/tool_renderers/_render_utils.pysrc/pythinker_code/ui/shell/tool_renderers/agent.pysrc/pythinker_code/ui/shell/update.pysrc/pythinker_code/ui/shell/update_orchestrator.pysrc/pythinker_code/ui/shell/visualize/_activity_tree.pysrc/pythinker_code/ui/shell/visualize/_blocks.pysrc/pythinker_code/ui/shell/visualize/_btw_panel.pysrc/pythinker_code/ui/shell/visualize/_live_view.pysrc/pythinker_code/ui/shell/visualize/_worklog.pysrc/pythinker_code/ui/terminal_capabilities.pysrc/pythinker_code/ui/theme.pytests/auth/test_openai_auth.pytests/cli/test_update_cli.pytests/core/test_config.pytests/core/test_openai_provider.pytests/core/test_shell_mcp_status.pytests/ui_and_conv/test_activity_tree.pytests/ui_and_conv/test_live_view_notifications.pytests/ui_and_conv/test_native_update_parity.pytests/ui_and_conv/test_prompt_tips.pytests/ui_and_conv/test_render_matrix.pytests/ui_and_conv/test_shell_design_system.pytests/ui_and_conv/test_shell_feedback_slash.pytests/ui_and_conv/test_shell_motion.pytests/ui_and_conv/test_shell_update.pytests/ui_and_conv/test_shell_welcome_info.pytests/ui_and_conv/test_terminal_capabilities.pytests/ui_and_conv/test_tool_call_block.pytests/ui_and_conv/test_tui_blocks_integration.pytests/ui_and_conv/test_tui_card_tool_renderers.pytests/ui_and_conv/test_tui_components.pytests/ui_and_conv/test_update_orchestrator.pytests/ui_and_conv/test_visualize_running_prompt.pytests/ui_and_conv/test_worklog_render.pyweb/src/components/ai-elements/subagent-steps.tsx
CI runs the suite with TERM=dumb and NO_COLOR=1 to keep logs clean. The new
terminal_capabilities module correctly degrades the TUI to ASCII glyphs and
no-color under those, but ~115 pre-existing UI-contract tests assert the rich
(Unicode + color + motion) appearance and only ever ran in UTF-8 dev shells.
- tests/conftest.py: pin a capable terminal (drop NO_COLOR/CLICOLOR/glyph/motion
overrides, set TERM=xterm-256color) before any pythinker import freezes the
glyph constants in ui.shell.glyphs. The degraded fallbacks stay covered by
tests/ui_and_conv/test_terminal_capabilities.py. Verified: full tests/ run
green under NO_COLOR=1 TERM=dumb PYTHONUTF8=1.
- docs p0-quick-wins: wrap inline-code GitHub Actions ${{ ... }} in <code v-pre>
so VitePress's Vue SSR stops evaluating them as template interpolation.
Fixes the docs build crash on PYTHINKER_HOME_REPO_DISPATCH_TOKEN. Verified
with a local build over the committed doc set.
- CHANGELOG.md (+ synced docs mirror): add Unreleased entries for the welcome
banner redesign and terminal-aware rendering, satisfying the changelog gate.
- slash.py: guard provider.api_key (SecretStr | None) before get_secret_value() to avoid AttributeError when no API key is configured. - mcp_status.py + glyphs.py: add an ASCII-aware LIST_BULLET glyph and use it for the MCP startup/inventory bullets instead of a hardcoded "•", so they degrade with the rest of the TUI under TERM=dumb / legacy code pages. - _blocks.py: build changed-paths via a list comprehension (PERF401). - test_update_cli.py, test_update_orchestrator.py: add -> None on the _isolate_update_files helpers. - p0-quick-wins plan: scope the release-bot App secrets to --visibility selected --repos pythinker-code instead of org-wide.
… fence langs, plan note)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-05-31-welcome-banner-redesign.md`:
- Around line 128-136: Update the post-implementation note to point readers to
the real implementation location and the key symbols to inspect: mention the
module that contains _print_welcome_info and list the helpers and constants to
review (_welcome_panel_width, _WELCOME_MAX_WIDTH, _WELCOME_PANEL_CHROME_WIDTH,
_truncate_middle_to_width, _welcome_value, _welcome_tip_lines) as well as the
Table.grid branch and width>=68 layout branch; instruct readers to open that
module and search for _print_welcome_info to see the final responsive/truncation
logic and logo layout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7292dd86-dc4b-4a9c-9781-682c434c2ccd
📒 Files selected for processing (4)
README.mddocs/superpowers/plans/2026-05-31-welcome-banner-redesign.mddocs/superpowers/specs/2026-05-31-release-orchestration-design.mddocs/superpowers/specs/2026-05-31-welcome-banner-redesign-design.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/homebrew-tap.yml:
- Line 6: Replace the loose tag glob "v[0-9]*.[0-9]*.[0-9]*" in the workflow
trigger with a stricter semver-only pattern so the job only runs on tags that
are exactly vMAJOR.MINOR.PATCH (no extra characters or pre/postfixes), and leave
the existing semver validation in the release script as defense-in-depth; locate
the tag pattern string in the homebrew tap workflow and update it to a regex
that matches only v<digits>.<digits>.<digits>.
In `@docs/superpowers/plans/2026-05-31-release-orchestration-p1-release-tool.md`:
- Around line 143-150: The fenced YAML block under the "Update the CI caller"
checklist item is missing blank lines before and after the triple-fence,
triggering MD031; edit the markdown so there is an empty line immediately before
the opening ``` and an empty line immediately after the closing ``` for that
YAML snippet (and do the same for the other fenced block noted around lines
152-154) to satisfy the blanks-around-fences rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ef9752ab-b6c7-42ec-98d9-410ecebd901d
📒 Files selected for processing (18)
.github/workflows/homebrew-tap.yml.github/workflows/linux-installer.yml.github/workflows/promote-release.yml.github/workflows/release-pythinker-cli.yml.github/workflows/windows-installer.ymldocs/superpowers/plans/2026-05-31-release-orchestration-p1-release-tool.mddocs/superpowers/plans/2026-05-31-release-orchestration-p2-distribution-channels.mddocs/superpowers/plans/2026-05-31-welcome-banner-redesign.mddocs/superpowers/specs/2026-05-31-release-orchestration-design.mdsrc/pythinker_code/ui/shell/mcp_status.pysrc/pythinker_code/ui/shell/slash.pysrc/pythinker_code/ui/shell/update_orchestrator.pytests/cli/test_review_wrapper.pytests/cli/test_secscan_wrapper.pytests/core/test_export_cli.pytests/core/test_shell_mcp_status.pytests/e2e/test_cli_error_output.pytests/ui_and_conv/test_update_orchestrator.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A broad feature branch covering shell/TUI work plus release-orchestration planning docs.
│separator, and the border/title switch to theme tokens for light-mode polish. Logo glyphs and palette unchanged.update_orchestratormodule + CLI wiring; terminal-capability detection (terminal_capabilities).feedback_repo) and slash command.subagent-stepscomponent update.Tests
test_update_orchestrator,test_terminal_capabilities,test_update_cli, banner redesign tests (footer chip / no pipe / split strapline).Notes
Addressed CodeRabbit review on the branch:
-> Noneon the update CLI handlers, removed a duplicate test assertion, corrected the backoff verification budget (7 retries / 364s) in the P0 plan, and de-hardcodeduvpaths in the P2 plan.Summary by CodeRabbit
New Features
update status,update log.Bug Fixes
Documentation
UI/Accessibility