fix(status-line): honor hidden jobs segment for background job count - #3784
fix(status-line): honor hidden jobs segment for background job count#3784pers0n4 wants to merge 1 commit into
Conversation
|
@pers0n4 — this PR can't be merged as-is: the merge attempt was refused and GitHub currently reports the branch as conflicting with
Please rebase |
b042c6e to
64d9a1b
Compare
yazzang-homelab
left a comment
There was a problem hiding this comment.
Independent architect review. The code change is right; the CHANGELOG entry is in the wrong section and needs one move before this lands.
Blocking — the entry was filed into an already-released version.
On this head, packages/coding-agent/CHANGELOG.md:
32| ## [0.12.11] - 2026-08-03
34| ### Fixed
...
40| - The legacy interactive footer now uses the session manager's cumulative usage index...
41| ### Fixed <- second heading, no blank line above
43| - Hiding the `jobs` status-line segment now also hides the background-job counter...
45| ## [0.12.10] - 2026-08-03
The new entry sits under ## [0.12.11], which shipped on 2026-08-03. It also introduces a duplicate ### Fixed heading directly against the preceding bullet with no blank line.
This is almost certainly not your doing. .gitattributes carries packages/*/CHANGELOG.md merge=union, and union never conflicts — it concatenates both sides of an overlapping hunk. Release commits insert ## [X.Y.Z] directly beneath the surviving ## [Unreleased] heading, so a branch that added entries under Unreleased overlaps exactly that region and gets silently relocated on rebase. #3929 has the reproduction and an audit finding 35 pre-existing instances on dev; #3932 removes the driver.
Fix: move the entry under ## [Unreleased] and drop the duplicate ### Fixed heading (reuse the existing one). Nothing else needs to change.
The code itself is sound. Verified:
jobsSegmentVisibleis computed fromeffectiveSettings, i.e. the output of#resolveSettings(), so preset users andpreset: "custom"users go through one path rather than the counter having its own placement rule.- Gating the count at the source rather than at the render site means the
runningBackgroundJobs > 0branch below stays untouched, so icon, color, and overflow behaviour are unchanged when the segment is visible. - Every bundled preset keeps
jobsinrightSegments, so default output does not move. The added test pins the hidden case with an explicitrightSegments: ["cost"]. - Rejecting a new
statusLine.showBackgroundJobssetting is the right call — it would have been a second source of truth for a placement the user already expressed.
One non-blocking note: with the segment hidden, getAsyncJobSnapshot() is no longer called at all on that path. That is fine today because the call is pure, but if it ever acquires a side effect (lazy snapshot materialization, for instance) the visible and hidden paths would diverge. Worth a comment if that ever becomes a possibility.
gajae.pr-review-verdict.v1 merge-blocked sha256:64d9a1b8d35bd6fa533439418b4189d51415a505 reviewer:architect evidence:https://github.com/Yeachan-Heo/gajae-code/pull/3784/files
`#collectStatusSegments()` appended a hardcoded "N jobs running" chip to the right group after rendering the configured segments, without consulting `statusLine.leftSegments` / `statusLine.rightSegments`. Setting the `jobs` segment to hidden in `/settings` removed only the monitor/cron widget, while the async background-job counter kept rendering with the same icon and the same `statusLineSubagents` color, so the setting looked ignored. `async.enabled` does not help either: `isBackgroundJobSupportEnabled()` always returns true, so the manager and the counter exist in every top-level session. Gate the counter on the `jobs` segment placement resolved by `#resolveSettings()`, so preset and `preset: custom` users share one code path. Every bundled preset keeps `jobs` in `rightSegments`, so default output is unchanged. Lore-id: 4b7e2c19 Rejected: new statusLine.showBackgroundJobs setting | duplicates the existing jobs segment placement the user already set Confidence: high Scope-risk: narrow Reversibility: clean Tested: jobs-segment suite (7); status-line-*, jobs-*, interactive-mode-status (130); biome check Not-tested: live TUI overflow trimming when the counter is the last right-group entry
64d9a1b to
b6e354b
Compare
|
CI가 안 도는 이유를 확인했다 — 네 잘못이 아니다. 이 PR의 워크플로 런은 푸시를 더 해도 달라지지 않는다. 승인 없이는 새 런도 같은 상태로 들어간다. 같은 사유로 막힌 PR이 6건이라 #3940 으로 정리해 올렸다. 리뷰는 CI와 무관하게 진행하고 있으니 코드 피드백은 그대로 받으면 된다. 다만 이 저장소는 머지에 exact-head CI 증거를 요구하므로, 승인이 떨어지기 전까지는 머지가 불가능하다는 점만 알아두면 된다. |
|
Closing during the emergency maintenance freeze. This PR is not in the retained critical or maintainer-owned set. Do not open a replacement PR unless a maintainer explicitly directs it. — |
|
CHANGELOG 항목이 이미 릴리스된 섹션에 들어가 있다.
원인은 리베이스로는 안 풀린다 — 위치가 이미 커밋돼 있어서 직접 옮겨야 한다. 해당 줄을 잘라 내 PR(#3844)도 같은 상태였고 방금 고쳤다. |
What
Hiding the
jobsstatus-line segment now also hides the background-job counter.StatusLineComponent#collectStatusSegments()appended a hardcodedN jobs runningchip to the right group after rendering the configured segments, without ever consultingstatusLine.leftSegments/statusLine.rightSegments:This PR gates that counter on the
jobssegment placement resolved by#resolveSettings(), so preset users andpreset: customusers share one code path.Why
GJC has two independent job-rendering paths:
jobssegment (status-line/segments.ts)JobsObserversnapshot — monitor/cron countsleft/rightSegmentstool-status-header.ts:809)AsyncJobManagerrunning jobs minus monitors — async bash / task subagentsSetting
jobsto hidden in/settingscalls#setSegmentPlacement(), which only removes"jobs"from the two arrays. The legacy chip never reads those arrays, and it renders with the sametheme.icon.agentsglyph and the samestatusLineSubagentscolor as the segment — so the user sees an identical-looking job indicator survive the setting and concludes it was ignored.Turning off
async.enableddoes not help either:isBackgroundJobSupportEnabled()unconditionally returnstrue, soAsyncJobManagerexists in every top-level session and the chip appears whenever any non-monitor background job runs.Existing coverage missed this because both legacy-chip tests in
test/jobs-segment.test.tsconfigurerightSegments: ["jobs"]; there was no hidden-placement case.Every bundled preset keeps
jobsinrightSegments(asserted by the existingAC4test), so default output is unchanged. Only users who explicitly hid the segment see a difference — which is the point.Testing
The new regression test fails on the parent commit with exactly the reported symptom:
Known unrelated failure on
devRoot
bun run checkcurrently fails atcheck:sdk-closureon unmodifiedorigin/dev(9d0a1c3), before and after this change:Both test files exist on
devbut are absent fromscripts/telegram-daemon-generation-manifest.json, which was last regenerated at45bc057— beforeb8ea855anda10c6deadded those tests. This PR touches neither the manifest nor any notifications code; the drift is pre-existing and out of scope here.GJC verdict
No independent architect/critic/human review has happened — the author of this change is also the only party that has looked at it, and the template treats self-approval as BLOCK.
Head:
4b240ca04fe80369e00d183047124ad4d4741139Hash reproduces with
git diff origin/dev..HEAD | shasum -a 256.devbun checkpasses — package-levelbun --cwd=packages/coding-agent run checkpasses; rootbun run checkfails only at the pre-existingcheck:sdk-closuretelegram manifest drift documented above