mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-02 21:50:52 +00:00
feat(cli): support refreshInterval in statusLine for periodic refresh (#3383)
* feat(cli): support refreshInterval in statusLine for periodic refresh The statusLine (#3311) re-runs only when Agent state changes (token count, model, git branch, etc.). Commands that display *external* data — a clock, rate-limit counters, CI build status — have no Agent event to hook into and go stale between messages. Add an optional `ui.statusLine.refreshInterval` field (seconds, minimum 1) that schedules a setInterval alongside the existing event-driven updates. Overlap with state-change debounce is safe: `doUpdate` kills any in-flight child and bumps the generation counter, so only the most recent output reaches the footer. Validation lives in `getStatusLineConfig`: - Must be `number`, `Number.isFinite(...)`, `>= 1` - Anything else is silently dropped (no interval scheduled) No changes to the default behavior — configs without `refreshInterval` behave exactly as before. * fix(cli): yield periodic statusLine tick when previous exec is in flight Review feedback on #3383: with `refreshInterval: 1` and a command whose real exec time exceeds 1s, each tick was unconditionally calling `doUpdate()` — which kills the in-flight child and bumps the generation counter — so the prior exec's callback was always discarded as stale. `setOutput` was never reached and the statusline stayed empty until `refreshInterval` was removed or the command became faster. Guard the interval callback with an `activeChildRef` check so a pending exec is allowed to finish. State-change triggers (model switch, token count, branch, etc.) still go through `scheduleUpdate` → `doUpdate` directly and legitimately preempt stale children; only the periodic tick yields. The existing 5s exec timeout is still the hard ceiling. Also drop the redundant `'refreshInterval' in raw` check — the `typeof raw.refreshInterval === 'number'` guard already excludes missing / undefined values. Tests: - Add regression test `'skips periodic ticks while a previous exec is still running'` — three ticks during one unfinished exec trigger zero new spawns; the next tick after callback completion does spawn. - Update two existing tests to resolve the mount exec before expecting subsequent ticks (the old tests implicitly relied on the starvation behavior being tolerated). * test(cli): assert user-visible lines state in starvation regression Self-review insight: the existing `skips periodic ticks while a previous exec is still running` test only counted `exec` calls — it confirmed the guard prevents redundant spawns, but would have silently passed even if the eventual callback was still being discarded as stale (which is the actual user-visible symptom of the starvation bug). Add `expect(result.current.lines).toEqual(['done'])` after resolving the mount's pending callback. Without the guard, generationRef would have bumped 3 times during the yielded ticks, the callback's captured gen would fail the stale check, `setOutput` would never fire, and `lines` would stay empty — now caught explicitly. * perf(cli): dedupe statusLine output to skip unchanged Footer re-renders Review feedback on #3383 (narrow terminal stacking): when `refreshInterval` fires at 1s and the command output is unchanged, the mount-and-setOutput cycle still allocates a new array and triggers a Footer re-render. Under certain narrow-terminal conditions, Ink's erase-line accounting mis-counts wrapped rows and stale content accumulates on screen. The Footer-layout root cause is in #3311's narrow-mode flex setup and Ink's truncate semantics, which is out of scope for this PR. But we can cut the re-render surface here by preserving the `lines` array reference when the command produces identical output — a strict Pareto improvement for any caller (clock-style statuslines with second-precision still re-render; rate-limit / branch / CI-status style statuslines that change infrequently stop triggering work every tick). Tests: - `preserves the same lines array reference when output is unchanged` asserts referential equality after a re-exec with identical stdout. - `produces a new reference when output changes` guards against over-eager dedup that would miss legitimate updates. * fix(cli): stabilize Footer rendering in narrow terminals Narrow-terminal E2E feedback on #3383: with `refreshInterval` at 1s, empty lines were accumulating above the input prompt each tick. Root cause is in the Footer flex layout — originally from #3311 — where Ink miscounts logical rows vs the physical rows the terminal actually uses. Two adjustments, both idiomatic (used elsewhere in the repo already): 1. Left column — `minWidth={0}`. Without this, Yoga's `min-width: auto` default keeps the Box at its natural content width, so a statusline wider than the terminal doesn't engage `<Text wrap="truncate">`; the text renders at content-width and the terminal wraps it physically. `minWidth={0}` lets the column shrink so the text child can truncate at container width. 2. Right section — `flexWrap="wrap"`. With multiple indicators (sandbox label, debug badge, dream, context-usage) the row can exceed a narrow terminal's width. Without `flexWrap` Ink lays them out in a single logical row, but the terminal physically wraps to two — Ink's erase sequence (`\e[2K\e[1A…` per logical row) then clears one row while two exist, and the extra row ghosts every re-render. With `wrap` Ink tracks the second row explicitly and erases correctly. Together these make the Footer's row count match between Ink's logical view and the terminal's physical view, so frequent re-renders (as `refreshInterval` enables) stop accumulating ghost rows. Needs verification in a real narrow TTY — from this environment I can reason about the flex semantics and confirm both props are supported by Ink's Box, but actually observing ghost-row elimination requires process.stdout.columns on a real terminal. * Revert "fix(cli): stabilize Footer rendering in narrow terminals" This reverts commit9758cda85f. Reason: I could not reproduce BZ-D's reported ghost-row stacking in tmux (40x25, 2-line statusline + real exec + Static history + refreshInterval: 1) over 14+ ticks. Both `minWidth={0}` and `flexWrap="wrap"` are legitimate defensive idioms, but without a failing repro I can't verify they address the reported bug, and I shouldn't ship a speculative layout change as "the fix". Keeping the output-dedup commit (e1d321186) — that one is a strict improvement regardless of the underlying Ink behavior. Will request BZ-D's specific terminal setup and reopen with a verified fix (or confirm the issue is specific to a particular emulator, not flex/Ink).
This commit is contained in:
parent
f340d95446
commit
4bf5bf22de
6 changed files with 352 additions and 20 deletions
|
|
@ -509,8 +509,15 @@ const SETTINGS_SCHEMA = {
|
|||
label: 'Status Line',
|
||||
category: 'UI',
|
||||
requiresRestart: false,
|
||||
default: undefined as { type: 'command'; command: string } | undefined,
|
||||
description: 'Custom status line display configuration.',
|
||||
default: undefined as
|
||||
| {
|
||||
type: 'command';
|
||||
command: string;
|
||||
refreshInterval?: number;
|
||||
}
|
||||
| undefined,
|
||||
description:
|
||||
'Custom status line display configuration. Optional `refreshInterval` (seconds, >= 1) re-runs the command on a timer so external data stays fresh.',
|
||||
showInDialog: false,
|
||||
},
|
||||
customThemes: {
|
||||
|
|
|
|||
|
|
@ -82,7 +82,9 @@ let stdinErrorHandler: ((err: Error) => void) | undefined;
|
|||
let mockKill: ReturnType<typeof vi.fn>;
|
||||
|
||||
function setStatusLineConfig(
|
||||
config: { type: string; command: string } | undefined,
|
||||
config:
|
||||
| { type: string; command: string; refreshInterval?: number }
|
||||
| undefined,
|
||||
) {
|
||||
mockSettings.merged = config ? { ui: { statusLine: config } } : {};
|
||||
}
|
||||
|
|
@ -779,4 +781,254 @@ describe('useStatusLine', () => {
|
|||
expect(result.current.lines).toEqual(['recovered']);
|
||||
});
|
||||
});
|
||||
|
||||
// --- Output deduplication (cuts unnecessary Footer re-renders) ---
|
||||
|
||||
describe('output deduplication', () => {
|
||||
it('preserves the same lines array reference when output is unchanged', async () => {
|
||||
setStatusLineConfig({ type: 'command', command: 'echo same' });
|
||||
const { result, rerender } = renderHook(() => useStatusLine());
|
||||
|
||||
await act(async () => {
|
||||
execCallback(null, 'same output\n', '');
|
||||
});
|
||||
const firstRef = result.current.lines;
|
||||
expect(firstRef).toEqual(['same output']);
|
||||
|
||||
// Trigger another exec with identical output (e.g. via state change).
|
||||
mockUIState.currentModel = 'new-model';
|
||||
rerender();
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(300);
|
||||
});
|
||||
await act(async () => {
|
||||
execCallback(null, 'same output\n', '');
|
||||
});
|
||||
|
||||
// Reference preserved → React can skip the Footer re-render.
|
||||
expect(result.current.lines).toBe(firstRef);
|
||||
});
|
||||
|
||||
it('produces a new reference when output changes', async () => {
|
||||
setStatusLineConfig({ type: 'command', command: 'echo tick' });
|
||||
const { result, rerender } = renderHook(() => useStatusLine());
|
||||
|
||||
await act(async () => {
|
||||
execCallback(null, 'first\n', '');
|
||||
});
|
||||
const firstRef = result.current.lines;
|
||||
expect(firstRef).toEqual(['first']);
|
||||
|
||||
mockUIState.currentModel = 'new-model';
|
||||
rerender();
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(300);
|
||||
});
|
||||
await act(async () => {
|
||||
execCallback(null, 'second\n', '');
|
||||
});
|
||||
|
||||
expect(result.current.lines).not.toBe(firstRef);
|
||||
expect(result.current.lines).toEqual(['second']);
|
||||
});
|
||||
});
|
||||
|
||||
// --- refreshInterval (periodic refresh) ---
|
||||
|
||||
describe('refreshInterval', () => {
|
||||
it('re-executes the command every N seconds', async () => {
|
||||
setStatusLineConfig({
|
||||
type: 'command',
|
||||
command: 'echo tick',
|
||||
refreshInterval: 2,
|
||||
});
|
||||
renderHook(() => useStatusLine());
|
||||
|
||||
// Mount executes once immediately
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(1);
|
||||
await act(async () => {
|
||||
execCallback(null, 'tick 1\n', '');
|
||||
});
|
||||
|
||||
// First interval tick after 2s — previous exec has completed, so
|
||||
// the tick is free to spawn a new one.
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(2000);
|
||||
});
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(2);
|
||||
await act(async () => {
|
||||
execCallback(null, 'tick 2\n', '');
|
||||
});
|
||||
|
||||
// Second interval tick after another 2s
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(2000);
|
||||
});
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it('does not start an interval when refreshInterval is omitted', async () => {
|
||||
setStatusLineConfig({ type: 'command', command: 'echo static' });
|
||||
renderHook(() => useStatusLine());
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(1);
|
||||
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(60_000);
|
||||
});
|
||||
// Still only the mount exec — no periodic refresh
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('rejects refreshInterval < 1 (no interval scheduled)', async () => {
|
||||
setStatusLineConfig({
|
||||
type: 'command',
|
||||
command: 'echo tick',
|
||||
refreshInterval: 0.5,
|
||||
});
|
||||
renderHook(() => useStatusLine());
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(1);
|
||||
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(60_000);
|
||||
});
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('rejects non-finite refreshInterval (no interval scheduled)', async () => {
|
||||
setStatusLineConfig({
|
||||
type: 'command',
|
||||
command: 'echo tick',
|
||||
refreshInterval: Number.POSITIVE_INFINITY,
|
||||
});
|
||||
renderHook(() => useStatusLine());
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(1);
|
||||
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(60_000);
|
||||
});
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('clears the interval when config is removed', async () => {
|
||||
setStatusLineConfig({
|
||||
type: 'command',
|
||||
command: 'echo tick',
|
||||
refreshInterval: 1,
|
||||
});
|
||||
const { rerender } = renderHook(() => useStatusLine());
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Remove the config — the interval should be torn down.
|
||||
setStatusLineConfig(undefined);
|
||||
rerender();
|
||||
|
||||
const callsAfterRemoval = vi.mocked(child_process.exec).mock.calls.length;
|
||||
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(10_000);
|
||||
});
|
||||
expect(vi.mocked(child_process.exec).mock.calls.length).toBe(
|
||||
callsAfterRemoval,
|
||||
);
|
||||
});
|
||||
|
||||
it('reschedules when refreshInterval changes', async () => {
|
||||
setStatusLineConfig({
|
||||
type: 'command',
|
||||
command: 'echo tick',
|
||||
refreshInterval: 5,
|
||||
});
|
||||
const { rerender } = renderHook(() => useStatusLine());
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(1);
|
||||
await act(async () => {
|
||||
execCallback(null, 'tick\n', '');
|
||||
});
|
||||
|
||||
// 2s passes — not yet a tick on the 5s schedule.
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(2000);
|
||||
});
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Swap to a 1s interval — the old 5s timer must be cleared, not kept.
|
||||
setStatusLineConfig({
|
||||
type: 'command',
|
||||
command: 'echo tick',
|
||||
refreshInterval: 1,
|
||||
});
|
||||
rerender();
|
||||
|
||||
// 1s later — fires on the new schedule.
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(1000);
|
||||
});
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('clears the interval on unmount', async () => {
|
||||
setStatusLineConfig({
|
||||
type: 'command',
|
||||
command: 'echo tick',
|
||||
refreshInterval: 1,
|
||||
});
|
||||
const { unmount } = renderHook(() => useStatusLine());
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(1);
|
||||
|
||||
unmount();
|
||||
|
||||
const callsAfterUnmount = vi.mocked(child_process.exec).mock.calls.length;
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(10_000);
|
||||
});
|
||||
expect(vi.mocked(child_process.exec).mock.calls.length).toBe(
|
||||
callsAfterUnmount,
|
||||
);
|
||||
});
|
||||
|
||||
it('skips periodic ticks while a previous exec is still running', async () => {
|
||||
// Starvation regression (#3383 review): with refreshInterval < command
|
||||
// latency, if every tick called doUpdate() it would kill the in-flight
|
||||
// child, generation++ would stale the eventual callback, and the user
|
||||
// would never see any output. This test asserts BOTH the guard (exec
|
||||
// call count) AND the user-visible result (rendered lines).
|
||||
setStatusLineConfig({
|
||||
type: 'command',
|
||||
command: 'slow-command',
|
||||
refreshInterval: 1,
|
||||
});
|
||||
const { result } = renderHook(() => useStatusLine());
|
||||
|
||||
// Mount exec: child is spawned, callback NOT yet resolved — child is
|
||||
// still "running" from the hook's perspective.
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(1);
|
||||
const pendingCallback = execCallback;
|
||||
|
||||
// Several interval ticks pass while the first exec is in flight.
|
||||
// Each tick must detect the running child and skip doUpdate().
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(1000);
|
||||
});
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(1000);
|
||||
});
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(1000);
|
||||
});
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(1);
|
||||
|
||||
// First exec finally completes — activeChildRef clears. Without the
|
||||
// guard, generationRef would have bumped 3 times above and the next
|
||||
// line would be ignored as stale, leaving `lines` permanently empty.
|
||||
await act(async () => {
|
||||
pendingCallback(null, 'done\n', '');
|
||||
});
|
||||
expect(result.current.lines).toEqual(['done']);
|
||||
|
||||
// Next tick is now free to spawn a new exec.
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(1000);
|
||||
});
|
||||
expect(child_process.exec).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -69,6 +69,10 @@ export interface StatusLineCommandInput {
|
|||
interface StatusLineConfig {
|
||||
type: 'command';
|
||||
command: string;
|
||||
// Re-run the command every N seconds so external data (git branch, quota,
|
||||
// clock) stays fresh even when no Agent state changes. Values < 1 are
|
||||
// rejected in getStatusLineConfig to avoid flooding the CLI with execs.
|
||||
refreshInterval?: number;
|
||||
}
|
||||
|
||||
const debugLog = createDebugLogger('STATUS_LINE');
|
||||
|
|
@ -93,6 +97,13 @@ function getStatusLineConfig(
|
|||
type: 'command',
|
||||
command: raw.command,
|
||||
};
|
||||
if (
|
||||
typeof raw.refreshInterval === 'number' &&
|
||||
Number.isFinite(raw.refreshInterval) &&
|
||||
raw.refreshInterval >= 1
|
||||
) {
|
||||
config.refreshInterval = raw.refreshInterval;
|
||||
}
|
||||
return config;
|
||||
}
|
||||
return undefined;
|
||||
|
|
@ -133,7 +144,10 @@ function buildMetricsPayload(
|
|||
* via stdin.
|
||||
*
|
||||
* Updates are debounced (300ms) and triggered by state changes (model switch,
|
||||
* new messages, vim mode toggle) rather than blind polling.
|
||||
* new messages, vim mode toggle) rather than blind polling. When the config
|
||||
* sets `refreshInterval` (seconds, >= 1), the command is additionally re-run
|
||||
* on a timer so external data (git branch, quota, clock) stays fresh even
|
||||
* when no Agent state has changed.
|
||||
*/
|
||||
export function useStatusLine(): {
|
||||
lines: string[];
|
||||
|
|
@ -145,6 +159,7 @@ export function useStatusLine(): {
|
|||
|
||||
const statusLineConfig = getStatusLineConfig(settings);
|
||||
const statusLineCommand = statusLineConfig?.command;
|
||||
const refreshInterval = statusLineConfig?.refreshInterval;
|
||||
|
||||
const [output, setOutput] = useState<string[]>([]);
|
||||
|
||||
|
|
@ -285,15 +300,27 @@ export function useStatusLine(): {
|
|||
(error, stdout) => {
|
||||
if (gen !== generationRef.current) return; // stale
|
||||
activeChildRef.current = undefined;
|
||||
if (!error && stdout) {
|
||||
const lines = stdout
|
||||
.replace(/\r?\n$/, '')
|
||||
.split(/\r?\n/)
|
||||
.filter(Boolean);
|
||||
setOutput(lines.slice(0, MAX_STATUS_LINES));
|
||||
} else {
|
||||
setOutput([]);
|
||||
}
|
||||
const nextLines =
|
||||
!error && stdout
|
||||
? stdout
|
||||
.replace(/\r?\n$/, '')
|
||||
.split(/\r?\n/)
|
||||
.filter(Boolean)
|
||||
.slice(0, MAX_STATUS_LINES)
|
||||
: [];
|
||||
// Skip the state update if the output is unchanged — avoids a
|
||||
// Footer re-render each periodic tick, which cuts wasted work
|
||||
// and reduces the window for Ink to miscount rows in narrow
|
||||
// terminals when `refreshInterval` runs at 1s (see #3383).
|
||||
setOutput((prev) => {
|
||||
if (
|
||||
prev.length === nextLines.length &&
|
||||
prev.every((v, i) => v === nextLines[i])
|
||||
) {
|
||||
return prev;
|
||||
}
|
||||
return nextLines;
|
||||
});
|
||||
},
|
||||
);
|
||||
} catch (err) {
|
||||
|
|
@ -389,6 +416,24 @@ export function useStatusLine(): {
|
|||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [statusLineCommand]);
|
||||
|
||||
// Periodic refresh — re-run the command every `refreshInterval` seconds.
|
||||
// The tick yields if a previous exec is still running: unlike state-change
|
||||
// triggers (which legitimately need to preempt stale data), the periodic
|
||||
// tick exists only to keep external data fresh, so killing an in-flight
|
||||
// child would starve commands that run longer than `refreshInterval` and
|
||||
// the statusline would never update. The 5s exec timeout still caps the
|
||||
// wait, and state-change triggers still go through `doUpdate` directly.
|
||||
useEffect(() => {
|
||||
if (!statusLineCommand || !refreshInterval) return;
|
||||
const timer = setInterval(() => {
|
||||
if (activeChildRef.current) return;
|
||||
doUpdate();
|
||||
}, refreshInterval * 1000);
|
||||
return () => {
|
||||
clearInterval(timer);
|
||||
};
|
||||
}, [statusLineCommand, refreshInterval, doUpdate]);
|
||||
|
||||
// Initial execution + cleanup
|
||||
useEffect(() => {
|
||||
hasMountedRef.current = true;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue