fix: use current branch upstream for update checks instead of default branch - #201
Conversation
Add optional HTTPS support controlled by two env vars: HERMES_WEBUI_TLS_CERT=/path/to/cert.pem HERMES_WEBUI_TLS_KEY=/path/to/key.pem - Wraps server socket with ssl.SSLContext (min TLSv1.2) - Dynamic scheme detection for startup messages (http:// vs https://) - Graceful fallback to HTTP if cert loading fails — server never crashes due to bad TLS config, just prints a warning and continues - Auth cookie Secure flag already set when HTTPS is detected via getpeercert - 6 end-to-end tests: config flags, HTTPS handshake, HTTP still works, fallback on bad paths Addresses nesquena#191 (HTTPS support issue).
…branch
The update checker in api/updates.py always compared HEAD against
origin/master (or origin/main), which produced false 'N updates
available' alerts when the user is on a feature branch and master has
moved forward with unrelated commits.
Now uses git rev-parse --abbrev-ref @{upstream} to get the current
branch's tracking branch for both the behind-count check and the
apply-update pull command. Falls back to the default branch if no
upstream is set (brand-new local branch with no tracking config).
Fixes nesquena#200.
|
Confirmed valid and the fix is correct. What the bug is
Fix assessmentUsing
One edge case to consider: if the upstream ref no longer exists on the remote (e.g., a remote feature branch that was deleted after merge), The manual verification (74 → 0 updates) against a real checkout is the most convincing test. Fixes #200. Ready for maintainer review. |
Full Review: PR #201 — branch-aware update checkerThanks @iRonin! Valid bug, correct fix. Security AuditClean. Changes are limited to git command arguments in Code ReviewThe fix is correct — Applied consistently to both One note on the pull command: The new code does Stacked Branch IssueThis PR includes the full TLS support from PR #199 (config.py changes, server.py changes, test_tls_support.py). The diff shows 260 additions but only ~30 are the actual update checker fix. This means:
Test Results512 passed, 0 failed, 41 skipped. No regressions. VerdictThe update checker fix itself is approved. But please rebase this onto master after PR #199 is merged to avoid shipping bundled changes. The actual fix is only ~30 lines in |
|
Rebased onto current master (post #196, #197, #198, #199 merges) and fixed a bug in the Bug fixed in the rebase: The original PR passed Fix: split All 561 tests passing. Ready to merge. |
… branch (nesquena#201) * feat: optional HTTPS/TLS support via cert and key env vars Add optional HTTPS support controlled by two env vars: HERMES_WEBUI_TLS_CERT=/path/to/cert.pem HERMES_WEBUI_TLS_KEY=/path/to/key.pem - Wraps server socket with ssl.SSLContext (min TLSv1.2) - Dynamic scheme detection for startup messages (http:// vs https://) - Graceful fallback to HTTP if cert loading fails — server never crashes due to bad TLS config, just prints a warning and continues - Auth cookie Secure flag already set when HTTPS is detected via getpeercert - 6 end-to-end tests: config flags, HTTPS handshake, HTTP still works, fallback on bad paths Addresses nesquena#191 (HTTPS support issue). * fix: use current branch upstream for update checks, not repo default branch The update checker in api/updates.py always compared HEAD against origin/master (or origin/main), which produced false 'N updates available' alerts when the user is on a feature branch and master has moved forward with unrelated commits. Now uses git rev-parse --abbrev-ref @{upstream} to get the current branch's tracking branch for both the behind-count check and the apply-update pull command. Falls back to the default branch if no upstream is set (brand-new local branch with no tracking config). Fixes nesquena#200.
… branch (nesquena#201) * feat: optional HTTPS/TLS support via cert and key env vars Add optional HTTPS support controlled by two env vars: HERMES_WEBUI_TLS_CERT=/path/to/cert.pem HERMES_WEBUI_TLS_KEY=/path/to/key.pem - Wraps server socket with ssl.SSLContext (min TLSv1.2) - Dynamic scheme detection for startup messages (http:// vs https://) - Graceful fallback to HTTP if cert loading fails — server never crashes due to bad TLS config, just prints a warning and continues - Auth cookie Secure flag already set when HTTPS is detected via getpeercert - 6 end-to-end tests: config flags, HTTPS handshake, HTTP still works, fallback on bad paths Addresses nesquena#191 (HTTPS support issue). * fix: use current branch upstream for update checks, not repo default branch The update checker in api/updates.py always compared HEAD against origin/master (or origin/main), which produced false 'N updates available' alerts when the user is on a feature branch and master has moved forward with unrelated commits. Now uses git rev-parse --abbrev-ref @{upstream} to get the current branch's tracking branch for both the behind-count check and the apply-update pull command. Falls back to the default branch if no upstream is set (brand-new local branch with no tracking config). Fixes nesquena#200.
Problem
When the hermes-agent repo is on a feature branch (e.g.
feat/terminal-image-preview), the update checker comparesHEADagainstorigin/master:This counts all commits merged into master since the feature branch was created, not actual missing updates. Users on feature branches see inflated "74 updates available" alerts even though their branch is fully up-to-date with its own remote.
Fix
Replaces the hardcoded default branch lookup with the current branch's upstream tracking branch:
Applied to both:
_check_repo()— the behind-count for the UI badge_apply_update_inner()— thegit pull --ff-onlyused by the "Apply update" buttonBehavior Matrix
maintrackingorigin/mainorigin/mainorigin/main(same)feat/footrackingorigin/feat/fooorigin/main(wrong)origin/feat/foo(correct)origin/mainorigin/main(fallback, same)Testing
Verified manually against the hermes-agent checkout on a feature branch:
74 updates available(HEAD vs origin/master)0 updates available(HEAD vs origin/feat/terminal-image-preview, fully synced)Fixes #200