v1.5.1: account-wide repo watching + ETA labels - #10
Merged
Conversation
New watch_account_repos config key (default false, operator opt-in locally): when true, the effective poll list becomes auto-discovered account repos UNION the explicit repos list MINUS repos_exclude, computed fresh each poll via resolve_repo_list (ci_status/logic.py) -- a pure function so account/explicit/exclude interaction is unit-testable without network mocking. Explicit repos are never subject to the active_within_days recency filter, since the operator named them on purpose. Discovery: RestPoller.fetch_account_repos() (ci_status/github.py) hits GET /user/repos?affiliation=owner&sort=pushed&per_page=100, paginating up to a 1000-repo cap. Only page 1 gets a conditional-request (ETag) slot -- deliberate, not an oversight: sort=pushed means page 1's ETag alone is the cheap common-case win (a single-page account gets a free 304 whenever nothing relevant changed), while re-fetching pages 2+ fresh on every re-enumeration (itself only every repo_refresh_minutes, not every poll) avoids a correctness trap where a stale cached page 2+ result could miss a pushed_at update to a repo that moved within page 2 without crossing into page 1. Returns None (not []) on a 304 or any failure, so the caller can distinguish "nothing changed / fetch failed, keep the previous list" from "this account genuinely owns zero repos." run_once gained a repo_cache parameter (same caller-owned-dict pattern as running_cache/overlay_state/quota_cache; omitted by default, which preserves the exact pre-v1.5.1 behavior of polling cfg["repos"] verbatim). _refresh_account_repos re-enumerates only when the cache is older than repo_refresh_minutes, keeping the previous list and logging a warning on a genuine enumeration failure -- never crashes, never falls back to an empty watch list. Repos that drop off the resolved list (excluded, aged out, deleted upstream) have their state_cache/ running_cache entries pruned and their poller-side ETag slots forgotten (RestPoller.forget_repo) so a stale alert or running badge can't linger, and a later-readded repo starts with a clean conditional-request slate. Two related fixes needed for this to work end-to-end: main()'s "no repos configured" check previously rejected watch_account_repos=true with an empty repos list outright; now it only errors when both are unset. next_poll_seconds previously iterated cfg_ci["repos"] to detect an active run, which would have silently missed any auto-discovered repo (never in that list) and failed to shorten the poll interval for it -- now checks running_cache.values() directly. Tests: resolve_repo_list (union/exclude/active-window filtering including the exact boundary instant, explicit repos never filtered, archived exclusion, malformed pushed_at handled defensively, dedup + deterministic sort), fetch_account_repos (pagination, page-1-only ETag, 304 vs failure vs genuinely-empty-account, partial-pagination-failure keeps earlier pages), forget_repo, _refresh_account_repos timing (stale re-fetches, fresh doesn't, failure keeps previous list, no-prior-list logs a warning), state_cache/running_cache pruning for dropped repos, next_poll_seconds seeing auto-discovered repos, config defaults. TZ=UTC uv run pytest -q: 209 passed (174 before this feature). Live check: --once --dry-run against a local throwaway config (never touched the operator's real config.toml) with watch_account_repos=true, against the real GitHub API. Discovered 14 owned repos; 9 within the 30-day active window (5 public, 4 private -- private repo names redacted from this report as <private-N>, per the brief, since reports may be quoted in public PRs; the physical display and local config are unaffected by this redaction, per the README's private-repo-name caveat). The dry-run payload reflected real live state (a genuine CI failure on one of the discovered repos), confirming discovery, resolution, and the existing failure-detection path compose correctly end to end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Next to the ETA numeral, appends a small muted label when it fits:
"remain" preferred, "left" as a fallback, omitted entirely if neither
fits (_eta_label, ci_status/logic.py). Grammar guard is a single prefix
check -- eta_text.startswith("~") -- reliable because every
remaining-estimate form _format_eta_text produces ("~4m", "~1h05m",
"~10h", ...) is "~"-prefixed by construction, and neither "soon" nor the
no-history elapsed form ("3m in") ever is; a label on either of those
would read as nonsense or be outright wrong (elapsed time, not a
remaining estimate).
Fit is decided by reusing calendar_countdown.logic's large-font
GLYPH_ADVANCE_PX/_text_width_px machinery, extended with nine new
glyphs ("~" plus the letters needed to measure "remain"/"left":
r, a, e, i, n, l, f, t -- "m" already existed), measured on-device via
the same successive-prefix rightmost-ink-column differencing technique
already documented on that table, self-validated by re-measuring "0"
through it and reproducing the existing 7px value with zero adjustment.
Deliberate modeling choice, documented in _eta_label's docstring: the
label itself renders in the *small* font, but its width is estimated
via the *large*-font table anyway, because on-device calibration found
that table always overestimates a small-font string's width here
("remain" measures 22px small-font vs. 35px via the large-font
estimate; "left" 11px vs. 20px) -- safe for a fits/doesn't-fit decision
(worst case: an omitted label that would have visually fit, never a
clipped one), and avoids needing a second glyph table.
Geometry: RUNNING_LABEL_Y=9 puts the label's ink at rows 11-15,
baseline-aligned with the large ETA numeral's own ink (rows 7-15) via
the calendar's established "+2px ink-offset" small-font model --
confirmed on-device. RUNNING_LABEL_COLOR="#8FA3B3FF", a muted gray-blue
extending the existing title/numeral brightness hierarchy one step
further.
Adding the label element changes the running badge's own element-id
shape between polls (as the ETA's width crosses a fit boundary over the
run's lifetime) -- no main.py changes needed for this: the unified
shape tracker from the prior revision (a generic frozenset-of-ids
comparison, not a hardcoded per-frame mapping) already detects any
shape change on any draw and clears first.
Tests: fit-decision boundaries at the exact measured widths ("~59m"
fits remain, "~1h00m" only fits left, a synthetic "~23h59m" -- wider
than any real ETA text can reach today -- fits neither, included to
exercise that branch defensively), both exclusion forms, the label
element's x-position, and both running-badge shape variants.
test_glyph_advance_table_covers_every_countdown_glyph relaxed from
exact-set equality to a subset check now that the table carries more
than _format_countdown alone needs.
TZ=UTC uv run pytest -q: 217 passed (209 before this feature).
On-device: both fit outcomes ("~57m remain", "~1h01m left") captured
through the ink-overlap + buffer gate -- title/eta/label ink rows all
within their expected ranges, no ink in the row-5 buffer for any of the
three text elements, no label ink in columns 0-1 or at the right edge
(no clip). Drawn to `preview` at priority 25 rather than the payload's
own priority 21: the live ci_status LaunchAgent was genuinely active
during this session (real runs on a private repo) and draws at
21 too, so a same-priority preview draw was observed being rejected
outright (equal priority, different application_name) until raised,
matching the precedent set by the original v1.5 badge-variant
verification script.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ate name
Critical: redacted a private repo name from
the spec doc's v1.5.1 ETA-label section, where it named which repo's
live activity collided with the on-device verification's own draws.
Swept the full branch diff for any other private-account repo names
(private set per the review: anything outside {agent-collab,
busybar-integrations, YouTube-AutoEncoder, Portable-GHAR,
VSC-RemoteSSH}) -- this was the only instance found in tracked
docs/tests. Note: the same phrase also appears verbatim in commit
19e7635's own message (not a tracked file); fixing that would require
amending an already-created commit, which this session's git-safety
constraints don't do without explicit instruction even though the
branch is unpushed -- flagging in the status reply rather than silently
amending.
Important -- untested validation fix: extracted main()'s "no repos
configured" check into config_requires_repos(cfg) -> str | None, a pure
function of cfg testable without exercising main()'s argparse/logging/
gh-auth/device-connection side effects. Covers: empty repos + account
mode off -> error; empty + account mode on -> ok; nonempty + mode off
-> ok; the v1.5.1 key absent entirely (old-style cfg) -> behaves like
mode off, no crash.
Important -- page-1 ETag lock-in: RestPoller.fetch_account_repos was
committing the page-1 ETag right after page 1's own response, before
pagination even started. A transient page-2+ failure would still return
the partial result collected so far (by design -- a partial result
beats discarding everything), but the ETag was already cached; the next
call would then send that ETag, get a 304 (page 1 itself genuinely
hadn't changed), and return None -- which main._refresh_account_repos
reads as "nothing changed, keep the cached list," permanently freezing
an over-100-repo account's watch list at that one incomplete run's
subset, with no future call able to recover the rest. Fixed: the ETag
is now committed only after a full pagination run succeeds; on an
incomplete run it's simply left as whatever it was (not overwritten),
which is still correct -- reaching that branch means page 1's response
this call was a fresh 200, not a 304, so the next call sending the same
old ETag will get another fresh 200, not a wrongful 304, giving
pagination another full chance rather than reusing the partial result.
TZ=UTC uv run pytest -q: 222 passed (217 before this fix). New tests:
4 for config_requires_repos, 1 regression test reproducing the ETag
lock-in end-to-end (partial-failure call keeps a 100-repo partial list
and does NOT commit the ETag; second call sends no If-None-Match and
recovers the full 102-repo list; third call, now that a full run has
succeeded, correctly commits the ETag and short-circuits via 304).
No on-device work needed for these fixes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff4ebd5498
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+274
to
+276
| if complete and new_etag is not None: | ||
| self._account_repos_etag = new_etag | ||
| return repos |
There was a problem hiding this comment.
When an account owns more than 100 repositories and page 1 succeeds but a later page times out, returns a non-200 response, or contains malformed data, this returns a non-None partial list. _refresh_account_repos consequently treats the truncated result as a successful snapshot and refreshes its timestamp, after which run_once prunes the omitted repositories from both state caches; those repositories stop being polled and existing failure alerts disappear until the next successful refresh. Return None whenever pagination is incomplete so the caller retains its previous complete list.
Useful? React with 👍 / 👎.
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.
Two operator-requested refinements:
watch_account_reposauto-discovers owned repos (paginated, activity-windowed, hourly refresh — new repos join automatically), withrepos_excludeopt-outs; dropped repos are fully pruned from all caches. Discovery ETag commits only after complete pagination (prevents a truncation lock-in for >100-repo accounts). Also fixes two latent bugs: config validation now accepts account-mode-with-empty-list, and the fast-poll cadence now triggers for discovered repos' active runs.~4m remain(falls back toleft, omits when neither fits — per-glyph width decision with 9 newly device-measured glyphs), guarded to remaining-estimate forms only.Tests: 222 (was 174).
🤖 Generated with Claude Code