fix: brew tap-trust recovery, antenna boot blink, export secret redaction - #120
Conversation
…tion Three independent fixes to the updater, welcome banner, and session export. Homebrew untrusted-tap recovery: - Homebrew 5.0's HOMEBREW_REQUIRE_TAP_TRUST refuses third-party taps until `brew trust <tap>` is run once, so the in-app updater's `brew upgrade` failed with only a generic "run manually" hint. Parse both the hard "Refusing to load ... from untrusted tap" refusal and the soft "Skipping ... not trusted" warning; on an interactive terminal, offer to run `brew trust` and retry the upgrade once, otherwise print the exact remediation. Also handles the silent-no-op case where an untrusted tap is skipped during `brew update`. Documented in both READMEs. Antenna boot animation: - Replace the welcome logo's infinite SGR slow-blink with a bounded boot animation: blink the antenna ball 7 times after the banner prints, then pin it steady. Gated to real terminals tall enough to keep the antenna row on screen; skipped under reduced motion and non-TTY. E2E PTY helper runs with reduced motion so the animation adds no latency there. Export secret redaction: - A tool result (grep/cat over a .env) could surface a secret value into the exported transcript in plaintext. Add a conservative redact_secrets() that replaces the value of secret-named keys (password/token/api_key/secret/...) with [REDACTED], wired as a chokepoint in both the markdown and YAML export builders. Non-secret keys (token_count, usernames, ports) are untouched.
|
Warning Review limit reached
More reviews will be available in 42 minutes and 55 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces three independent user-facing features: Homebrew untrusted-tap detection with interactive recovery during in-app updates; secret-value redaction from exported session transcripts; and a fixed-count antenna-blink boot animation replacing indefinite SGR blinking on interactive terminals. ChangesHomebrew Untrusted Tap Handling
Secret Redaction in Exports
Welcome Antenna Boot Animation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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 `@tests/utils/test_sensitive.py`:
- Around line 45-47: Add a new unit test to cover the benign-keys edge case so
redact_secrets does not redact keys where the secret hint is followed by
additional characters (e.g., token_count or access_key_id). Create a test
function (e.g., test_redact_ignores_keys_with_suffix_after_hint) in the same
test module that passes a string like "token_count=100\naccess_key_id=AKIA..."
to redact_secrets and asserts the output equals the input; ensure the test name
and assertion reference redact_secrets so the intent and behavior are documented
and protected.
🪄 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: b7d11837-e07a-4b43-ab75-3aadc5055966
📒 Files selected for processing (10)
CHANGELOG.mdREADME.mdsrc/pythinker_code/ui/shell/__init__.pysrc/pythinker_code/ui/shell/update.pysrc/pythinker_code/utils/export.pysrc/pythinker_code/utils/sensitive.pytests/e2e/shell_pty_helpers.pytests/ui_and_conv/test_shell_update.pytests/ui_and_conv/test_shell_welcome_info.pytests/utils/test_sensitive.py
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Keys where the secret hint is not immediately before the separator (token_count, access_key_id) must not be redacted. Lock that intentional behavior in per CodeRabbit's suggestion on #120.
GitHub code-quality flagged implicit string concatenation of adjacent literals inside the list arguments to _homebrew_untrusted_tap (a common missing-comma footgun). Use explicit '+' so the single-line intent is unambiguous to readers and the scanner.
Three independent, self-contained fixes. Each ships with tests;
make check-pythinker-codeis green.1. Homebrew untrusted-tap recovery
Homebrew 5.0 added
HOMEBREW_REQUIRE_TAP_TRUST, which refuses to load formulas from third-party taps untilbrew trust <tap>is run once. The in-app updater'sbrew upgrade pythinker-codetherefore failed for every Homebrew ≥5 user on first update, printing only a generic "try running manually" hint._homebrew_untrusted_tap()parses both the hardRefusing to load … from untrusted tap <tap>refusal and the softSkipping <tap> because it is not trustedwarning (the latter only counts when it names our tap).brew trust <tap>, then refresh and retry the upgrade once. Non-interactively (or on decline): print the exactbrew trust pythoughts-labs/pythinkerremediation.brew updateskips the untrusted tap andbrew upgradeexits 0 without advancing the version.2. Antenna boot animation
Replaces the welcome logo's infinite SGR slow-blink with a bounded boot animation: the antenna ball blinks 7 times after the banner prints, then pins steady. Gated to real terminals tall enough to keep the antenna row on screen; skipped under reduced motion and non-TTY. The e2e PTY helper now runs with
PYTHINKER_REDUCED_MOTION=1so the animation adds no latency/cursor-noise there.3. Export secret redaction
A tool result (e.g.
grep/catover a.env) could surface a secret value into the exported transcript in plaintext (observed: anADMIN_PASSWORD=…landing in a.mdexport). Adds a conservativeredact_secrets()that replaces the value of secret-named keys (password/token/api_key/secret/…) with[REDACTED], wired as a single chokepoint in both the markdown and YAML export builders. Non-secret keys (token_count, usernames, ports) are left untouched.Tests
tests/ui_and_conv/test_shell_update.py— 9 new untrusted-tap cases (parse, prompt+retry, decline, trust-failure, silent-no-op).tests/ui_and_conv/test_shell_welcome_info.py— 7-blink contract, motion/TTY/short-terminal gating.tests/utils/test_sensitive.py— redaction unit cases incl. the exact leaked patterns.tests/e2e/shell_pty_helpers.py— reduced-motion env.Not included (follow-up)
Mid-message inline
/commandhandling and the related agent-behavior issues (whole-file rewrite, weak verification,EnterPlanModetool-availability confusion) are deliberately left out — they belong in the test-pinned system-prompt surface and warrant a focused, agent-mediated change rather than a mechanical parser (which testing against a real GPT-5.5 prompt showed would discard the user's task).Summary by CodeRabbit
New Features
brew trust[REDACTED]Documentation