Skip to content

Improve terminal input status layout #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Desperado merged 2 commits into from
Jul 18, 2026
Merged

Improve terminal input status layout #152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ef820c
Improve terminal input status layout
Desperado Jul 17, 2026
a5f7766
Address context metric review feedback
Desperado Jul 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions internal/agent/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,14 @@
AppConfig *api.Config // persistent user preferences
History []api.Message
Usage api.TokenUsage
Logger *sysutil.Logger
Ollama *OllamaClient // optional self-hosted LLM
Mode OllamaMode // off, chat, or full
Cerebras *CerebrasClient // optional Cerebras backend; when set, owns the whole turn
// LastContextTokens is the input-token count reported by the most recent

Check notice on line 66 in internal/agent/agent.go

View check run for this annotation

Sigilix / Sigilix

[P3] testing: New LastContextTokens field and its reset logic are untested 

The PR adds a new exported field LastContextTokens to the Agent struct, along with reset logic in ClearHistory, Run, runStreamingLoop, and updates in callStreamingAPI and callAPI. This field is used to estimate context window occupancy in the TUI. No test coverage exists for this field's lifecycle, leaving regression risk for the UI's context-fill metric. Add a test that verifies LastContextTokens is zeroed on history clear, set on API success, and not carried over between backend switches.

Also uncovered: internal/repl/repl.go
More Info

- Threat model: If the field retains stale values, the UI will display incorrect context usage, misleading the user about remaining capacity.
- Specific code citations: Lines 66-68 define the field; lines 133, 149, 339, 670, 830 update it.
- Existing protections: No existing tests cover this field; the only test file changed is internal/tui/input_test.go, which only tests UI rendering.
- Proposed mitigation: Add unit tests to internal/agent/agent_test.go that exercise the new field's behavior across the reset and update points.
- Alternative mitigations considered: Rely on integration tests via the REPL, but unit tests are more precise and faster.
- Severity calibration: Score 4 because the field is reachable via exported methods and UI depends on it; incorrect values degrade user trust but do not cause data loss or security issues.

PR-level (score 2) · internal/agent/agent.go
// model request. Unlike Usage.InputTokens, it is not cumulative and can be
// used to estimate the currently occupied context window in the TUI.
LastContextTokens int
Logger *sysutil.Logger
Ollama *OllamaClient // optional self-hosted LLM
Mode OllamaMode // off, chat, or full
Cerebras *CerebrasClient // optional Cerebras backend; when set, owns the whole turn

tools []api.ToolDef
client *http.Client
Expand Down Expand Up @@ -126,6 +130,7 @@
// ClearHistory resets conversation history.
func (a *Agent) ClearHistory() {
a.History = []api.Message{}
a.LastContextTokens = 0

@sigilix sigilix Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 LOGICGROUNDED Ollama backend does not update LastContextTokens after a turn

The diff establishes a convention: every backend that reports per-turn input tokens sets Agent.LastContextTokens so the TUI can estimate context-window fill. The Anthropic streaming path (line 662), the non-streaming API path (line 822), and the Cerebras path (cerebras_agent.go line 71) all set a.LastContextTokens = <per-turn input tokens>. The Ollama path in agent.go does not, so after an Ollama turn LastContextTokens stays stale (zero or the previous backend's value), and the status bar shows a misleading context-fill percentage. Add a.LastContextTokens = <ollama per-turn input tokens> in the Ollama response-handling branch.

Example:

input: Ollama backend active, a turn completes with 5000 input tokens
actual: `LastContextTokens` remains 0 (or the previous backend's value)
expected: `LastContextTokens` == 5000, status bar shows correct context fill

Suggested fix:

a.LastContextTokens = ollamaResp.Usage.InputTokens

Why this wasn't caught: No test covers LastContextTokens being set after an Ollama turn; the existing TestContextWindow only tests the window-size helper.

More Info
  • Threat model: Users on the Ollama backend see a stale or zero context-fill percentage in the input status bar, misleading them about how full the context window is.
  • Specific code citations: Convention sites: agent.go line 662 (a.LastContextTokens = ev.Message.Usage.InputTokens), line 822 (a.LastContextTokens = apiResp.Usage.InputTokens), cerebras_agent.go line 71 (a.LastContextTokens = resp.Usage.PromptTokens). Violation site: the Ollama response-handling code in agent.go (the callOllama or equivalent function) — no LastContextTokens assignment.
  • Existing protections: None. LastContextTokens is only set in the three paths listed above; the Ollama path is a fourth code path that handles its own response and never touches the field.
  • Proposed mitigation: In the Ollama response handler, after extracting the per-turn input token count, assign a.LastContextTokens = <that count>.
  • Alternative mitigations considered: A default-to-zero approach would hide the metric for Ollama users, but the diff's intent is to show it for every backend that can report it. Ollama responses do carry usage info, so the metric is available.
  • Severity calibration: Score 3: the bug is real and user-visible (wrong context-fill display), but it only affects Ollama users and does not cause data loss or crashes.
Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/agent/agent.go
Line: 133

Comment:
**Ollama backend does not update `LastContextTokens` after a turn**

The diff establishes a convention: every backend that reports per-turn input tokens sets `Agent.LastContextTokens` so the TUI can estimate context-window fill. The Anthropic streaming path (line 662), the non-streaming API path (line 822), and the Cerebras path (`cerebras_agent.go` line 71) all set `a.LastContextTokens = <per-turn input tokens>`. The Ollama path in `agent.go` does not, so after an Ollama turn `LastContextTokens` stays stale (zero or the previous backend's value), and the status bar shows a misleading context-fill percentage. Add `a.LastContextTokens = <ollama per-turn input tokens>` in the Ollama response-handling branch.

Example:
input: Ollama backend active, a turn completes with 5000 input tokens
actual: `LastContextTokens` remains 0 (or the previous backend's value)
expected: `LastContextTokens` == 5000, status bar shows correct context fill

Threat model:
Users on the Ollama backend see a stale or zero context-fill percentage in the input status bar, misleading them about how full the context window is.

Specific code citations:
Convention sites: `agent.go` line 662 (`a.LastContextTokens = ev.Message.Usage.InputTokens`), line 822 (`a.LastContextTokens = apiResp.Usage.InputTokens`), `cerebras_agent.go` line 71 (`a.LastContextTokens = resp.Usage.PromptTokens`). Violation site: the Ollama response-handling code in `agent.go` (the `callOllama` or equivalent function) — no `LastContextTokens` assignment.

Existing protections:
None. `LastContextTokens` is only set in the three paths listed above; the Ollama path is a fourth code path that handles its own response and never touches the field.

Proposed mitigation:
In the Ollama response handler, after extracting the per-turn input token count, assign `a.LastContextTokens = <that count>`.

Alternative mitigations considered:
A default-to-zero approach would hide the metric for Ollama users, but the diff's intent is to show it for every backend that can report it. Ollama responses do carry usage info, so the metric is available.

Severity calibration:
Score 3: the bug is real and user-visible (wrong context-fill display), but it only affects Ollama users and does not cause data loss or crashes.

Suggested fix shape:
a.LastContextTokens = ollamaResp.Usage.InputTokens

Why this wasn't caught:
No test covers `LastContextTokens` being set after an Ollama turn; the existing `TestContextWindow` only tests the window-size helper.

How can I resolve this? If you propose a fix, please make it concise.

@sigilix sigilix Bot Jul 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 STYLEGROUNDED ClearHistory resets LastContextTokens but Run and runStreamingLoop also reset it — triple reset is redundant but not harmful

Low-confidence finding — expand to read

ClearHistory (agent.go:130) sets LastContextTokens = 0. Both Run (line 149) and runStreamingLoop (line 338) also reset the field to zero at the start of each turn. The REPL's /clear command (repl.go line 261) calls ClearHistory and also sets lastContextTokens = 0. This triple reset is defensive and not a bug, but it creates ambiguity about which site owns the reset contract. If a future change removes one reset without updating the others, the field could carry a stale value from a previous turn.

More Info
  • Threat model: A future refactor removes the reset in Run or runStreamingLoop assuming ClearHistory handles it, but ClearHistory is only called on /clear, not between normal turns. The field would then carry a stale value across turns.
  • Specific code citations: ClearHistory (agent.go:130), Run (agent.go:149), runStreamingLoop (agent.go:338), REPL /clear handler (repl.go:261).
  • Existing protections: The redundancy itself is the protection — all three sites reset the field. The risk is that a maintainer sees the redundancy and removes one site.
  • Proposed mitigation: Document which site is the canonical reset point (e.g., runStreamingLoop and Run are the turn-boundary resets; ClearHistory is the full-session reset) so future maintainers understand the contract.
  • Alternative mitigations considered: A single reset in a shared helper called by all three sites would eliminate the redundancy but adds indirection.
  • Severity calibration: Score 2 because the redundancy is currently harmless and the risk is purely about future maintainability.

Example:

input: Normal turn sequence without /clear
actual: LastContextTokens reset three times (harmless redundancy)
expected: Single canonical reset point

Suggested fix:

// In ClearHistory doc comment: // Resets the full conversation state. Per-turn context resets are handled by Run/runStreamingLoop.
Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/agent/agent.go
Line: 133

Comment:
**`ClearHistory` resets `LastContextTokens` but `Run` and `runStreamingLoop` also reset it — triple reset is redundant but not harmful**

`ClearHistory` (agent.go:130) sets `LastContextTokens = 0`. Both `Run` (line 149) and `runStreamingLoop` (line 338) also reset the field to zero at the start of each turn. The REPL's `/clear` command (repl.go line 261) calls `ClearHistory` and also sets `lastContextTokens = 0`. This triple reset is defensive and not a bug, but it creates ambiguity about which site owns the reset contract. If a future change removes one reset without updating the others, the field could carry a stale value from a previous turn.

Example:
input: Normal turn sequence without /clear
actual: LastContextTokens reset three times (harmless redundancy)
expected: Single canonical reset point

Threat model:
A future refactor removes the reset in `Run` or `runStreamingLoop` assuming `ClearHistory` handles it, but `ClearHistory` is only called on `/clear`, not between normal turns. The field would then carry a stale value across turns.

Specific code citations:
`ClearHistory` (agent.go:130), `Run` (agent.go:149), `runStreamingLoop` (agent.go:338), REPL `/clear` handler (repl.go:261).

Existing protections:
The redundancy itself is the protection — all three sites reset the field. The risk is that a maintainer sees the redundancy and removes one site.

Proposed mitigation:
Document which site is the canonical reset point (e.g., `runStreamingLoop` and `Run` are the turn-boundary resets; `ClearHistory` is the full-session reset) so future maintainers understand the contract.

Alternative mitigations considered:
A single reset in a shared helper called by all three sites would eliminate the redundancy but adds indirection.

Severity calibration:
Score 2 because the redundancy is currently harmless and the risk is purely about future maintainability.

Suggested fix shape:
// In ClearHistory doc comment: // Resets the full conversation state. Per-turn context resets are handled by Run/runStreamingLoop.

How can I resolve this? If you propose a fix, please make it concise.

}

// CancelCurrent cancels the current streaming request if one is in progress.
Expand All @@ -141,6 +146,9 @@
// Run executes a prompt through the agent loop and returns the final text response.
// Used for non-interactive (one-shot) mode.
func (a *Agent) Run(prompt string) (string, error) {
// This is a new top-level turn; do not show a prior request's context size
// if this request fails before the provider reports usage.
a.LastContextTokens = 0

@sigilix sigilix Bot Jul 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 ERRORGROUNDED Agent.Run leaves LastContextTokens stale when callAPI fails

Run resets LastContextTokens to 0 at the top of the method (line 149) to avoid showing a prior request's context size on failure. However, if callAPI (line 160) returns an error, the method returns immediately without updating LastContextTokens — the field retains the zero value set at the top. This is the correct contract (zero = no context estimate), but it is fragile: any future code that reads LastContextTokens between Run calls on the same Agent instance could misinterpret a zero as "no context used" when the last successful request actually had a non-zero context. The sibling runStreamingLoop (line 338) and the REPL's CLI-backend path (repl.go line 1098) both explicitly reset to zero before starting a turn, establishing a convention that zero means "this turn has not yet reported usage." Run follows this convention but does so implicitly via the top-of-method reset; the early-return path is correct but undocumented. Consider adding a comment at the early return noting that the zero value is intentional, or resetting LastContextTokens in a defer to make the contract explicit regardless of exit path.

Example:

input: Agent.Run('test') where callAPI fails
actual: LastContextTokens = 0 (stale from top-of-method reset)
expected: LastContextTokens = 0 (intentional, but fragile)

Suggested fix:

// At the early return: // LastContextTokens intentionally remains 0 — this turn produced no usage.

Why this wasn't caught: No test exercises the Run failure path and asserts LastContextTokens remains zero.

More Info
  • Threat model: A future maintainer adds code between Run calls that reads LastContextTokens and assumes a non-zero value means the last request succeeded. The stale zero from a failed Run would be indistinguishable from a fresh Agent with no history.
  • Specific code citations: Agent.Run (agent.go:146-170) resets LastContextTokens at line 149, calls callAPI at line 160, and returns early on error at line 162 without touching the field again. runStreamingLoop (agent.go:338) and repl.go:1098 both explicitly reset to zero.
  • Existing protections: The REPL's inputStatus closure reads ag.LastContextTokens only after a successful turn (repl.go line 1200), so the stale zero is not currently observable. The protection is temporal coupling, not a guard.
  • Proposed mitigation: Add a comment at the early return (line 162) noting that LastContextTokens intentionally remains zero, or wrap the reset in a defer to make the contract independent of exit path.
  • Alternative mitigations considered: A defer-based reset would be more robust but changes the control flow for the success path as well. A comment is the smallest fix.
  • Severity calibration: Score 3 because the bug is latent — no current code path reads the stale value incorrectly — but the inconsistency between the three reset sites makes the contract easy to break in future changes.
Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/agent/agent.go
Line: 151

Comment:
**`Agent.Run` leaves `LastContextTokens` stale when `callAPI` fails**

`Run` resets `LastContextTokens` to 0 at the top of the method (line 149) to avoid showing a prior request's context size on failure. However, if `callAPI` (line 160) returns an error, the method returns immediately without updating `LastContextTokens` — the field retains the zero value set at the top. This is the correct contract (zero = no context estimate), but it is fragile: any future code that reads `LastContextTokens` between `Run` calls on the same Agent instance could misinterpret a zero as "no context used" when the last successful request actually had a non-zero context. The sibling `runStreamingLoop` (line 338) and the REPL's CLI-backend path (repl.go line 1098) both explicitly reset to zero before starting a turn, establishing a convention that zero means "this turn has not yet reported usage." `Run` follows this convention but does so implicitly via the top-of-method reset; the early-return path is correct but undocumented. Consider adding a comment at the early return noting that the zero value is intentional, or resetting `LastContextTokens` in a defer to make the contract explicit regardless of exit path.

Example:
input: Agent.Run('test') where callAPI fails
actual: LastContextTokens = 0 (stale from top-of-method reset)
expected: LastContextTokens = 0 (intentional, but fragile)

Threat model:
A future maintainer adds code between `Run` calls that reads `LastContextTokens` and assumes a non-zero value means the last request succeeded. The stale zero from a failed `Run` would be indistinguishable from a fresh Agent with no history.

Specific code citations:
`Agent.Run` (agent.go:146-170) resets `LastContextTokens` at line 149, calls `callAPI` at line 160, and returns early on error at line 162 without touching the field again. `runStreamingLoop` (agent.go:338) and `repl.go:1098` both explicitly reset to zero.

Existing protections:
The REPL's `inputStatus` closure reads `ag.LastContextTokens` only after a successful turn (repl.go line 1200), so the stale zero is not currently observable. The protection is temporal coupling, not a guard.

Proposed mitigation:
Add a comment at the early return (line 162) noting that `LastContextTokens` intentionally remains zero, or wrap the reset in a defer to make the contract independent of exit path.

Alternative mitigations considered:
A defer-based reset would be more robust but changes the control flow for the success path as well. A comment is the smallest fix.

Severity calibration:
Score 3 because the bug is latent — no current code path reads the stale value incorrectly — but the inconsistency between the three reset sites makes the contract easy to break in future changes.

Suggested fix shape:
// At the early return: // LastContextTokens intentionally remains 0 — this turn produced no usage.

Why this wasn't caught:
No test exercises the `Run` failure path and asserts `LastContextTokens` remains zero.

How can I resolve this? If you propose a fix, please make it concise.

a.History = append(a.History, api.Message{
Role: "user",
Content: prompt,
Expand Down Expand Up @@ -327,6 +335,11 @@
}

func (a *Agent) runStreamingLoop(term *tui.Terminal) (string, error) {
// All backends share this top-level turn boundary, including Ollama. Ollama's
// streaming protocol does not expose usage, so leaving this at zero hides the
// context-fill metric instead of carrying a stale value from another backend.
a.LastContextTokens = 0

@sigilix sigilix Bot Jul 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 LOGICGROUNDED Ollama streaming path never updates LastContextTokens

runStreamingLoop sets a.LastContextTokens = 0 at the top and the comment says 'Ollama's streaming protocol does not expose usage, so leaving this at zero hides the context-fill metric'. However, the Ollama path inside runStreamingLoop (line 338-344) calls a.runOllamaLoop(term) which returns without ever setting LastContextTokens. This is intentional per the comment, but the REPL's inputStatus closure reads lastContextTokens (which is set from ag.LastContextTokens only on success). If the Ollama turn succeeds, lastContextTokens stays at the zero reset from the top of the turn, so the status panel shows no context-fill metric — which is the desired behavior. No bug here.

More Info
  • Threat model: N/A — this is intentional behavior.
  • Specific code citations: internal/agent/agent.go lines 335-344.
  • Existing protections: The comment on line 338 explicitly documents the intent.
  • Proposed mitigation: No fix needed.
  • Alternative mitigations considered: N/A
  • Severity calibration: Score 4 is too high; this is intentional.
Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/agent/agent.go
Line: 341

Comment:
**Ollama streaming path never updates LastContextTokens**

`runStreamingLoop` sets `a.LastContextTokens = 0` at the top and the comment says 'Ollama's streaming protocol does not expose usage, so leaving this at zero hides the context-fill metric'. However, the Ollama path inside `runStreamingLoop` (line 338-344) calls `a.runOllamaLoop(term)` which returns without ever setting `LastContextTokens`. This is intentional per the comment, but the REPL's `inputStatus` closure reads `lastContextTokens` (which is set from `ag.LastContextTokens` only on success). If the Ollama turn succeeds, `lastContextTokens` stays at the zero reset from the top of the turn, so the status panel shows no context-fill metric — which is the desired behavior. No bug here.

Threat model:
N/A — this is intentional behavior.

Specific code citations:
`internal/agent/agent.go` lines 335-344.

Existing protections:
The comment on line 338 explicitly documents the intent.

Proposed mitigation:
No fix needed.

Alternative mitigations considered:
N/A

Severity calibration:
Score 4 is too high; this is intentional.

How can I resolve this? If you propose a fix, please make it concise.


// Cerebras backend owns the entire turn via its own native function-calling
// loop (full tool set). No Claude fallback — the user has no Anthropic key.
if a.Cerebras != nil {
Expand Down Expand Up @@ -654,6 +667,7 @@
var ev sseMessageStart
if err := json.Unmarshal([]byte(rawData), &ev); err == nil {
a.Usage.InputTokens += ev.Message.Usage.InputTokens
a.LastContextTokens = ev.Message.Usage.InputTokens

@sigilix sigilix Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 LOGICGROUNDED LastContextTokens is not reset at turn start in the streaming API path, leaking the previous turn's value on failure

The CC backend (CCAgent.Run) resets lastTurnIn, lastTurnOut, and lastTurnOK to zero at the top of every turn, so a failed turn shows no stale context usage. The streaming API path (callStreamingAPI) only sets a.LastContextTokens when the SSE message_start event arrives (line 662). If the request fails before that event — or if the event never carries usage — the status bar retains the previous turn's LastContextTokens, misrepresenting the current context fill. The fix is to zero a.LastContextTokens at the start of callStreamingAPI, matching the CC backend's contract.

Example:

Turn 1: API succeeds, LastContextTokens = 12000. Turn 2: API fails before message_start. Status bar shows 'ctx 12.0k/200.0k (6%)' — the stale value from Turn 1.

Suggested fix:

a.LastContextTokens = 0

Why this wasn't caught: No test exercises the LastContextTokens value after a failed API call.

More Info
  • Threat model: A user sees a stale context-window percentage in the TUI status bar after a failed API call, leading to incorrect assumptions about how full the context window is.
  • Specific code citations: CCAgent.Run resets lastTurnIn/Out/OK at line 361 of cc_agent.go. callStreamingAPI sets LastContextTokens at line 662 of agent.go but never resets it at the top of the function.
  • Existing protections: The REPL's lastContextTokens variable is only updated on success (repl.go line 1195), but the Agent field itself is not cleared on failure, so a subsequent success that doesn't carry usage would still show the stale value.
  • Proposed mitigation: Add a.LastContextTokens = 0 at the top of callStreamingAPI, before the request is sent.
  • Alternative mitigations considered: Resetting in the REPL after every turn would work but would miss the case where the Agent is used outside the REPL. Resetting in the Agent is the correct single point of truth.
  • Severity calibration: Score 3: a stale UI metric, not a correctness or security bug. The blast radius is limited to the status bar display, and the likelihood is moderate (any failed API call triggers it).
Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/agent/agent.go
Line: 662

Comment:
**`LastContextTokens` is not reset at turn start in the streaming API path, leaking the previous turn's value on failure**

The CC backend (`CCAgent.Run`) resets `lastTurnIn`, `lastTurnOut`, and `lastTurnOK` to zero at the top of every turn, so a failed turn shows no stale context usage. The streaming API path (`callStreamingAPI`) only sets `a.LastContextTokens` when the SSE `message_start` event arrives (line 662). If the request fails before that event — or if the event never carries usage — the status bar retains the previous turn's `LastContextTokens`, misrepresenting the current context fill. The fix is to zero `a.LastContextTokens` at the start of `callStreamingAPI`, matching the CC backend's contract.

Example:
Turn 1: API succeeds, LastContextTokens = 12000. Turn 2: API fails before message_start. Status bar shows 'ctx 12.0k/200.0k (6%)' — the stale value from Turn 1.

Threat model:
A user sees a stale context-window percentage in the TUI status bar after a failed API call, leading to incorrect assumptions about how full the context window is.

Specific code citations:
CCAgent.Run resets lastTurnIn/Out/OK at line 361 of cc_agent.go. callStreamingAPI sets LastContextTokens at line 662 of agent.go but never resets it at the top of the function.

Existing protections:
The REPL's `lastContextTokens` variable is only updated on success (repl.go line 1195), but the Agent field itself is not cleared on failure, so a subsequent success that doesn't carry usage would still show the stale value.

Proposed mitigation:
Add `a.LastContextTokens = 0` at the top of `callStreamingAPI`, before the request is sent.

Alternative mitigations considered:
Resetting in the REPL after every turn would work but would miss the case where the Agent is used outside the REPL. Resetting in the Agent is the correct single point of truth.

Severity calibration:
Score 3: a stale UI metric, not a correctness or security bug. The blast radius is limited to the status bar display, and the likelihood is moderate (any failed API call triggers it).

Suggested fix shape:
a.LastContextTokens = 0

Why this wasn't caught:
No test exercises the LastContextTokens value after a failed API call.

How can I resolve this? If you propose a fix, please make it concise.

a.Usage.Requests++
if a.Cfg.Verbose {
fmt.Printf("[SSE] message_start: input_tokens=%d\n", ev.Message.Usage.InputTokens)
Expand Down Expand Up @@ -813,6 +827,7 @@

// Track usage
a.Usage.InputTokens += apiResp.Usage.InputTokens
a.LastContextTokens = apiResp.Usage.InputTokens

@sigilix sigilix Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 LOGICGROUNDED LastContextTokens is not reset at turn start in the direct API path, leaking the previous turn's value on failure

The CC backend resets its per-turn usage counters at the top of Run(). The direct API path (callAPI) sets a.LastContextTokens only after a successful response (line 822). If the HTTP request fails, times out, or returns a non-2xx status, LastContextTokens retains the previous turn's value. The fix is to zero a.LastContextTokens at the start of callAPI, matching the CC backend's contract.

Example:

Turn 1: direct API succeeds, LastContextTokens = 8000. Turn 2: HTTP 502. Status bar shows 'ctx 8.0k/200.0k (4%)' — stale.

Suggested fix:

a.LastContextTokens = 0

Why this wasn't caught: No test exercises the LastContextTokens value after a failed direct API call.

More Info
  • Threat model: Same as the streaming path: a stale context-window percentage in the TUI after a failed direct API call.
  • Specific code citations: CCAgent.Run resets at cc_agent.go line 361. callAPI sets LastContextTokens at agent.go line 822 but never resets it at the top.
  • Existing protections: The REPL only reads LastContextTokens on success (repl.go line 1195), but the field itself is not cleared on failure.
  • Proposed mitigation: Add a.LastContextTokens = 0 at the top of callAPI, before the HTTP request.
  • Alternative mitigations considered: Same as the streaming path — resetting in the REPL is less robust than resetting in the Agent.
  • Severity calibration: Score 3: stale UI metric, moderate likelihood on any failed direct API call.
Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/agent/agent.go
Line: 822

Comment:
**`LastContextTokens` is not reset at turn start in the direct API path, leaking the previous turn's value on failure**

The CC backend resets its per-turn usage counters at the top of `Run()`. The direct API path (`callAPI`) sets `a.LastContextTokens` only after a successful response (line 822). If the HTTP request fails, times out, or returns a non-2xx status, `LastContextTokens` retains the previous turn's value. The fix is to zero `a.LastContextTokens` at the start of `callAPI`, matching the CC backend's contract.

Example:
Turn 1: direct API succeeds, LastContextTokens = 8000. Turn 2: HTTP 502. Status bar shows 'ctx 8.0k/200.0k (4%)' — stale.

Threat model:
Same as the streaming path: a stale context-window percentage in the TUI after a failed direct API call.

Specific code citations:
CCAgent.Run resets at cc_agent.go line 361. callAPI sets LastContextTokens at agent.go line 822 but never resets it at the top.

Existing protections:
The REPL only reads LastContextTokens on success (repl.go line 1195), but the field itself is not cleared on failure.

Proposed mitigation:
Add `a.LastContextTokens = 0` at the top of `callAPI`, before the HTTP request.

Alternative mitigations considered:
Same as the streaming path — resetting in the REPL is less robust than resetting in the Agent.

Severity calibration:
Score 3: stale UI metric, moderate likelihood on any failed direct API call.

Suggested fix shape:
a.LastContextTokens = 0

Why this wasn't caught:
No test exercises the LastContextTokens value after a failed direct API call.

How can I resolve this? If you propose a fix, please make it concise.

a.Usage.OutputTokens += apiResp.Usage.OutputTokens
a.Usage.Requests++

Expand Down
36 changes: 36 additions & 0 deletions internal/agent/cc_agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ type CLIAgent interface {
Cleanup()
}

// TurnStatsProvider is optionally implemented by CLI agents that can report
// token usage for their most recent Run. The REPL folds these numbers into
// the session totals shown in the input status bar; backends whose stream
// carries no usage information simply don't implement it (or return ok=false).
type TurnStatsProvider interface {
LastTurnStats() (inputTokens, outputTokens int, ok bool)
}

// CCAgent orchestrates a Claude Code CLI subprocess for LLM inference.
// Inference runs through the user's Claude Code login, so qmax-code does not
// need a QM-held Anthropic API key. Because this agent uses `claude --print`,
Expand All @@ -55,6 +63,9 @@ type CCAgent struct {
mcpConfigInfo os.FileInfo
sctx *api.SessionContext
lastToolName string // track last tool name for result display
lastTurnIn int // token usage of the most recent turn (from CC's result event)
lastTurnOut int
lastTurnOK bool // true once a result event carried usage this turn
mu sync.Mutex
runMu sync.Mutex
runCancel context.CancelFunc // non-nil while Run() is active
Expand Down Expand Up @@ -347,6 +358,7 @@ func sanitizeCCUserPrompt(prompt string) (string, error) {
// CC's subscription handles inference; qmax handles tools via MCP.
func (a *CCAgent) Run(userMsg string, term *tui.Terminal) (string, error) {
a.mu.Lock()
a.lastTurnIn, a.lastTurnOut, a.lastTurnOK = 0, 0, false
if !sameMCPConfigFile(a.mcpConfigPath, a.mcpConfigInfo) {
a.mu.Unlock()
if err := a.WriteMCPConfig(); err != nil {
Expand Down Expand Up @@ -500,6 +512,14 @@ type ccEvent struct {
Message *ccEventMsg `json:"message,omitempty"`
Result string `json:"result,omitempty"`
IsError bool `json:"is_error,omitempty"`
Usage *ccUsage `json:"usage,omitempty"`
}

// ccUsage is emitted on Claude Code's final result event. Keep the fields we
// surface in the UI typed, while allowing CC to add other usage fields.
type ccUsage struct {
InputTokens int `json:"input_tokens"`
OutputTokens int `json:"output_tokens"`
}

type ccEventMsg struct {
Expand Down Expand Up @@ -591,6 +611,13 @@ func (a *CCAgent) parseStream(stdout interface{ Read([]byte) (int, error) }, ter

case "result":
finalResult = event.Result
if event.Usage != nil {
a.mu.Lock()
a.lastTurnIn = event.Usage.InputTokens
a.lastTurnOut = event.Usage.OutputTokens
a.lastTurnOK = event.Usage.InputTokens > 0 || event.Usage.OutputTokens > 0
a.mu.Unlock()
}
if event.IsError && event.Result != "" {
term.PrintError("CC error: " + event.Result)
}
Expand All @@ -601,6 +628,15 @@ func (a *CCAgent) parseStream(stdout interface{ Read([]byte) (int, error) }, ter
return finalResult
}

// LastTurnStats returns Claude Code's usage from the most recently completed
// turn. It is intentionally optional so other CLI backends need not invent
// token figures when their protocol does not expose them.
func (a *CCAgent) LastTurnStats() (inputTokens, outputTokens int, ok bool) {
a.mu.Lock()
defer a.mu.Unlock()
return a.lastTurnIn, a.lastTurnOut, a.lastTurnOK
}

// parseCCBlocks unmarshals a CC message content field (string or []block).
func parseCCBlocks(raw json.RawMessage) []ccBlock {
if len(raw) == 0 {
Expand Down
1 change: 1 addition & 0 deletions internal/agent/cerebras_agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ func (a *Agent) RunCerebrasAgent(term *tui.Terminal) (string, bool) {

choice := resp.Choices[0]
a.Usage.InputTokens += resp.Usage.PromptTokens
a.LastContextTokens = resp.Usage.PromptTokens

@sigilix sigilix Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 LOGICGROUNDED LastContextTokens is not reset at turn start in the Cerebras path, leaking the previous turn's value on failure

The CC backend resets its per-turn usage counters at the top of Run(). The Cerebras path (RunCerebrasAgent) sets a.LastContextTokens only after a successful response (line 71). If the Cerebras request fails, LastContextTokens retains the previous turn's value. The fix is to zero a.LastContextTokens at the start of RunCerebrasAgent, matching the CC backend's contract.

Example:

Turn 1: Cerebras succeeds, LastContextTokens = 5000. Turn 2: Cerebras API error. Status bar shows 'ctx 5.0k/200.0k (2%)' — stale.

Suggested fix:

a.LastContextTokens = 0

Why this wasn't caught: No test exercises the LastContextTokens value after a failed Cerebras call.

More Info
  • Threat model: Stale context-window percentage in the TUI after a failed Cerebras call.
  • Specific code citations: CCAgent.Run resets at cc_agent.go line 361. RunCerebrasAgent sets LastContextTokens at cerebras_agent.go line 71 but never resets it at the top.
  • Existing protections: The REPL only reads LastContextTokens on success (repl.go line 1195), but the field itself is not cleared on failure.
  • Proposed mitigation: Add a.LastContextTokens = 0 at the top of RunCerebrasAgent, before the request loop.
  • Alternative mitigations considered: Resetting in the REPL is less robust than resetting in the Agent.
  • Severity calibration: Score 3: stale UI metric, moderate likelihood on any failed Cerebras call.
Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/agent/cerebras_agent.go
Line: 71

Comment:
**`LastContextTokens` is not reset at turn start in the Cerebras path, leaking the previous turn's value on failure**

The CC backend resets its per-turn usage counters at the top of `Run()`. The Cerebras path (`RunCerebrasAgent`) sets `a.LastContextTokens` only after a successful response (line 71). If the Cerebras request fails, `LastContextTokens` retains the previous turn's value. The fix is to zero `a.LastContextTokens` at the start of `RunCerebrasAgent`, matching the CC backend's contract.

Example:
Turn 1: Cerebras succeeds, LastContextTokens = 5000. Turn 2: Cerebras API error. Status bar shows 'ctx 5.0k/200.0k (2%)' — stale.

Threat model:
Stale context-window percentage in the TUI after a failed Cerebras call.

Specific code citations:
CCAgent.Run resets at cc_agent.go line 361. RunCerebrasAgent sets LastContextTokens at cerebras_agent.go line 71 but never resets it at the top.

Existing protections:
The REPL only reads LastContextTokens on success (repl.go line 1195), but the field itself is not cleared on failure.

Proposed mitigation:
Add `a.LastContextTokens = 0` at the top of `RunCerebrasAgent`, before the request loop.

Alternative mitigations considered:
Resetting in the REPL is less robust than resetting in the Agent.

Severity calibration:
Score 3: stale UI metric, moderate likelihood on any failed Cerebras call.

Suggested fix shape:
a.LastContextTokens = 0

Why this wasn't caught:
No test exercises the LastContextTokens value after a failed Cerebras call.

How can I resolve this? If you propose a fix, please make it concise.

a.Usage.OutputTokens += resp.Usage.CompletionTokens
a.Usage.Requests++

Expand Down
15 changes: 15 additions & 0 deletions internal/api/models.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,18 @@ func IsValidClaudeModelName(m string) bool {
func ValidClaudeModelsHelp() string {
return "auto, sonnet, opus, haiku, " + ModelFable + ", " + ModelSonnet5 + ", " + ModelSonnet + ", " + ModelOpus + ", " + ModelOpus1M + ", " + ModelOpus47 + ", " + ModelHaiku
}

// ContextWindow returns the assumed context-window size in tokens for a model
// ID across all backends. It feeds the status-bar fill estimate only, so a
// conservative 200k default for unknown (non-Anthropic) models is acceptable.
func ContextWindow(model string) int {
m := strings.ToLower(model)
switch {
case strings.Contains(m, "[1m]"):
return 1_000_000
case m == ModelFable, m == ModelSonnet5:
return 1_000_000
default:
return 200_000
}
}
15 changes: 15 additions & 0 deletions internal/api/models_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,18 @@ func TestIsValidClaudeModelNameIncludesFableAndSonnet5(t *testing.T) {
}
}
}

func TestContextWindow(t *testing.T) {
for _, tc := range []struct {
model string
want int
}{
{ModelSonnet5, 1_000_000},
{"claude-opus-4-6[1m]", 1_000_000},
{"unknown-model", 200_000},
} {
if got := ContextWindow(tc.model); got != tc.want {
t.Errorf("ContextWindow(%q) = %d, want %d", tc.model, got, tc.want)
}
}
}
60 changes: 59 additions & 1 deletion internal/repl/repl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,42 @@

var inputHistory []string
var lastCtrlC time.Time
sessionStarted := time.Now()

@sigilix sigilix Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 LOGICGROUNDED LastContextTokens is set from different sources in API vs CLI paths, breaking the declared contract

The Agent.LastContextTokens field is documented as 'the input-token count reported by the most recent model request' and is set directly in callStreamingAPI, callAPI, and RunCerebrasAgent. However, in the CLI-agent path (CC/Codex), lastContextTokens is instead set from TurnStatsProvider.LastTurnStats(), which returns the input tokens from the CLI subprocess's result event. This means ag.LastContextTokens is stale (zero or from a prior API call) when a CLI backend is active, while the REPL's local lastContextTokens variable holds the correct value. The inputStatus closure reads lastContextTokens into StatusInfo.ContextUsed, so the display is correct, but the Agent field's contract is violated — it does not reflect the most recent model request when a CLI backend is used.

More Info
  • Threat model: Any code that reads ag.LastContextTokens expecting it to reflect the most recent request (as documented) will get a stale or zero value when a CLI backend is active. This is a latent correctness bug for future consumers of the field.
  • Specific code citations: internal/agent/agent.go:66-68 declares LastContextTokens with the contract 'input-token count reported by the most recent model request'. internal/agent/agent.go:662 and internal/agent/agent.go:822 set it in the API paths. internal/repl/repl.go:1164-1170 sets the local lastContextTokens from TurnStatsProvider but never updates ag.LastContextTokens.
  • Existing protections: The REPL's inputStatus closure reads the local lastContextTokens variable, not ag.LastContextTokens, so the TUI display is correct. No other code in this diff reads ag.LastContextTokens.
  • Proposed mitigation: After the TurnStatsProvider block in the REPL, also set ag.LastContextTokens = inputTokens so the field stays consistent with its documented contract.
  • Alternative mitigations considered: Remove the LastContextTokens field from Agent and rely solely on the REPL-local variable, but that would break the API path's direct usage. Keeping the field and updating it in both paths is the minimal fix.
  • Severity calibration: Score 3 because the TUI display is correct (the local variable is used), but the documented field contract is broken, creating a latent bug for any future code that reads ag.LastContextTokens.
Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/repl/repl.go
Line: 165

Comment:
**`LastContextTokens` is set from different sources in API vs CLI paths, breaking the declared contract**

The `Agent.LastContextTokens` field is documented as 'the input-token count reported by the most recent model request' and is set directly in `callStreamingAPI`, `callAPI`, and `RunCerebrasAgent`. However, in the CLI-agent path (CC/Codex), `lastContextTokens` is instead set from `TurnStatsProvider.LastTurnStats()`, which returns the input tokens from the CLI subprocess's result event. This means `ag.LastContextTokens` is stale (zero or from a prior API call) when a CLI backend is active, while the REPL's local `lastContextTokens` variable holds the correct value. The `inputStatus` closure reads `lastContextTokens` into `StatusInfo.ContextUsed`, so the display is correct, but the `Agent` field's contract is violated — it does not reflect the most recent model request when a CLI backend is used.

Threat model:
Any code that reads `ag.LastContextTokens` expecting it to reflect the most recent request (as documented) will get a stale or zero value when a CLI backend is active. This is a latent correctness bug for future consumers of the field.

Specific code citations:
`internal/agent/agent.go:66-68` declares `LastContextTokens` with the contract 'input-token count reported by the most recent model request'. `internal/agent/agent.go:662` and `internal/agent/agent.go:822` set it in the API paths. `internal/repl/repl.go:1164-1170` sets the local `lastContextTokens` from `TurnStatsProvider` but never updates `ag.LastContextTokens`.

Existing protections:
The REPL's `inputStatus` closure reads the local `lastContextTokens` variable, not `ag.LastContextTokens`, so the TUI display is correct. No other code in this diff reads `ag.LastContextTokens`.

Proposed mitigation:
After the `TurnStatsProvider` block in the REPL, also set `ag.LastContextTokens = inputTokens` so the field stays consistent with its documented contract.

Alternative mitigations considered:
Remove the `LastContextTokens` field from `Agent` and rely solely on the REPL-local variable, but that would break the API path's direct usage. Keeping the field and updating it in both paths is the minimal fix.

Severity calibration:
Score 3 because the TUI display is correct (the local variable is used), but the documented field contract is broken, creating a latent bug for any future code that reads `ag.LastContextTokens`.

How can I resolve this? If you propose a fix, please make it concise.

var lastTask string
var lastTurnDur time.Duration
var lastContextTokens int

inputStatus := func() *tui.StatusInfo {
backend := "api"
if ag.Cfg.Context != nil && ag.Cfg.Context.Backend != "" {
backend = ag.Cfg.Context.Backend
}
model := ag.Cfg.Model
effort, permissionMode := "", ""
if ag.AppConfig != nil {
if ag.AppConfig.ModelOverride != "" {
model = ag.AppConfig.ModelOverride
}
effort = ag.AppConfig.Effort
if backend != "api" {
permissionMode = ag.AppConfig.OrchPermissionMode
}
}
return &tui.StatusInfo{
Backend: backend,
Model: model,
Effort: effort,
PermissionMode: permissionMode,
OutputVerbose: ag.Cfg.OutputVerbose,
Task: lastTask,
TokensIn: ag.Usage.InputTokens,
TokensOut: ag.Usage.OutputTokens,
ContextUsed: lastContextTokens,
ContextWindow: api.ContextWindow(model),
LastTurnDur: lastTurnDur,
SessionStarted: sessionStarted,
}
}

// Prompt consent and open cloud session at startup (idempotent — safe to call again later).
startCloudSession()
Expand All @@ -184,7 +220,7 @@
fmt.Println()
inputHistory = append(inputHistory, input)
} else {
result := tui.ReadInput(term.Prompt(), inputHistory, ag.Cfg.OutputVerbose)
result := tui.ReadInput(term.Prompt(), inputHistory, ag.Cfg.OutputVerbose, inputStatus())

if result.OutputToggle {
toggleOutputVerbose(ag, cliAgent, term)
Expand Down Expand Up @@ -222,6 +258,7 @@
continue
case input == "/clear":
ag.ClearHistory()
lastContextTokens = 0
if oc, ok := cliAgent.(*agent.OpenCodeAgent); ok {
oc.ClearSession()
}
Expand Down Expand Up @@ -1058,6 +1095,11 @@
// Normal mode: use direct Anthropic API.
var llmResult string
var err error
// Clear the UI's per-turn estimate before starting any backend. This also
// covers CLI backends, whose usage is reported outside Agent.RunStreaming.
lastContextTokens = 0
Comment on lines +1098 to +1100

@sigilix sigilix Bot Jul 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 LOGICGROUNDED Race on Agent.LastContextTokens between REPL and agent goroutines

ag.LastContextTokens is written by the agent's streaming goroutine (in callStreamingAPI when processing SSE message_start events) and read by the REPL goroutine (in the inputStatus closure and on line 1200). There is no synchronization — no mutex, no atomic, no channel. In Go, concurrent reads and writes to a plain int field are a data race. The fix is to protect LastContextTokens with a mutex or use atomic.Int64.

More Info
  • Threat model: Under the Go memory model, concurrent read/write of a non-atomic int is undefined behavior. The race detector will flag it, and in practice the TUI could read a torn or stale value.
  • Specific code citations: Write: internal/agent/agent.go line 670 (a.LastContextTokens = ev.Message.Usage.InputTokens) inside callStreamingAPI. Read: internal/repl/repl.go line 1200 (lastContextTokens = ag.LastContextTokens) and line 192 (ContextUsed: lastContextTokens).
  • Existing protections: None — no mutex, no atomic, no channel.
  • Proposed mitigation: Change LastContextTokens to atomic.Int64 in the Agent struct, or wrap accesses with a mutex.
  • Alternative mitigations considered: Could pass the value through a channel, but an atomic is simpler for a single scalar.
  • Severity calibration: Score 4 — a real data race that the Go race detector will catch. In practice the impact is a stale/zero display, but it's a correctness bug in concurrent code.
Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/repl/repl.go
Line: 1098-1100

Comment:
**Race on Agent.LastContextTokens between REPL and agent goroutines**

`ag.LastContextTokens` is written by the agent's streaming goroutine (in `callStreamingAPI` when processing SSE `message_start` events) and read by the REPL goroutine (in the `inputStatus` closure and on line 1200). There is no synchronization — no mutex, no atomic, no channel. In Go, concurrent reads and writes to a plain `int` field are a data race. The fix is to protect `LastContextTokens` with a mutex or use `atomic.Int64`.

Threat model:
Under the Go memory model, concurrent read/write of a non-atomic int is undefined behavior. The race detector will flag it, and in practice the TUI could read a torn or stale value.

Specific code citations:
Write: `internal/agent/agent.go` line 670 (`a.LastContextTokens = ev.Message.Usage.InputTokens`) inside `callStreamingAPI`. Read: `internal/repl/repl.go` line 1200 (`lastContextTokens = ag.LastContextTokens`) and line 192 (`ContextUsed: lastContextTokens`).

Existing protections:
None — no mutex, no atomic, no channel.

Proposed mitigation:
Change `LastContextTokens` to `atomic.Int64` in the `Agent` struct, or wrap accesses with a mutex.

Alternative mitigations considered:
Could pass the value through a channel, but an atomic is simpler for a single scalar.

Severity calibration:
Score 4 — a real data race that the Go race detector will catch. In practice the impact is a stale/zero display, but it's a correctness bug in concurrent code.

How can I resolve this? If you propose a fix, please make it concise.

@sigilix sigilix Bot Jul 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 ERRORGROUNDED CLI-backend error path leaves lastContextTokens and ag.LastContextTokens stale

The REPL's main turn loop resets lastContextTokens and ag.LastContextTokens to zero at line 1098 before starting any backend. For CLI backends (cc/codex), the success path updates both fields from TurnStatsProvider at lines 1168-1173. However, if cliAgent.Run returns an error (line 1165), the code falls through to line 1198 where lastTurnDur is recorded, but neither lastContextTokens nor ag.LastContextTokens is updated — they remain zero from the pre-turn reset. This is consistent with the convention that zero means "no usage reported this turn," but the success path for the direct-API backend (line 1200) explicitly copies ag.LastContextTokens into lastContextTokens only when err == nil. The CLI-backend error path does not mirror this guard, making the contract implicit. If a future change reads lastContextTokens after a failed CLI turn, it would see zero and could misinterpret it as "the model used no context" rather than "the turn failed."

Example:

input: CLI backend (cc) turn that fails mid-request
actual: lastContextTokens = 0, ag.LastContextTokens = 0 (from pre-turn reset)
expected: same values, but the contract is implicit — a reader might expect a sentinel or explicit guard

Suggested fix:

// After the CLI error path, before lastTurnDur:
// lastContextTokens intentionally remains 0 — this turn produced no usage.

Why this wasn't caught: No test exercises a failed CLI-backend turn and asserts the context-token fields remain zero.

More Info
  • Threat model: A future UI change that displays lastContextTokens after a failed turn would show zero, misleading the user into thinking the model context was empty when the turn actually failed before reporting usage.
  • Specific code citations: REPL turn loop (repl.go:1095-1205): reset at line 1098, CLI success path at lines 1168-1173, CLI error path falls through to line 1198 without updating context fields. Direct-API success path at line 1200 explicitly guards the copy with err == nil.
  • Existing protections: The inputStatus closure reads lastContextTokens only when constructing the status for the next input prompt, which only happens after a successful turn. The stale zero is not currently observable.
  • Proposed mitigation: Add an explicit lastContextTokens = 0 (or leave the existing reset as-is) with a comment noting that zero means 'no usage reported' on both success-without-stats and failure paths.
  • Alternative mitigations considered: A sentinel value like -1 would distinguish 'no report' from 'zero tokens,' but would require changes to the TUI rendering. The current zero convention is simpler and sufficient if documented.
  • Severity calibration: Score 3 because the inconsistency is latent — no current code path reads the stale value — but the contract is implicit and fragile.
Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/repl/repl.go
Line: 1100

Comment:
**CLI-backend error path leaves `lastContextTokens` and `ag.LastContextTokens` stale**

The REPL's main turn loop resets `lastContextTokens` and `ag.LastContextTokens` to zero at line 1098 before starting any backend. For CLI backends (cc/codex), the success path updates both fields from `TurnStatsProvider` at lines 1168-1173. However, if `cliAgent.Run` returns an error (line 1165), the code falls through to line 1198 where `lastTurnDur` is recorded, but neither `lastContextTokens` nor `ag.LastContextTokens` is updated — they remain zero from the pre-turn reset. This is consistent with the convention that zero means "no usage reported this turn," but the success path for the direct-API backend (line 1200) explicitly copies `ag.LastContextTokens` into `lastContextTokens` only when `err == nil`. The CLI-backend error path does not mirror this guard, making the contract implicit. If a future change reads `lastContextTokens` after a failed CLI turn, it would see zero and could misinterpret it as "the model used no context" rather than "the turn failed."

Example:
input: CLI backend (cc) turn that fails mid-request
actual: lastContextTokens = 0, ag.LastContextTokens = 0 (from pre-turn reset)
expected: same values, but the contract is implicit — a reader might expect a sentinel or explicit guard

Threat model:
A future UI change that displays `lastContextTokens` after a failed turn would show zero, misleading the user into thinking the model context was empty when the turn actually failed before reporting usage.

Specific code citations:
REPL turn loop (repl.go:1095-1205): reset at line 1098, CLI success path at lines 1168-1173, CLI error path falls through to line 1198 without updating context fields. Direct-API success path at line 1200 explicitly guards the copy with `err == nil`.

Existing protections:
The `inputStatus` closure reads `lastContextTokens` only when constructing the status for the next input prompt, which only happens after a successful turn. The stale zero is not currently observable.

Proposed mitigation:
Add an explicit `lastContextTokens = 0` (or leave the existing reset as-is) with a comment noting that zero means 'no usage reported' on both success-without-stats and failure paths.

Alternative mitigations considered:
A sentinel value like -1 would distinguish 'no report' from 'zero tokens,' but would require changes to the TUI rendering. The current zero convention is simpler and sufficient if documented.

Severity calibration:
Score 3 because the inconsistency is latent — no current code path reads the stale value — but the contract is implicit and fragile.

Suggested fix shape:
// After the CLI error path, before lastTurnDur:
// lastContextTokens intentionally remains 0 — this turn produced no usage.

Why this wasn't caught:
No test exercises a failed CLI-backend turn and asserts the context-token fields remain zero.

How can I resolve this? If you propose a fix, please make it concise.

ag.LastContextTokens = 0
turnStarted := time.Now()

// In CC/Codex mode, start a pre-connect goroutine that watches for a
// live_browser_url in the side-channel file every second. When one
Expand All @@ -1070,15 +1112,15 @@
stream *vnc.VNCStream // nil on dial failure
}
preConnChan := make(chan preConnResult, 1)
// watchCtx is only used to stop the polling ticker (abort a pending
// drainLiveURLFromChild loop). It is intentionally NOT passed to
// DialVNC — the stream needs a context that outlives the goroutine.
watchCtx, watchCancel := context.WithCancel(context.Background())
defer watchCancel() // ensures cancel on any return/continue path
if cliAgent != nil {
go func() {
// Do NOT defer watchCancel here — it is called by the main
// goroutine after the agent turn ends. If we cancelled it here

Check warning on line 1123 in internal/repl/repl.go

View check run for this annotation

Sigilix / Sigilix

[P1] security: Missing authorization check before updating agent usage from CLI backend stats 

The REPL updates ag.Usage and lastContextTokens based on cliAgent.(agent.TurnStatsProvider).LastTurnStats() without verifying the CLI backend's reported token counts are authorized for the current session. A malicious or compromised CLI backend could inflate usage metrics, affecting billing estimates and session context-window calculations. The fix is to verify the CLI backend's identity or scope before accepting its stats.
More Info

- Threat model: A compromised or malicious CLI backend (e.g., a tampered claude binary) could report arbitrary token counts, skewing the agent's internal usage tracking. This could lead to incorrect context-window occupancy estimates (affecting UI display) and misreporting of token consumption for billing/credit tracking.
- Specific code citations: Lines 1123-1132 in internal/repl/repl.go call stats.LastTurnStats() and update ag.Usage and lastContextTokens unconditionally.
- Existing protections: None; the code trusts any CLIAgent that implements TurnStatsProvider.
- Proposed mitigation: Add a check that the CLI backend is the currently active backend for this session (e.g., compare cliBackend variable or a session-scoped identifier) before accepting its stats. Alternatively, only accept stats from backends that were explicitly configured and validated at startup.
- Alternative mitigations considered: Ignore CLI-reported stats entirely and rely solely on the agent's own usage tracking; however, some backends (like CC) may have accurate token counts that improve UX. Another option is to sign stats with a session-specific nonce, but that adds complexity.
- Severity calibration: Score 4 because exploitation requires a compromised CLI backend (specific condition), but the impact is a integrity violation of session metrics that could affect user trust and billing.

Example:
A tampered claude binary returns LastTurnStats() with inputTokens: 999999, outputTokens: 999999, reported: true. The UI shows a context window 99% full, and the session's token count is inflated.

Suggested fix:
if cliAgent != nil && cliBackend == "cc" { // only accept stats from the active backend
    if stats, ok := cliAgent.(agent.TurnStatsProvider); ok {
        if inputTokens, outputTokens, reported := stats.LastTurnStats(); reported {
            // update usage
        }
    }
}
Lines 1115-1123 (high, score 4) · internal/repl/repl.go:1115-1123
// on exit, DialVNC (which uses context.Background()) is fine,
// but stopping the ticker via Done() would still work.
ticker := time.NewTicker(1 * time.Second)
Expand Down Expand Up @@ -1123,6 +1165,15 @@
llmResult, err = cliAgent.Run(cleanInput, term)
term.StopThinking()
if err == nil {
if stats, ok := cliAgent.(agent.TurnStatsProvider); ok {

@sigilix sigilix Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 LOGICGROUNDED Direct-API backend silently omits per-turn status metrics that CC/Codex backends report

The TurnStatsProvider interface is only implemented by CCAgent. The REPL's post-turn block (line ~1164) calls cliAgent.(agent.TurnStatsProvider) only when cliAgent != nil, which is true for CC/Codex but false for the direct API path. The direct-API Agent struct has a LastContextTokens field populated during API calls, but it does not implement TurnStatsProvider, so the status bar's per-turn token counts and context-window estimate are silently absent when using the API backend. The inputStatus closure already reads ag.LastContextTokens directly, so the data is available — the gap is that the REPL never updates lastContextTokens from ag.LastContextTokens for the API path in the same structured way it does for CLI backends. Fix: either have the direct-API Agent implement TurnStatsProvider (returning a.LastContextTokens, a.Usage.OutputTokens - previousOutput, true), or add an explicit API-path branch in the REPL that reads ag.LastContextTokens and updates lastContextTokens.

More Info
  • Threat model: Users on the direct API backend see no per-turn context-window fill or token counts in the status bar, while CC/Codex users do. This is a UX inconsistency, not a correctness bug — no data is lost, but the feature is silently gated on backend choice.
  • Specific code citations: TurnStatsProvider interface defined in internal/agent/cc_agent.go:32-35. Implemented only by CCAgent at cc_agent.go:631-635. REPL calls it at repl.go:1164-1172 inside if cliAgent != nil. Direct-API Agent has LastContextTokens at agent.go:66-69 but no LastTurnStats() method.
  • Existing protections: None. The inputStatus closure reads ag.LastContextTokens directly, but the REPL's lastContextTokens local is only updated via the TurnStatsProvider path (CLI backends) or the else branch at line ~1193 (which sets it from ag.LastContextTokens when cliAgent == nil). The else branch at line 1193 does update lastContextTokens from ag.LastContextTokens, so the status bar WILL show context usage for the API path — but only after the turn completes, and only via the lastContextTokens local, not via the TurnStatsProvider interface. The asymmetry is that the API path has no TurnStatsProvider implementation, making the interface a partial abstraction.
  • Proposed mitigation: Either implement TurnStatsProvider on *Agent (returning a.LastContextTokens, a.Usage.OutputTokens - previousOutput, true) so the REPL's single cliAgent.(agent.TurnStatsProvider) branch covers all backends, or add an explicit else branch in the REPL that reads ag.LastContextTokens and updates lastContextTokens for the API path. The latter already exists at line 1193, so the actual gap is narrower than it first appears.
  • Alternative mitigations considered: Remove TurnStatsProvider and have the REPL always read ag.LastContextTokens directly. This is simpler but loses the CC-specific output-token reporting.
  • Severity calibration: Score 3: the API path does update lastContextTokens at line 1193, so the status bar is not completely broken — but the TurnStatsProvider interface is a partial abstraction that only one backend implements, which is a design smell that will cause confusion when adding new backends.
Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/repl/repl.go
Line: 1164

Comment:
**Direct-API backend silently omits per-turn status metrics that CC/Codex backends report**

The `TurnStatsProvider` interface is only implemented by `CCAgent`. The REPL's post-turn block (line ~1164) calls `cliAgent.(agent.TurnStatsProvider)` only when `cliAgent != nil`, which is true for CC/Codex but false for the direct API path. The direct-API `Agent` struct has a `LastContextTokens` field populated during API calls, but it does not implement `TurnStatsProvider`, so the status bar's per-turn token counts and context-window estimate are silently absent when using the API backend. The `inputStatus` closure already reads `ag.LastContextTokens` directly, so the data is available — the gap is that the REPL never updates `lastContextTokens` from `ag.LastContextTokens` for the API path in the same structured way it does for CLI backends. Fix: either have the direct-API `Agent` implement `TurnStatsProvider` (returning `a.LastContextTokens, a.Usage.OutputTokens - previousOutput, true`), or add an explicit API-path branch in the REPL that reads `ag.LastContextTokens` and updates `lastContextTokens`.

Threat model:
Users on the direct API backend see no per-turn context-window fill or token counts in the status bar, while CC/Codex users do. This is a UX inconsistency, not a correctness bug — no data is lost, but the feature is silently gated on backend choice.

Specific code citations:
`TurnStatsProvider` interface defined in `internal/agent/cc_agent.go:32-35`. Implemented only by `CCAgent` at `cc_agent.go:631-635`. REPL calls it at `repl.go:1164-1172` inside `if cliAgent != nil`. Direct-API `Agent` has `LastContextTokens` at `agent.go:66-69` but no `LastTurnStats()` method.

Existing protections:
None. The `inputStatus` closure reads `ag.LastContextTokens` directly, but the REPL's `lastContextTokens` local is only updated via the `TurnStatsProvider` path (CLI backends) or the `else` branch at line ~1193 (which sets it from `ag.LastContextTokens` when `cliAgent == nil`). The `else` branch at line 1193 does update `lastContextTokens` from `ag.LastContextTokens`, so the status bar WILL show context usage for the API path — but only after the turn completes, and only via the `lastContextTokens` local, not via the `TurnStatsProvider` interface. The asymmetry is that the API path has no `TurnStatsProvider` implementation, making the interface a partial abstraction.

Proposed mitigation:
Either implement `TurnStatsProvider` on `*Agent` (returning `a.LastContextTokens, a.Usage.OutputTokens - previousOutput, true`) so the REPL's single `cliAgent.(agent.TurnStatsProvider)` branch covers all backends, or add an explicit `else` branch in the REPL that reads `ag.LastContextTokens` and updates `lastContextTokens` for the API path. The latter already exists at line 1193, so the actual gap is narrower than it first appears.

Alternative mitigations considered:
Remove `TurnStatsProvider` and have the REPL always read `ag.LastContextTokens` directly. This is simpler but loses the CC-specific output-token reporting.

Severity calibration:
Score 3: the API path does update `lastContextTokens` at line 1193, so the status bar is not completely broken — but the `TurnStatsProvider` interface is a partial abstraction that only one backend implements, which is a design smell that will cause confusion when adding new backends.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 1165 to +1168

@sigilix sigilix Bot Jul 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 LOGICGROUNDED CLI agent usage double-counts if the agent also reports usage via Agent.Usage

Low-confidence finding — expand to read

When a CLI agent (CC/Codex) implements TurnStatsProvider, the REPL manually adds the reported tokens to ag.Usage.InputTokens and ag.Usage.OutputTokens (lines 1168-1171). If the CLI agent's Run method also updates ag.Usage internally (e.g., by calling into the shared agent code), the tokens would be double-counted. The diff does not show the CLI agent implementations, so this cannot be confirmed, but the pattern is fragile.

More Info
  • Threat model: Token counts in the status panel are inflated, showing more usage than actually occurred.
  • Specific code citations: internal/repl/repl.go lines 1168-1171: ag.Usage.InputTokens += inputTokens etc.
  • Existing protections: None visible in the diff — the CLI agent implementations are not shown.
  • Proposed mitigation: Either have the CLI agent update ag.Usage directly (and not return stats), or have the REPL be the sole updater. Document the contract.
  • Alternative mitigations considered: Could add a boolean flag to TurnStatsProvider indicating whether the agent already updated usage.
  • Severity calibration: Score 3 — potential double-counting, but the CLI agent implementations are not in the diff, so this is a design concern rather than a confirmed bug.
Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/repl/repl.go
Line: 1165-1168

Comment:
**CLI agent usage double-counts if the agent also reports usage via Agent.Usage**

When a CLI agent (CC/Codex) implements `TurnStatsProvider`, the REPL manually adds the reported tokens to `ag.Usage.InputTokens` and `ag.Usage.OutputTokens` (lines 1168-1171). If the CLI agent's `Run` method also updates `ag.Usage` internally (e.g., by calling into the shared agent code), the tokens would be double-counted. The diff does not show the CLI agent implementations, so this cannot be confirmed, but the pattern is fragile.

Threat model:
Token counts in the status panel are inflated, showing more usage than actually occurred.

Specific code citations:
`internal/repl/repl.go` lines 1168-1171: `ag.Usage.InputTokens += inputTokens` etc.

Existing protections:
None visible in the diff — the CLI agent implementations are not shown.

Proposed mitigation:
Either have the CLI agent update `ag.Usage` directly (and not return stats), or have the REPL be the sole updater. Document the contract.

Alternative mitigations considered:
Could add a boolean flag to `TurnStatsProvider` indicating whether the agent already updated usage.

Severity calibration:
Score 3 — potential double-counting, but the CLI agent implementations are not in the diff, so this is a design concern rather than a confirmed bug.

How can I resolve this? If you propose a fix, please make it concise.

if inputTokens, outputTokens, reported := stats.LastTurnStats(); reported {
ag.Usage.InputTokens += inputTokens
ag.Usage.OutputTokens += outputTokens
ag.Usage.Requests++
lastContextTokens = inputTokens
ag.LastContextTokens = inputTokens
}
}
// Mirror the turn into ag.History so autoSave records it.
// agent.CCAgent/agent.CodexAgent manage their own subprocess state; qmax's
// history would otherwise stay empty and autoSave would no-op.
Expand All @@ -1144,6 +1195,13 @@
} else {
llmResult, err = ag.RunStreaming(input, term)
}
lastTurnDur = time.Since(turnStarted)

@sigilix sigilix Bot Jul 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 LOGICGROUNDED Stale lastContextTokens after a failed turn

After a failed turn (err != nil), lastContextTokens is not updated, so the status panel continues to show the context usage from the previous successful turn. This is misleading — the user sees a context-fill percentage that does not reflect the current state. Fix: set lastContextTokens = 0 in the error path, matching the reset done at the start of each turn.

More Info
  • Threat model: The user sees a stale context-fill percentage after a failed request, which could lead them to believe the context window is more full than it actually is.
  • Specific code citations: internal/repl/repl.go lines 1195-1201: the if err == nil block updates lastTask and lastContextTokens; the error path does not.
  • Existing protections: None — the error path falls through without touching lastContextTokens.
  • Proposed mitigation: Add lastContextTokens = 0 (or ag.LastContextTokens) in an else branch or before the if block.
  • Alternative mitigations considered: Could also set lastContextTokens = ag.LastContextTokens unconditionally after the turn, but that would require ensuring ag.LastContextTokens is zeroed on failure.
  • Severity calibration: Score 3 — a minor UX issue (stale display) with no data loss or crash.
Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/repl/repl.go
Line: 1198

Comment:
**Stale lastContextTokens after a failed turn**

After a failed turn (err != nil), lastContextTokens is not updated, so the status panel continues to show the context usage from the previous successful turn. This is misleading — the user sees a context-fill percentage that does not reflect the current state. Fix: set lastContextTokens = 0 in the error path, matching the reset done at the start of each turn.

Threat model:
The user sees a stale context-fill percentage after a failed request, which could lead them to believe the context window is more full than it actually is.

Specific code citations:
internal/repl/repl.go lines 1195-1201: the if err == nil block updates lastTask and lastContextTokens; the error path does not.

Existing protections:
None — the error path falls through without touching lastContextTokens.

Proposed mitigation:
Add lastContextTokens = 0 (or ag.LastContextTokens) in an else branch or before the if block.

Alternative mitigations considered:
Could also set lastContextTokens = ag.LastContextTokens unconditionally after the turn, but that would require ensuring ag.LastContextTokens is zeroed on failure.

Severity calibration:
Score 3 — a minor UX issue (stale display) with no data loss or crash.

How can I resolve this? If you propose a fix, please make it concise.

if err == nil {
lastTask = cleanInput
if cliAgent == nil {
lastContextTokens = ag.LastContextTokens
}
}

// Stop queue reader and wait for the goroutine to exit before we
// touch stdin again (either via the queue loop or tui.ReadInput).
Expand Down
Loading
Loading