chore(frontend/views): medium/low findings from the 2026-07-28 full review

[cristim]

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have been fixed or moved.

Tracking issue for the MEDIUM and LOW findings from the 2026-07-28 full-repo review of the frontend view modules: recommendations.ts (5481 lines), settings.ts, plans.ts, riexchange.ts, history.ts, dashboard.ts, apikeys.ts, users/*, index.html, and tests-e2e/. Every innerHTML / insertAdjacentHTML assignment in scope (60 of the 115 repo-wide) was read end to end; every ?? 0 / || 0 / || <literal> hit in the six large view modules was triaged; every date-to-string conversion, every document/window listener registration, and every action-button permission gate was traced to its call site. The HIGH findings from the same pass are filed as individual issues.

Each checklist item is independently actionable. Please keep the file:line, failure scenario and fix direction with the item when splitting any of these out.

The recurring shape across most of these: a correct implementation of the exact pattern already exists elsewhere in the same module or its sibling, and this site did not adopt it. Every item below names the in-repo reference implementation, so none of them require a design decision.

Findings from this pass that are NOT in this list

Recorded so nothing looks dropped:

• Unescaped payment in the Opportunities table (recommendations.ts:2954): filed as its own HIGH issue.
• Unescaped purchase.payment in the planned-purchases term cell (plans.ts:690): filed as its own HIGH issue.
• default_coverage of 0 rewritten to 80 on the next Save (settings.ts:3184, :3192): filed as its own HIGH issue.
• Attribute-context injection on planId (plans.ts:2077, was :2034): evidence added to LeanerCloud/cloud-commitments-cli#1157, which already owns this exact site.
• history.ts date-range defaults and the API-key expiry default seeded from the UTC calendar day: evidence added to LeanerCloud/cloud-commitments-cli#1287, which already owns the toISOString().split('T')[0] sweep. The comment there also adds a new half not currently in Sweep toISOString().split('T')[0] -> toLocalDateInputValue across frontend date inputs #51's scope: the submit-side new Date(expiresAtInput) parse in apikeys.ts:191-195.

---

• No stale-response guard on the main list loaders, so a slow first request overwrites a newer render (MEDIUM)

  • Where: frontend/src/recommendations.ts:467-598 (loadRecommendations), frontend/src/history.ts:275-345 (loadHistory), frontend/src/plans.ts:~50-93 (loadPlans), frontend/src/riexchange.ts:252-278 (loadConvertibleRIs)
  • What: each loader awaits the API and then unconditionally writes module/global state and renders (state.setRecommendations(...) + renderRecommendationsList(...) at recommendations.ts:566-580; currentRIs = await …; renderRIsTable(…) at riexchange.ts:268-269). None carries a request-generation token. The coalescing microtask in setupRecommendationsHandlers (recommendations.ts:279-289) only collapses the two synchronous subscriber fires of a single chip change; it does nothing across two separate user actions. The module's own comment at :271-272 names "a stale-overwrite risk if the first response lands after the second" as the thing to avoid, and the sibling loadUtilization(generation) at riexchange.ts:312-316 implements exactly the guard that is missing here.
  • Failure scenario: an operator selects account A (a 4000-row tenant, ~6 s) then, a second later, account B (200 rows, ~0.4 s). B renders; A's response lands afterwards and overwrites state.setRecommendations and the table. The topbar chip reads "B" while the rows and the summary cards are A's. resolvePurchaseTarget() (recommendations.ts:3752-3756) reads that same clobbered state.getVisibleRecommendations(), so a Purchase click commits money against account A's recommendations while the UI says account B. This is a money-path consequence, not a cosmetic one.
  • Fix: add a module-scoped loadGeneration, increment it at loader entry, and re-check it before every state write and every render, mirroring riexchange.ts:312-316. Regression test must issue two overlapping loads with the slow-first ordering; a test with a single load or fast-first ordering passes with the bug present.

• Purchase-modal "Totals" Eff. % uses the AWS reconstruction the per-row helper explicitly refuses (MEDIUM)

  • Where: frontend/src/recommendations.ts:5165-5177 (updatePurchaseModalTotals), versus :1401-1416 (effectiveSavingsPct) and :1437-1443 (displaySavingsPct); consumed at :5244
  • What: effectiveSavingsPct returns null for AWS rows without on_demand_cost (:1409), with a comment stating that "the reconstruction formula (monthly_cost + savings + amortized) diverges from the true on-demand baseline … producing misleadingly high percentages. Return null so the UI renders '--' rather than a silently-wrong value." updatePurchaseModalTotals re-implements that denominator at :5169-5173 with no provider guard, folding exactly those discredited values into the weighted average at :5244. It also ignores the provider-authoritative savings_percentage that displaySavingsPct prefers for the per-row cells.
  • Failure scenario: a user selects AWS rows whose on_demand_cost is absent. Every per-row "Eff. %" cell shows — (an honest unknown), while the Totals row directly underneath prints a concrete weighted percentage derived from the formula the codebase declared wrong. The user authorises the purchase against a number the row-level code refused to display, and the two surfaces visibly disagree on the same screen.
  • Fix: call displaySavingsPct/effectiveSavingsPct from the totals accumulator (or replicate the provider === 'aws' && !hasOnDemand bail), and render — when the contributing denominator set is empty rather than a fabricated 0.

• Capacity scaling scales savings but not the on-demand baseline, understating Eff. % (MEDIUM)

  • Where: frontend/src/recommendations.ts:3917-3932 (handleBulkPurchaseClick), consumed at :5295-5365 (renderPurchaseModalRow)
  • What: the scaled clone multiplies count, upfront_cost, monthly_cost and savings by ratio, but the ...r spread carries on_demand_cost through unscaled. effectiveSavingsPct (:1412-1416) then divides the scaled numerator by the unscaled denominator.
  • Failure scenario: an operator sets Capacity to 50% and clicks Purchase on an Azure or GCP row that has on_demand_cost but no provider savings_percentage (so displaySavingsPct falls through to the reconstruction). The modal's Eff. % column shows roughly half the real percentage: a genuine 30% commitment renders as 15%, at the exact moment of authorisation. The error scales with the capacity setting, so it is largest for the partial purchases the control exists to enable.
  • Fix: scale on_demand_cost by the same ratio in the clone at :3922-3931. It is a per-rec total like savings and upfront_cost, not a per-unit rate. Regression test should assert Eff. % is invariant under the capacity ratio for a row with a known on_demand_cost.

• Reshape-recommendations table dereferences utilization_percent with no null guard (MEDIUM)

  • Where: frontend/src/riexchange.ts:1334 and :1342; sink at :1318
  • What: rec.utilization_percent >= 95 ? … and rec.utilization_percent.toFixed(1) are both called unguarded inside the .map() that builds the entire table's innerHTML. The sibling table 950 lines above handles the same field defensively (:374-376: utilPct === null ? '' : …, rendering a placeholder), and the module's own filter extractor treats it as possibly-absent (:614: r.utilization_percent ?? Number.NaN).
  • Failure scenario: the backend omits utilization_percent on one reshape row (a Cost Explorer partial response, the same gap the ?? Number.NaN at :614 anticipates). .toFixed throws a TypeError inside .map(), the container.innerHTML assignment at :1318 never runs, and the Reshape Recommendations panel is left showing the stale skeleton with no error surfaced anywhere. Even without the throw: undefined >= 95 and undefined >= 70 are both false, so an unknown utilization is styled util-red, i.e. presented to the operator as a definite "underutilised, exchange this" signal.
  • Fix: mirror :374-376. Resolve to number | null once, then render — with no colour class when null.

• One malformed last_used_at blanks the entire API-keys table (LOW)

  • Where: frontend/src/apikeys.ts:88; sink at :101
  • What: new Date(key.last_used_at).toISOString() is called inside the row .map(). toISOString() throws a RangeError on an Invalid Date. Every other timestamp on the same row goes through formatDateTime/formatRelativeTime, which both guard with isNaN(d.getTime()) (utils.ts:96, :110).
  • Failure scenario: one row with a malformed or zero-value last_used_at throws during render, so container.innerHTML = table at :101 never executes. The user sees no keys at all and no error message, and therefore cannot reach the Revoke button for any key, including a key they are trying to revoke because it leaked.
  • Fix: reuse formatDateTime for the title attribute (or guard the value) so a bad timestamp degrades to one blank cell instead of an empty page.

• Dead "unknown savings" guard: the -- state can never render (LOW)

  • Where: frontend/src/recommendations.ts:800-804; pageLevelRange at :1073-1126; type at :1032-1033
  • What: the middle branch tests plr.savingsMin === null || plr.savingsMax === null, but pageLevelRange initialises both to 0 and only ever += numbers, and PageLevelRange declares them as number. The branch is unreachable, so the card always falls through to formatCostForPeriod(0, period), i.e. "$0".
  • Failure scenario: when savings data is genuinely absent or all-zero, the Potential Monthly Savings card asserts a confident $0 rather than the -- unknown the code was written to show. This is directly counter to renderHistorySummary (history.ts:355-375), which renders -- specifically "rather than fabricating all-zero values that look like real financial aggregates". Same family as fix(frontend): silent fallbacks fabricate money/coverage values on Dashboard, History, Plans, Opportunities cloud-commitments-cli#1086.
  • Fix: either propagate nullability out of pageLevelRange (track whether any cell contributed a savings value at all) or delete the dead branch, so a future reader does not mistake it for live protection. Prefer the former; the -- state was the intended behaviour.

• Ramp progress fabricates a denominator (LOW)

  • Where: frontend/src/plans.ts:1028
  • What: `${rampSchedule.current_step || 0}/${rampSchedule.total_steps || 1} steps`. A plan whose total_steps is absent or 0 renders as "0/1 steps".
  • Failure scenario: a plan with an unknown step count displays as a concrete one-step plan at 0% progress. The operator reads "one purchase pending" where the truth is "step data is missing", and does not investigate. Note the row two lines above already does this correctly: info.term !== null ? … : '--' (:1016).
  • Fix: render -- when total_steps is not a positive number, matching :1016.

• CSS-selector injection in id-keyed querySelector lookups (LOW)

  • Where: frontend/src/users/userList.ts:240,258,267,285,312,315,344,385,388; frontend/src/plans.ts:337 and :1202
  • What: selectors are built by string interpolation of userId / groupId / column with no escaping. history.ts:101 in the same codebase already does it correctly: `tr[data-execution-id="${CSS.escape(execID)}"]`.
  • Failure scenario: an id containing " or ] makes querySelector throw a SyntaxError. In toggleUserGroup (:239-242) that throw skips the checkbox-revert on the validation-failure path, leaving the checkbox visually checked for a group membership the backend rejected and never saved. The operator believes a user is in a group they are not in.
  • Fix: wrap the interpolated values in CSS.escape(...), as history.ts:101 does.
  • Do not mistake this for the group-permission bug. This item is a UI-robustness defect in the group-membership view; it is not the permission-escalation defect. The frontend group-edit permission bug and the Add Purchaser group + remove implicit admin grant for execute/approve-any/retry-any purchases (separation of duties) cloud-commitments-cli#923 separation-of-duties carve-out bypass are both closed by one backend fix: UpdateGroupAPI writes the client-supplied permission list with no grant ceiling and no system_managed check. That is tracked in sec(auth): the #923 money separation-of-duties carve-out is voidable by any admin in one request cloud-commitments-cli#1550. Hiding or hardening a UI control here changes nothing about it, because the client can call the API directly. Anyone touching group/permission-editing views must read sec(auth): the #923 money separation-of-duties carve-out is voidable by any admin in one request cloud-commitments-cli#1550 first so we do not ship a half-fix that only hides the control.

• Numeric column filters coerce absent money to $0 (LOW)

  • Where: frontend/src/plans.ts:247-249; frontend/src/history.ts:928-930 and :1639-1640; frontend/src/riexchange.ts:612-613
  • What: the numeric filter extractors return p.upfront_cost ?? 0 / estimated_savings ?? 0. The correct "absent, do not match" sentinel is already used in-repo one line away: riexchange.ts:614 returns ?? Number.NaN for utilization_percent.
  • Failure scenario: a user filters Upfront with =0 to find no-upfront commitments. Rows whose upfront is unknown (the backend omitted the field) match as well. The display cells for those same rows correctly render -- (formatCurrency returns '--' for null, utils.ts:32), so the filtered table visibly shows -- rows under a =0 filter: a self-contradicting screen the user has to guess at.
  • Fix: return Number.NaN for absent values so every comparison predicate excludes them, matching riexchange.ts:614.

• Bulk-purchase capacity accepts fractional values from localStorage (LOW)

  • Where: frontend/src/recommendations.ts:3427; consumed at :3919 and :3927
  • What: Math.max(1, Math.min(100, Number(parsed.capacity) || 100)) clamps the range but never checks for an integer, unlike every other numeric parse in the codebase (settings.ts:992, plans.ts:2171, recommendations.ts:694 all enforce Number.isInteger, per the project's own feedback_strict_int_parse rule).
  • Failure scenario: a hand-edited or migrated cudly.recommendations.bulkPurchase.v1 value of 33.33 flows into Math.floor((r.count * 33.33) / 100) at :3919, and into the capacity_percent the backend records for audit (see the comment at :3927). The audit trail then records a capacity no UI control can produce and no user can reproduce when reconciling a purchase.
  • Fix: reject non-integers back to the default, matching the sibling parsers.

• E2E suite covers none of the risk areas above (LOW)

  • Where: frontend/tests-e2e/recommendations.spec.ts (136 lines, 4 tests)
  • What: the suite covers numeric column filtering, the loading/zero-result state, purchase submission and plan submission. No test exercises hostile field values, permission-gated button visibility, date-range defaults, or concurrent loads. The unit suite does carry targeted XSS regressions (xss-purchase-status.test.ts, xss-provider-class.test.ts, history-filter-popover-xss.test.ts) but each pins exactly one field.
  • Failure scenario: this is the mechanism by which both HIGH findings from this review survived. The unescaped payment in recommendations.ts:2954 and plans.ts:690 is invisible to CI, and xss-purchase-status.test.ts renders the exact row that carries the plans.ts one, reporting it green.
  • Fix: make the XSS regressions field-parameterised (assert that every string field of the fixture row renders escaped) instead of one test per known-bad field. Add an E2E case for the slow-first concurrent-load ordering described in the first item.

• target="_blank" without rel="noopener noreferrer" (LOW)

  • Where: frontend/src/index.html:46
  • What: the API Docs header link omits rel. The only other target="_blank" in the file (:1248, the AWS IAM console link) sets it correctly.
  • Failure scenario: limited. The href is same-origin /docs/ and current browsers imply noopener for target="_blank". Filed for consistency with :1248 and to keep the convention greppable, not as an exploitable gap.
  • Fix: add rel="noopener noreferrer".

---

Checked and found clean (recorded so the next review pass does not re-derive them)

• Escaping helpers (utils.ts:194-211): escapeHtml encodes & < > " ', so it is genuinely attribute-safe and escapeHtmlAttr's delegation to it is correct.
• dashboard.ts: KPI tiles (:291-333), upcoming-purchase cards (:441-521) and the details modal (:554+) are built entirely via createElement/textContent. current_coverage/target_coverage correctly use ?? null -> '--' (:282-289) rather than a fabricated default, i.e. the fix(frontend): silent fallbacks fabricate money/coverage values on Dashboard, History, Plans, Opportunities cloud-commitments-cli#1086 fix held.
• riexchange.ts: the Azure table (:414+) and empty states (:157-163) are DOM-constructed; loadUtilization has a proper generation guard (:312-316).
• users/userList.ts: every interpolated field is escaped in both text and attribute positions, and the document-level delegated listener is correctly de-duplicated with an AbortController (:414-423), i.e. the fix(frontend): plans.ts provider XSS (missing whitelist) + duplicate document/modal listeners (users, plans) cloud-commitments-cli#1034 H2 fix held.
• users/permissionMatrix.ts, users/filters.ts:125, users/userModals.ts:193: fully escaped.
• settings.ts: the account list (:481-635) and all four override editors (:695-1121) are DOM-constructed with strict integer validation (:992).
• history.ts row and queue rendering: all API strings escaped; providerCell (:493-498) whitelists the provider before it reaches a class attribute.
• permissions.ts:195-223: canAccess fails closed for non-admins when effectivePermissions has not loaded, and app.ts:63-79 awaits the permissions fetch before setupEventListeners/updateUserUI, so there is no first-paint window in which controls render against an empty permission set.
• Duplicate listeners: all setup*Handlers are called exactly once from app.ts:104; plans.ts:2211-2214 additionally stores unsubscribe handles. All popover global listeners (recommendations.ts:2372-2376, :2605-2621; plans.ts:588-590; riexchange.ts:995-1001, :1263-1269) are paired with an explicit removeEventListener on close. No stacking found.

Findings from the 2026-09-02 codebase audit

Added by an automated audit of 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd (tip of origin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report: docs/audits/codebase-audit-2026-09-02.md.

A12-004 (high)

The same clone at recommendations.ts:3917-3932 has a second, harder failure for Savings Plans. SP recs carry Count 1 by construction (providers/aws/recommendations/parser_sp.go:382, 'Savings Plans don't have a count'), so Math.floor(1 * capacity / 100) is 0 for every capacity in 1..99 and the rec hits continue: it vanishes from the purchase with no per-row notice, and an SP-only selection aborts with 'Try a higher %'. The Go sizing code special-cases exactly this, scaling SavingsPlanDetails.HourlyCommitment and never touching Count (pkg/recfilter/sizing.go:45-55). Any SP rec that did survive would also show upfront_cost/monthly_cost/savings scaled by the count ratio against an unscaled hourly commitment in details. Fix direction: branch on isSavingsPlanService and scale the SP hourly commitment by capacity/100 while leaving count at 1, mirroring ApplyCoverage. Finding A12-004.

A12-011 (high)

The stale-response item here covers loadConvertibleRIs at riexchange.ts:252-278, but there is a second hole one layer down that a generation guard on that loader alone will not close. utilizationGeneration is incremented only inside loadConvertibleRIs (riexchange.ts:273), so loadExchangeableAzureRIs (:289-311) and renderGCPEmptyStates (:120-136) never invalidate an in-flight loadUtilization. Switching the provider chip from AWS to Azure while a Cost Explorer utilization call is pending lets the stale response pass its own guard at :316-323 and call renderRIsTable, replacing the freshly rendered Azure reservations with the AWS empty state. The fix needs the counter bumped at every provider entry point, not just the AWS one. (audit finding A12-011)

A12-023 (medium)

A second defect in the same pageLevelRange accumulator as the dead unknown-savings branch item. The docstring at frontend/src/recommendations.ts:1052-1053 states that upfrontMin is the upfront "of the variant whose savingsMin was contributed", but cellSummary scans savings and upfront_cost in independent min/max comparisons at :1016-1020, so for any cell whose cheapest-upfront variant is not its lowest-savings variant the pairing is false. That is the cross-extremum flaw the payback section immediately below explicitly avoids and documents as "NOT attainable". No number is currently wrong, because the card renders upfrontMin/upfrontMax as an independent range through formatSavingsRange at :806 and nothing computes on the claimed pairing, so this is a wrong docstring rather than a wrong figure. Worth correcting in the same pass, since a future reader will take the pairing at face value. Finding A12-023.

A12-026 (medium)

A gate divergence the sweep did not surface, across two of the reviewed modules. The Home widget's Cancel button and the Plans row's Disable button both call DELETE /api/purchases/planned/{id} (dashboard.ts:648-669, plans.ts:812-824) but compute eligibility differently: canCancelUpcomingPurchase grants on cancel-any:purchases alone (dashboard.ts:412-421), while the Plans row ANDs canManageScheduledPurchase, whose full-scope set is only admin/update-any (plans.ts:673-679, :717-725). A user holding cancel-any:purchases but not update-any:purchases, looking at a scheduler-created row with a null created_by_user_id, sees a working Cancel button on Home and no Disable button on Plans for the same execution. One shared canCancelScheduledExecution(row) called from both surfaces removes the asymmetry. (audit finding A12-026)

A12-037 (medium)

A coverage hole in history.ts worth adding to this list. All eleven history*.test.ts files mock getAmortizeUpfront to return false and none flips it (history.test.ts:52 and its nine siblings), and no fixture creates history-controls or purchases-approval-queue-section -- those exist only in frontend/src/index.html:180, :242. So both mountAmortizeCheckbox calls, syncAmortizeCheckbox, the subscribeAmortizeUpfront re-render path, the amortizedMonthly cell in both tables (history.ts:1138-1141) and the "(amortized)" header are never executed. A regression that added the upfront twice, or dropped it, would ship green on a money column. A fixture with those containers plus one test flipping getAmortizeUpfront to true, asserting the cell equals monthly + upfront/(term*12) and the header text, closes it. (audit finding A12-037)

A12-043 (medium)

Why several RI-exchange defects survive CI, for this list. executeExchange is mocked in three suites (riexchange.test.ts:12, riexchange-column-filters.test.ts:27, riexchange-active-ri-filters.test.ts:20) and no test anywhere reads mockedApi.executeExchange.mock.calls -- the suite asserts only the quote request shape. That is why the missing region in the posted body and the max_payment_due_usd currency assumption both ship green. renderExchangeHistory, canApproveRIExchangeRow and handleRIExchangeApproveClick appear in no test file at all, so the history table's cells and its Approve gating are equally unguarded. Two tests would cover it: one driving quote through execute and asserting the full posted body, one rendering a pending history record and asserting the cells plus the Approve gate. (audit finding A12-043)

A11-017 (low)

One correction to the sibling-parser list in the fractional-capacity item: settings.ts:992 is cited there as correctly enforcing Number.isInteger, and it does, but the create path in the same file does not. submitOverrideForm (frontend/src/settings.ts:1852-1856) validates coverage with Number.isFinite(n) && n >= 0 && n <= 100 only, so 50.5 is accepted and stored. handleCoverageOverrideChange, the inline editor on the same row, then rejects that stored value with "Coverage must be an integer between 0 and 100", so any later edit of the row is refused until the value changes. The browser blocks the fractional value today because #override-coverage carries step="1" (index.html:1196) inside a real submit form, so reaching the stored value needs a client that bypasses HTML validation. Adding !Number.isInteger(n) to the create-path condition makes the two guards agree. Finding A11-017.

A11-018 (low)

A second way the same table lies rather than errors. frontend/src/apikeys.ts:50-55 ends its shape fallback in ?? [], so a response carrying neither api_keys nor a bare array (a wrapped envelope, a partial response) resolves to empty and renderApiKeysList paints 'No API keys yet - Create an API key to let automation tools call CUDly programmatically'. An operator reviewing which keys exist before revoking credentials is shown 'none' for a deployment that has active keys. The catch branch already has a correct error path at :69-75 that this case bypasses. It composes with the client-level defect where a body-stripped or truncated 200 resolves as null (frontend/src/api/client.ts:283-287), which reaches this same fallback. Treating 'neither known shape' as an error would route it through renderApiKeysListError. Audit finding A11-018.

A11-020 (low)

Three dead-code items in users/* that the gate sweep did not surface, all confirmed against index.html: there is no #user-role-filter, no #user-group-filter and no #bulk-group-btn anywhere in it, so the roleFilter state, the case 'role' branch in handleFilterChange, the applyFilters role block (users/filters.ts:35-39) and the prompt() handler (users/handlers.ts:76-85) are all unreachable. The dead role rule is also wrong if it is ever revived: roleFilter !== 'admin' && isAdminUser excludes admins for any non-admin filter value, so a "readonly" selection would return every non-admin rather than read-only users. The surviving bulk path is the #bulk-group-select dropdown at handlers.ts:88, and bulkAddToGroup still uses a native confirm() (users/userActions.ts:200) where every sibling path uses confirmDialog. (audit finding A11-020)

A12-057 (low)

One more history.ts finding for this list, on a money-authorising surface. The entire marketplace price summary is wrapped in if (purchase) (frontend/src/history.ts:1451-1512) while the confirmDialog and the createMarketplaceListing call sit outside it (:1520-1535). When lastPurchases.find returns undefined - a stale render, or the row scrolled out of the fetched window - the dialog renders only the generic fee note with no RI id, no remaining term, no list price and no net proceeds, and 'Confirm listing' still submits. The user authorises a marketplace sale with no amount on screen. Aborting with an error toast when the lookup fails is the fix; a dialog that authorises money should never open without an amount. Audit finding A12-057.

A12-060 (low)

Same shape as the 'Ramp progress fabricates a denominator' bullet, one function over. const completed = summary.total_completed ?? total (frontend/src/history.ts:405) falls back to the full row count, so an API that supplies total_pending and total_purchases but omits total_completed - the partial-deploy case the surrounding comment is written for - renders '10 completed, 3 pending' for a dataset with 7 completed. The lines immediately above (:371-390) go out of their way to render -- rather than fabricate money values, then this one fabricates a count. Rendering the detail line only when summary.total_completed != null matches the treatment already in the same function. Audit finding A12-060.

A12-061 (low)

A fixture-versus-production mismatch in history.ts for this list. renderHistoryList (:1136) and renderApprovalQueue (:1792) both stamp data-execution-id, so a pending purchase produces two rows with the same id, and applyExecutionDeepLink resolves it with an unscoped document.querySelector (:122-124) that takes the first in DOM order. In index.html the queue precedes the list (:180 before :253), so production highlights the queue row; every test fixture appends history-list first (e.g. history-approval-queue.test.ts:105-107), so the tests exercise the opposite element, and the deep-link tests use a hand-built single table that never sees the duplicate. Scoping the deep-link query to #history-list (or giving queue rows a distinct attribute) and reordering the fixtures to match index.html fixes both halves. (audit finding A12-061)

A12-062 (low)

A gate detail in history.ts worth recording alongside the others, because its risk is what a future reader does with it. cancel-any:purchases and revoke-any:purchases are absent from ADMIN_CARVED_OUTS (permissions.ts:173-182), so canAccess('cancel-any','purchases') already returns true for an admin:* holder on both the effective-permissions path and the loading fallback (permissions.ts:333, :350). The leading canAccess('admin','*') || term at history.ts:540 and :677 is therefore outcome-neutral. Its presence here but deliberate absence from rbacAllowsApprove and canRetryFailedRow reads as an intentional asymmetry, which invites a future edit to "fix" the approve path by adding it back and reopening the LeanerCloud/cloud-commitments-cli#923 carve-out. Dropping the redundant term from both gates leaves all four row predicates gating on the verb alone. (audit finding A12-062)

A12-063 (low)

The checked-and-clean note that riexchange.ts's empty states at :157-163 are sound holds only for the pre-filter case. When every row is excluded by a column filter the tables render a thead over an empty tbody with no explanation: frontend/src/history.ts:1124-1194 for purchase history and :1769 for the approval queue, with the only empty states at :1101-1104 covering the pre-filter case; riexchange.ts:330 and :1285 have the same gap for the RI and recommendation tables. A user who unchecks every value in a Provider popover sees a blank table and no hint that a filter is responsible. A single "No rows match the active column filters, clear filters" row when the post-filter list is empty but the pre-filter list is not would close it. Finding A12-063.

A12-065 (low)

One more for this list, on the RI-exchange table. When getRIUtilization fails or is throttled the catch logs and returns without recording the failure (frontend/src/riexchange.ts:324-326), leaving currentUtilization empty so every row renders the permanent loading ellipsis at :378. The operator cannot distinguish 'Cost Explorer is slow' from 'utilization is unavailable', and an active utilization_pct column filter silently matches zero rows because the extractor returns NaN for every row. Recording the failure in module state and rendering 'n/a' with the error in a title attribute is enough. Audit finding A12-065.

A12-068 (low)

Small dead-code item for riexchange.ts, in scope for this pass. MODE_VALUES is referenced only by its own definition and a void MODE_VALUES; statement (frontend/src/riexchange.ts:71, :76). The comment above the void says "MODE_VALUES is used in saveAutomationSettings", but that function reads modeInput.value directly at :2046 and there is no other reference, so the void exists purely to defeat the unused-variable lint and the comment sends a reader looking for a label-to-value inversion that does not exist. Deleting the constant and the void is the whole fix. (audit finding A12-068)

A12-070 (low)

One more riexchange.ts item for this list. The cross-family staleness banner cannot distinguish soft from hard, because both branches state the same freshness bound. The function contract at frontend/src/riexchange.ts:509 says soft means data may be up to 12 hours old, while the rendered copy at :547-550 says "may be up to 24h old" for soft and "older than 24h" for hard. An operator about to act on a cross-family alternative therefore learns nothing from the distinction; only the colour differs. Fix is either to make the soft copy say 12h or to drop the hardcoded hour figures and rely on ageLabel, which is already interpolated into both strings. Note #100 tracks a separate backend defect on the same feature, where a freshness read error leaves RecsStaleness empty and stale recs render as fresh. Audit finding A12-070.

[cristim]

The existing capacity-scaling/on-demand-baseline finding remains reproducible on main 5405fd1e90dffdb701a46d4b93868b947e1fbf05 and PR LeanerCloud/cloud-commitments-cli#2071 candidate 8c00eafc10bbe0fb88fc89f192376ba94720513a. An independent fresh-context Astra reviewer reproduced identical behavior on both: at 50% capacity, savings scale but on_demand_cost does not; a fixture with original savings $900 and on-demand cost $1,900 displays 23.7% rather than 47.4%. The relevant helper is frontend/src/recommendations.ts:3973 on that candidate. This is retained under this existing tracking item, not duplicated or bundled into LeanerCloud/cloud-commitments-cli#2071. Full independent review and test limitations: LeanerCloud/cloud-commitments-cli#2071 (comment)

[cristim]

The existing capacity-scaling finding was independently reconfirmed during the final Astra review of LeanerCloud/cloud-commitments-cli#2071 at 23b6dc27422b18471057b6c12cd66507af12a852 against base5405fd1e90dffdb701a46d4b93868b947e1fbf05. The scaling clone leaves on_demand_cost unchanged while scaling savings; updatePurchaseModalTotals can therefore halve the weighted effective-savings percentage at50% capacity. The original base scaling loop has the same omission, and the percentage helpers are unchanged. This remains this issue’s existing item, not an introduced regression or an extra fix in LeanerCloud/cloud-commitments-cli#2071. Independent review record: LeanerCloud/cloud-commitments-cli#2071 (comment) . Verification was committed-source tracing and isolated probes, not a cloud purchase.