fix(config): show live Herdr sidebar size for uncommitted worker changes - #21
Merged
Merged
Conversation
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.
Intent
"why does status never update live and only after a stage/commit?"
"why does the stats not appear under the [worker] spawns???"
Context for reading those asks: the Herdr Agents sidebar shows a size line under each worker (
+<added> · −<deleted> · ✎ <files>), fed by the omp extensionconfig/herdr-sidebar.ts(installed as~/.omp/agent/extensions/code-factory-herdr-sidebar.ts). Two workers were running when the second ask came in, one with commits and one with only uncommitted edits, and neither had a size line under it. The current code:git diff --shortstat <base>...HEAD, so only committed work counts and a staged or edited file changes nothingif (!pr && !stat.files) stat = {})session_startandagent_end, and a worker usually stays inside one long turn for most of its task, so the line freezes until that turn endsWhat Changed
config/herdr-sidebar.ts: the size line now comes fromgit diff --shortstat --merge-base <base>, so it counts committed, staged and unstaged tracked changes. A newdiffStatadds untracked, non-ignored files, counting lines the way git does (one for a symlink, none for a binary file). It triesorigin/mainand thenorigin/masterwhen the base ref is missing. It no longer clears the size for a worker with no pull request and no commit yet. Instead, an empty diff or no base ref gives no size. The diff runs withdiff.autorefreshindex=falseso it doesn't rewrite the index.config/herdr-sidebar.ts: a 10-second timer started onsession_start(cleared onsession_shutdown) refreshes the size during a long turn. These ticks skip the PR and issue lookup and report only when the size differs from the last one reported successfully, so a failed report is retried on the next tick.run()also gets a 64 MiBmaxBuffer.docs/herdr.mddocuments the new size source and refresh cadence.tests/test_herdr_sidebar.pyis reworked and adds cases for uncommitted, staged and untracked work, theorigin/mainfallback, no base ref, an empty diff, the index staying unchanged, size updates without a turn end, and retry after a failed report.Risk Assessment
git diffthat can contend for the worker's index.lock, an unbounded untracked-file scan on a 10 s loop, a missed retry after a failed report, and an unrequested base-ref fallback.Testing
The 69 existing sidebar tests pass. I then drove every scenario under real omp 18.4.2 in an isolated real Herdr server and compared each size with git's own count. This covered uncommitted, staged, untracked and committed work updating live in one idle session, and clearing on an empty diff. It also covered the index-untouched guard, retry after a failed report, git-equivalent untracked counting, the base-ref fallback and the home-pane no-size guard. The base-commit extension showed no size line in the same setup. The sidebar evidence is a text dump of the real Herdr TUI rendered from a pty (pyte). I captured no PNG screenshot, so there is no image of the sidebar. The dump shows the size row under
└ work. Not exercised: a real model turn ending (agent_end), because the isolated omp has no model credentials. The 10-second timer andsession_startpaths are what the change relies on and were driven.+2 · −1 · ✎ 2appears under└ workin the Herdr Agents sidebar within ~2s, no turn run.git/indexbyte-identical, with noindex.lock, and the worker's owngit addsucceedswho, never a size, under real omp/exitin real omp clears the tokens and removes the sidebar entry, and no omp process remainsEvidence: Real omp + real Herdr TUI: edit/stage/commit/reset sequence with sidebar dump after each step
Evidence: Full Herdr TUI screen (150x40 pty): Agents sidebar with the size line under the worker, beside the real omp session
Evidence: Real omp: index untouched, failed-report retry, untracked parity, home-pane guard
Evidence: Real omp: commit-only worker at session start; base-ref fallback and no-base cases
Evidence: Real omp: base-commit extension shows no size line (before the change)
Evidence: Real omp: /exit clears tokens and removes the sidebar entry
~/.no-mistakes/evidence/01M3TDR33V67QD2E73F3588E0V/pytest-worker-tests.txt)~/.no-mistakes/evidence/01M3TDR33V67QD2E73F3588E0V/scenario-A-live-updates.txt)Evidence: Agents sidebar rendered by the real Herdr TUI with real omp, worker with uncommitted edits only
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
config/herdr-sidebar.ts:86- The size now comes fromgit diff --shortstat --merge-base <ref>, which compares against the working tree. It runs every 10 s inside every worker's checkout. The old<base>...HEADform never touched the index or working tree. I checked this in a scratch repo: aftertouch fon a tracked file, thisgit diffrewrites.git/index, so it must take.git/index.lock. It does so whenever tracked files have stale stat info, such as after a touch, checkout, rebase or stash, or just after agit add(racy timestamps). A worker's owngit add,git commitorgit stashthat lands in that window fails withfatal: Unable to create '.git/index.lock': File exists. The background timer, not the worker, causes that failure.git --no-optional-locksandGIT_OPTIONAL_LOCKS=0do not prevent it fordiff; I verified both still rewrite the index.-c diff.autorefreshindex=falsedoes prevent it. Fix: run the diff asgit -c diff.autorefreshindex=false diff --shortstat --merge-base <ref>.ls-files --othersandrev-parsedo not write the index, so line 86 is the only site.config/herdr-sidebar.ts:234-statis assigned at line 232 beforereport()runs. The timer tick then decides whether to report by comparingJSON.stringify(stat)withbefore. Sequence: a long turn, the size changes, the 10 s tick setsstatand callsreport(), andreport()throws (Herdr restarting or timing out). The outercatchswallows the error. At the next tickbeforealready equalsstat, so nothing is reported. The sidebar keeps the old size until the size changes again or the turn ends, which is the 'does not update live' symptom this change is meant to fix. Fix: keep areportedsnapshot that is set only afterawait report()succeeds, and comparestatagainst it rather than against the pre-refresh value. The turn-end and lookup paths report every time, so they are unaffected.config/herdr-sidebar.ts:91- The new untracked-file counting runs every 10 s per worker instead of once per turn end, and three things go wrong with it. (1)readFileloads each whole file, including large binaries, and only afterwards checks the first 8000 bytes for a NUL. A worker with a few hundred MB of unignored output (screenshots, logs, downloads) therefore re-reads all of it every cycle, thentoString('latin1').split('\n')allocates a string and array the size of each text file (line 93). Theponytail:comment on line 88 names this ceiling, but the timer multiplies its cost. (2)readFilefollows symlinks. git counts an untracked symlink as 1 file with 1 line, which I checked withgit add -Nand--shortstat. The code counts the target's line count instead. A symlink to a directory hits EISDIR and counts 0 lines. A symlink to a huge file or a special file reads the target in full. Anode_modules/ignore pattern does not match a symlink, so this case is reachable. (3)runuses execFile's default 1 MiBmaxBuffer. A large untracked listing (a tree with roughly 17k or more unignored files) makesls-filesreject. The.catch(() => stat)at line 232 then silently freezes the old size indefinitely. Behavior-preserving fix:lstatfirst and count a symlink as 1 line; open each file and read only the first 8000 bytes for the binary check, so binaries are never fully read; raisemaxBufferor stream thels-filesoutput. A hard cap on bytes or files would change the displayed counts, so that is the author's decision. Thedocs/herdr.mdsentence 'counted as a new file (its lines, or none for a binary file)' and the comment on line 87 claim git parity that symlinks break.config/herdr-sidebar.ts:79- Component not required by the intent: theorigin/main, thenorigin/masterbase fallback in thefor (const r of [base, "origin/main", "origin/master"])loop. The intent names three defects: committed-only counting, clearing the size when there is no PR or commit, and refreshing only onsession_startandagent_end. None of them asks for a different base-ref resolution. The change also reverses the existing documented contract (a checkout withoutorigin/HEADshows no counts until its PR opens;git remote set-head origin --autoadds the ref). It rewritestest_worker_without_origin_head_shows_no_sizeinto..._falls_back_to_origin_main(tests/test_herdr_sidebar.py:456) and adds a no-base test (tests/test_herdr_sidebar.py:462). The fallback can also compare against a stale or unrelatedorigin/mainwhen the repo's default branch is something else. Smallest honest remedy: remove the fallback sobaseis the only ref, restore the old no-origin/HEADtest and doc sentence, and drop the extra test. This needs the author's decision because it was a deliberate behavior change.tests/test_herdr_sidebar.py:478- The 10 s refresh is the fix for the first ask ('status never update live'). No added test drives it.worker_reportcallssession_startonce and reads the last report. Every new test passes through thesession_startrefresh, which the old code also ran, so only the working-tree counting is covered. DeletingsetIntervalat config/herdr-sidebar.ts:248, or breaking thelookup=falseskip or the report-on-change gate at line 234, would leave the whole suite green. The reported failure (the size line frozen inside one long turn) is not reproduced. Add a test that runs the real extension: callsession_start, edit or stage a file in the checkout, wait past one tick (about 11 s, or use Bun fake timers), and assert a secondreport-metadatacall carries the new counts. A tick with no change should add no report.🔧 Fix applied.
5 warnings still open:
config/herdr-sidebar.ts:86- The size now comes fromgit diff --shortstat --merge-base <ref>, which compares against the working tree. It runs every 10 s inside every worker's checkout. The old<base>...HEADform never touched the index or working tree. I checked this in a scratch repo: aftertouch fon a tracked file, thisgit diffrewrites.git/index, so it must take.git/index.lock. It does so whenever tracked files have stale stat info, such as after a touch, checkout, rebase or stash, or just after agit add(racy timestamps). A worker's owngit add,git commitorgit stashthat lands in that window fails withfatal: Unable to create '.git/index.lock': File exists. The background timer, not the worker, causes that failure.git --no-optional-locksandGIT_OPTIONAL_LOCKS=0do not prevent it fordiff; I verified both still rewrite the index.-c diff.autorefreshindex=falsedoes prevent it. Fix: run the diff asgit -c diff.autorefreshindex=false diff --shortstat --merge-base <ref>.ls-files --othersandrev-parsedo not write the index, so line 86 is the only site.config/herdr-sidebar.ts:234-statis assigned at line 232 beforereport()runs. The timer tick then decides whether to report by comparingJSON.stringify(stat)withbefore. Sequence: a long turn, the size changes, the 10 s tick setsstatand callsreport(), andreport()throws (Herdr restarting or timing out). The outercatchswallows the error. At the next tickbeforealready equalsstat, so nothing is reported. The sidebar keeps the old size until the size changes again or the turn ends, which is the 'does not update live' symptom this change is meant to fix. Fix: keep areportedsnapshot that is set only afterawait report()succeeds, and comparestatagainst it rather than against the pre-refresh value. The turn-end and lookup paths report every time, so they are unaffected.config/herdr-sidebar.ts:91- The new untracked-file counting runs every 10 s per worker instead of once per turn end, and three things go wrong with it. (1)readFileloads each whole file, including large binaries, and only afterwards checks the first 8000 bytes for a NUL. A worker with a few hundred MB of unignored output (screenshots, logs, downloads) therefore re-reads all of it every cycle, thentoString('latin1').split('\n')allocates a string and array the size of each text file (line 93). Theponytail:comment on line 88 names this ceiling, but the timer multiplies its cost. (2)readFilefollows symlinks. git counts an untracked symlink as 1 file with 1 line, which I checked withgit add -Nand--shortstat. The code counts the target's line count instead. A symlink to a directory hits EISDIR and counts 0 lines. A symlink to a huge file or a special file reads the target in full. Anode_modules/ignore pattern does not match a symlink, so this case is reachable. (3)runuses execFile's default 1 MiBmaxBuffer. A large untracked listing (a tree with roughly 17k or more unignored files) makesls-filesreject. The.catch(() => stat)at line 232 then silently freezes the old size indefinitely. Behavior-preserving fix:lstatfirst and count a symlink as 1 line; open each file and read only the first 8000 bytes for the binary check, so binaries are never fully read; raisemaxBufferor stream thels-filesoutput. A hard cap on bytes or files would change the displayed counts, so that is the author's decision. Thedocs/herdr.mdsentence 'counted as a new file (its lines, or none for a binary file)' and the comment on line 87 claim git parity that symlinks break.config/herdr-sidebar.ts:79- Component not required by the intent: theorigin/main, thenorigin/masterbase fallback in thefor (const r of [base, "origin/main", "origin/master"])loop. The intent names three defects: committed-only counting, clearing the size when there is no PR or commit, and refreshing only onsession_startandagent_end. None of them asks for a different base-ref resolution. The change also reverses the existing documented contract (a checkout withoutorigin/HEADshows no counts until its PR opens;git remote set-head origin --autoadds the ref). It rewritestest_worker_without_origin_head_shows_no_sizeinto..._falls_back_to_origin_main(tests/test_herdr_sidebar.py:456) and adds a no-base test (tests/test_herdr_sidebar.py:462). The fallback can also compare against a stale or unrelatedorigin/mainwhen the repo's default branch is something else. Smallest honest remedy: remove the fallback sobaseis the only ref, restore the old no-origin/HEADtest and doc sentence, and drop the extra test. This needs the author's decision because it was a deliberate behavior change.config/herdr-sidebar.ts:103- Carried over from round 1 and not yet decided. Theorigin/main, thenorigin/masterfallback infor (const r of [base, "origin/main", "origin/master"])is not needed by the intent. The intent names three defects: committed-only counting, clearing the size when there is no PR or commit, and refreshing only onsession_startandagent_end. None of them asks for a different base-ref resolution. The fallback also reverses the documented contract that a checkout withoutorigin/HEADshows no counts until its PR opens, anddocs/herdr.mdand the header comment now describe it. It can give a silently wrong number. Take a repo whose default branch isdevelop, with noorigin/HEADbut a staleorigin/main. The diff is then taken against that stale branch and the sidebar shows inflated counts. A PR whose base ref was never fetched falls through toorigin/mainthe same way, where before it kept the previous value. Tests tied to the fallback:test_worker_without_origin_head_falls_back_to_origin_main(tests/test_herdr_sidebar.py:482) andtest_worker_without_any_base_ref_shows_no_size(tests/test_herdr_sidebar.py:488). Smallest remedy: usebaseas the only ref. Restore the original no-origin/HEADtest and the doc sentence, and drop the extra test. The remedy changes deliberate behavior, so the author must decide.✅ **Test** - passed
✅ No issues found.
+2 · −1 · ✎ 2appears under└ workin the Herdr Agents sidebar within ~2s, no turn run.git/indexbyte-identical, with noindex.lock, and the worker's owngit addsucceedswho, never a size, under real omp/exitin real omp clears the tokens and removes the sidebar entry, and no omp process remainspython3 -m pytest tests/test_herdr_sidebar.py -q(69 passed)python3 -m pytest tests/test_herdr_sidebar.py -k worker -v(8 worker-size tests passed)Real omp 18.4.2 run live in an isolated real Herdr 0.9.3 server (temp HOME/XDG, Agents layout from config/default.yml). The extension was loaded withomp --no-extensions -e <worktree>/config/herdr-sidebar.ts, with FM_TASK_ID set, in scratch git clones. Tokens were read withherdr pane getand compared with a git oracle (git add -N+git diff --shortstat). A real Herdr TUI client ran in a 150x40 pty (TIOCSWINSZ set before attaching), rendered through pyte from uv's ephemeral cache.Real-omp A: unstaged edit, stage plus untracked file, local commit, more edits, then reset --hard + clean. Sizes were +2 −1 ✎2, then +5 −1 ✎3, then the same after the commit, then +8 −1 ✎3, then cleared. The sidebar line appeared and disappeared with them.Real-omp B: a worker with only a local commit shows +1 ✎1 at session start, matching the oracle with a clean working tree.Real-omp C: with a stale mtime on a tracked file, a control showed plaingit diff --merge-baserewrites.git/index. During about 25s of timer ticks the index was byte-identical, with noindex.lock, and the worker'sgit addsucceeded.Real-omp D: withHERDR_BIN_PATHpointing at a wrapper that failsreport-metadata, a size edit was reported nowhere for 14s. After recovery the size appeared 6s later with no new edit.Real-omp E: untracked parity with git for an ignored file, a binary, symlinks (including to a directory and /dev/zero), a nested repo and a file without a final newline. The extension and the oracle both gave +11, 8 files.Real-omp F1/F2: with no origin/HEAD the size comes from origin/main (+1 ✎1). With no base ref at all there are no tokens.Real-omp G: a home pane (no FM_TASK_ID, no PR) with a dirty checkout reported onlywho, never a size.Real-omp baseline: the extension from f1a6d76 loaded the same way showed no tokens and no sidebar line for uncommitted edits (25s), and none after a local commit in the same session (25s).Quit and cleanup:/exitreturned each pane to a shell prompt and cleared the tokens, and the worker entry left the sidebar. Apscheck found no omp, host.ts or TUI-capture process from my runs. I stopped the isolated Herdr servers and removed /tmp/hs. The worktreegit status --ignoredis clean, including the ignored pytest and pycache directories I removed.Earlier stub-host runs (host.ts) loaded the real extension and fired fake pi events against real Herdr and git. They are corroboration only; every scenario above was then re-driven under real omp.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.