Skip to content

fix(usage): replace inline casts with typeof guards in TUI renderer#3318

Open
oldschoola wants to merge 3 commits into
can1357:mainfrom
oldschoola:fix/usage-inline-cast-access
Open

fix(usage): replace inline casts with typeof guards in TUI renderer#3318
oldschoola wants to merge 3 commits into
can1357:mainfrom
oldschoola:fix/usage-inline-cast-access

Conversation

@oldschoola

Copy link
Copy Markdown
Contributor

Problem

The TUI usage renderer (command-controller.ts) used as string | undefined inline casts to read email/accountId/projectId/planType from UsageReport.metadata (Record<string, unknown>). These casts fabricated the type without runtime validation, violating the ts-no-inline-cast-access project rule.

Changes

Replaced all inline casts with typeof guards in 4 locations:

  • formatAccountLabel — limit + report identity resolution
  • formatUnlimitedReportLabel — report-only identity resolution
  • resetAccountLines label — saved-reset credits display
  • unlimitedReports tier — plan type suffix

Behavioral improvement

Empty-string metadata values ("") previously passed through the ?? chain and were displayed as the account label. With typeof guards, empty strings are falsy and fall through to the next fallback (scope.accountId, scope.projectId, or "account N").

This matches the pattern already used in the ACP path (usage-report.ts:formatUsageReportAccount).

Tests

  • bun check passes (all 16 packages)
  • All usage rendering tests pass (18/18)

@github-actions github-actions Bot added the vouched Passed the vouch gate label Jun 23, 2026
@roboomp roboomp added fix review:p1 triaged tui Terminal UI rendering and display labels Jun 23, 2026

@roboomp roboomp left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Rank: P1. Tight two-file fix and the unsafe casts are gone, but formatAccountLabel currently drops valid scoped fallbacks when metadata is "" or non-string.
One blocking inline finding on that fallback order; existing usage tests did not cover this TUI renderer edge. I attempted bun test packages/coding-agent/test/acp-builtins.test.ts -t usage, but this worktree failed before tests with missing ./tool-views.generated.js.
Thanks for the focused cleanup.

Comment on lines +1347 to +1349
const accountId = report.metadata?.accountId ?? limit.scope.accountId;
if (typeof accountId === "string" && accountId) return accountId;
const projectId = report.metadata?.projectId ?? limit.scope.projectId;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

blocking: ?? limit.scope.accountId / ?? limit.scope.projectId runs before the type and empty-string guard, so invalid metadata suppresses a valid scoped fallback. Example: metadata.accountId === "" with limit.scope.accountId === "acct-1" makes line 1347 produce "", line 1348 reject it, and the renderer falls through to projectId/account N instead of acct-1. The PR body says empty metadata falls through to scope.accountId/scope.projectId; guard the metadata value first, then fall back to limit.scope.*.

@oldschoola

Copy link
Copy Markdown
Contributor Author

Fixed in latest push. Empty-string metadata values now fall through to the scope fallback instead of suppressing it.

@oldschoola

Copy link
Copy Markdown
Contributor Author

Addressed blocking review in latest push (6d25917):

The ?? operator doesn't help when metadata.accountId === "" (empty string is not null/undefined, so it suppresses the scope fallback). Now the metadata value is checked for truthiness first, then falls back to limit.scope.accountId/limit.scope.projectId:

const metaAccountId = report.metadata?.accountId;
const accountId = typeof metaAccountId === string && metaAccountId ? metaAccountId : limit.scope.accountId;

The TUI usage renderer (command-controller.ts) used 'as string |
undefined' inline casts to read email/accountId/projectId/planType
from UsageReport.metadata (Record<string, unknown>). These casts
fabricated the type without runtime validation, violating the
ts-no-inline-cast-access rule.

Replaced all casts with typeof guards. This also fixes a behavioral
edge case: empty-string metadata values ('') previously passed
through the ?? chain and were displayed as the account label.
With the typeof guard, empty strings are falsy and fall through to
the next fallback (scope.accountId, scope.projectId, or 'account N').

Affected functions:
- formatAccountLabel (limit + report identity resolution)
- formatUnlimitedReportLabel (report-only identity resolution)
- resetAccountLines label (saved-reset credits display)
- unlimitedReports tier (plan type suffix)
Address blocking review: ?? limit.scope.accountId runs before the type
and empty-string guard, so metadata.accountId='' suppresses a valid
scoped fallback. Check metadata value for truthiness first, then fall
back to limit.scope.*.
@oldschoola oldschoola force-pushed the fix/usage-inline-cast-access branch from 6d25917 to 44f26be Compare June 23, 2026 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix review:p1 triaged tui Terminal UI rendering and display vouched Passed the vouch gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants