fix: follow app privacy setting on serve dashboard - #2960
Closed
urda wants to merge 1 commit into
Closed
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 027aefadf9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+712
to
+714
| let dashboardIdentityMode = Self.dashboardIdentityFlagPresent(in: values) | ||
| ? decodedIdentityMode | ||
| : nil |
There was a problem hiding this comment.
When --identity is omitted, these lines now make serve follow the app preference, but both the option help near ServeOptions.identity and CLIHelp.serveHelp still tell users that identity defaults to full account emails. Consequently, someone consulting codexbar serve --help can miss that a GUI toggle changes the HTTP response and can break clients that depend on full email identities; document the preference-following default and the explicit full/redacted overrides in the executable help.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
Codex review: needs real behavior proof before merge. Reviewed August 15, 2026, 11:43 PM ET / August 16, 2026, 03:43 UTC. ClawSweeper reviewWhat this changesThe PR makes Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 10 items remain Keep open: this changes the shipped no-flag dashboard identity contract into one mutable by a GUI preference, so it needs an explicit product decision and proof before merge. Likely related people: steipete (medium confidence). Priority: P2 Review scores
Verification
How this fits togetherThe serve command produces authenticated dashboard snapshots for the built-in web UI. Identity selection controls whether account emails in those snapshots are full or redacted, and the selected mode participates in response caching. flowchart LR
A[Serve command options] --> C[Identity mode decision]
B[App privacy preference] --> C
C --> D[Dashboard snapshot route]
D --> E[Response cache]
E --> F[Built-in web dashboard]
Decision needed
Why: The current docs, executable help, and runtime all establish a full default, while the proposed behavior can alter machine-facing output during an unrelated GUI action. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve the stable omitted-flag full-identity contract unless a maintainer explicitly approves application-setting control; then align every help and provider-specific document, add route-level coverage, and show a redacted live-server transcript. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: current main defaults serve snapshots to full identity, while the PR changes the omitted-flag path to resolve from the defaults-backed privacy setting per request. Is this the best way to solve the issue? No: the implementation is mechanically coherent, but changing a documented machine-facing default based on a GUI preference is not the safest solution without explicit owner approval or an opt-in mode. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f6daa60e8f88. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (2 earlier review cycles)
|
urda
force-pushed
the
urda/serve-hide-personal-info
branch
from
August 16, 2026 02:58
027aefa to
f6f08c1
Compare
The serve dashboard ignored the app's "Hide personal information" toggle and always served full account emails unless --identity was passed. Resolve the identity mode per request when the flag is absent, so the toggle applies without a serve restart, and fold the resolved mode into the response cache key so a body cached before a toggle cannot be replayed after it. An explicit --identity still wins. codexbar dashboard keeps its full default.
urda
force-pushed
the
urda/serve-hide-personal-info
branch
from
August 16, 2026 03:38
f6f08c1 to
d55a1b7
Compare
Owner
Finesssee
added a commit
to nesszer/Win-CodexBar
that referenced
this pull request
Aug 18, 2026
Finesssee
added a commit
to nesszer/Win-CodexBar
that referenced
this pull request
Aug 18, 2026
Finesssee
added a commit
to nesszer/Win-CodexBar
that referenced
this pull request
Aug 18, 2026
* Port upstream 0.50.1: Cursor rename + Ollama cookie stripping (steipete#2951, steipete#2949) * Port upstream 0.50.1: OpenCode Go session+weekly pace in CLI (steipete#2957) * Port upstream 0.50.1: chart axis label centering (steipete#2974) * Port upstream 0.50.1: Kiro re-authenticate via kiro-cli login (steipete#2340) * Port upstream 0.50.1: serve identity follows app redaction per-request (steipete#2960) * Port upstream 0.50.1: Codex routed pricing, auth.json read-only, known-zero history, Antigravity dashboard lanes, Claude OAuth revoked detection steipete#2946 Codex routed models pricing — codex_routed_provider() + strip_route_prefix() in cost_pricing.rs; deepseek/, kimi/, opencode/ routes price against matching models.dev provider; unknown provider/ prefixes return None. steipete#2944 Codex auth.json read-only during refresh — codex_external_oauth_sources_allowed setting (default OFF); is_external_oauth + last_refresh fields on CodexCredentials; enforce_external_oauth_gate (8-day staleness window). steipete#2932 Codex known-zero history — known_zero field on CostSummary; set in both cache-debounce and full-scan paths (history_coverage_established && sessions_count == 0); knownZero JSON field + CLI text update. steipete#2963 Antigravity dashboard lanes — quota-bucket dedup in parse_user_status; models sharing the same (remaining_fraction, reset_time) collapse to one lane. steipete#2516 Claude revoked vs missing OAuth — ProviderError::OAuthRevoked variant; revocation detection (401/403 with revoked/invalid_grant/token_revoked); 15-min CLI result cache (LazyLock<Mutex<Option<CachedCliResult>>>); fetch_via_auto returns cached CLI result when OAuth revoked, and stale cache when all live sources fail. * Port upstream 0.50.1: Mistral PAYG spend, cost-summary display style, per-provider accent color Item 1 — Mistral PAYG current-month API spend (steipete#2821, steipete#2947): - Add MonthlyPlan variant to MetricPreference enum - Add currency_symbol field to CostSnapshot with with_currency_symbol builder - Set currency_symbol from Mistral billing API response - Add MonthlyPlan to bridge label/parse, tray selected_metric_percent (None = no bar) - Show formatted cost amount in provider_status_label for MonthlyPlan - Add format_cost_amount helper in bridge.rs - Add Mistral monthly spend row in MenuCardDetails - Add monthlyPlan option to MenuBarMetricSection for Mistral Item 2 — Menu cost-summary display style per provider (steipete#2976): - Add CostSummaryDisplayStyle enum (Compact/Detailed/Hidden) in types.rs - Add cost_summary_display_style field to Settings + Default + RawSettings round-trip - Add to SettingsSnapshot bridge + SettingsUpdate + apply in Tauri commands - Add bridge label/parse functions - Apply in MenuCardDetails: hidden hides cost section, compact shows used/limit only - Update describeCard to filter hasCost for hidden style - Add Select control in UsageSpendTab Item 3 — Per-provider accent color override (steipete#2972): - Add accent_color field to ProviderConfig - Add brand_color(ProviderId) function in provider.rs (mirrors frontend registry) - Add accent_color/set_accent_color/effective_accent_color accessors in Settings - Add set/get/get_effective Tauri commands + normalize_hex_accent_color validator - Add providerAccentColors map to SettingsSnapshot bridge - Add setProviderAccentColor/getProviderAccentColor/getProviderEffectiveAccentColor to tauri.ts - Create AccentColorSection component (hex input, color picker, reset button) - Inject --provider-accent CSS variable on MenuCard article and ChartsSection - Update menu-metric__bar-fill and chart colors to use --provider-accent fallback - Update chartPalette providerCostColor/providerCreditsColor fallback chain - Pass providerAccentColors through TrayPanel/PopOutPanel/ProvidersTab/ProviderDetailPane Also: - Add 11 new locale keys to all 7 .ftl files + keys.ts - Add costSummaryDisplayStyle + providerAccentColors to all test mock SettingsSnapshot - Update chartPalette tests for --provider-accent fallback * Resolve merge conflicts and fix gate checks * Thermo-nuclear: extract ollama cookies, codex pricing, simplify accent color * fix: correct serde attribute placement on currency_symbol after rebase
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
codexbar serveweb dashboard ignored the app's "Hide personal information" setting. It always showed full account emails unless the operator passed--identity redactedat startup.This change makes the serve dashboard follow that setting.
--identityis absent, each request reads the app'shidePersonalInfopreference. The toggle on redacts email local parts. The toggle off keeps full emails.resetTimesShowAbsoluteandweeklyProgressWorkDaysfrom the same defaults domain.--identity redactedor--identity fullstill wins, so a scripted client keeps the mode that it asked for.codexbar dashboardkeeps its full identity default. A one-shot JSON command must not change its output because of a GUI toggle.serveDashboardSnapshotRoutemoved out ofhandleServeRequest. The new lines pushed that function past the 150-line SwiftLint limit, so the route got its own function instead of a lint suppression.Commands run
swift buildmake check(SwiftFormat and SwiftLint strict): 0 violationsmake test: 864 selections in 72 groups, 0 failures, 0 retriesTests
Tests/CodexBarTests/CLIServeDashboardIdentityTests.swiftcovers:fullfrom an absent flag.These tests run no live provider probe and open no Keychain prompt.
Notes
The raw
/usageand/costroutes still return full emails. Only the dashboard snapshot follows the setting, and the web UI reads that snapshot. This preserves the documented shape of the JSON API routes.