Skip to content

Follow app privacy setting on serve dashboard - #2967

Merged
steipete merged 5 commits into
mainfrom
fix/2960-serve-privacy-setting
Aug 16, 2026
Merged

Follow app privacy setting on serve dashboard#2967
steipete merged 5 commits into
mainfrom
fix/2960-serve-privacy-setting

Conversation

@steipete

Copy link
Copy Markdown
Owner

Maintainer refresh of #2960 by @urda (fork branch went CONFLICTING after today's merge train and is not maintainer-editable). All code and tests are @urda's work, rebased onto current main with the CHANGELOG union-resolved and a credit line added.

Closes #2960.

  • codexbar serve dashboard follows the app's "Hide personal information" setting when no --identity flag is given, resolved per request so toggling applies without a restart.
  • Explicit --identity redacted|full still wins and ignores the app setting.

urda and others added 2 commits August 15, 2026 20:38
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.
# Conflicts:
#	CHANGELOG.md
@clawsweeper

clawsweeper Bot commented Aug 16, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3805441200

ℹ️ 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 +203 to +207
"com.steipete.codexbar",
"com.steipete.codexbar.debug",
]
for domain in domains {
if let value = UserDefaults(suiteName: domain)?.object(forKey: "hidePersonalInfo") as? Bool {

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 Badge Select the defaults domain for the running app

When serve is launched from a debug app and the release defaults domain already contains hidePersonalInfo, this release-first loop always returns that value and never consults com.steipete.codexbar.debug. Consequently, toggling privacy in the running debug app has no effect on its dashboard. Resolve the domain from the containing app/bundle before reading the setting rather than returning the first value found across both domains.

Useful? React with 👍 / 👎.

Comment thread docs/cli.md
- Starts no HTTP server and requires no dashboard bearer token. See `docs/dashboard-api.md` for the shared payload contract.
- `codexbar serve` starts a foreground HTTP server for usage and cost JSON, a token-gated dashboard snapshot, and a built-in web UI at `/`.
- Dashboard snapshot identity defaults to full account emails; use `--identity redacted` to hide email local parts, especially when responses cross a network.
- Dashboard snapshot identity follows the app's "Hide personal information" setting when `--identity` is absent: the toggle on redacts email local parts, off keeps full emails. The setting is read per request, so a change applies without a serve restart. Pass `--identity redacted` or `--identity full` to pin the mode and ignore the app setting, especially when responses cross a network.

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 Badge Update serve --help to describe the new default

This documents the new behavior, but CodexBarCLI.serveHelp still tells users that snapshot identity defaults to full emails. Since CLIIO prints that hard-coded text for codexbar serve --help, users and scripts consulting the command itself receive semantics that contradict this change and may unexpectedly get redacted identities. Update the built-in help alongside this documentation.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Aug 16, 2026
@clawsweeper

clawsweeper Bot commented Aug 16, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed August 16, 2026, 1:18 AM ET / 05:18 UTC.

ClawSweeper review

What this changes

The PR makes codexbar serve follow the app’s Hide personal information setting when --identity is omitted, while retaining explicit redacted or full modes.

Merge readiness

⚠️ Ready for maintainer review - 5 items remain

Keep this owner-authored PR open for two bounded corrections: it can read the wrong app privacy preference in debug builds, and its built-in help contradicts the new default.

Priority: P2
Reviewed head: c666b209aa92c0f453c431c0c73de3a9ec7b834e

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The central implementation is focused and tested at the pure-function level, but two concrete privacy/contract defects block merge.
Proof confidence 🌊 off-meta tidepool Not applicable: This is maintainer-authored work, so the external-contributor real-behavior-proof gate does not apply.
Patch quality 🦐 gold shrimp (3/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This is maintainer-authored work, so the external-contributor real-behavior-proof gate does not apply.
Evidence reviewed 7 items Per-request identity implementation: The PR intentionally leaves an absent identity flag unresolved and resolves it for each dashboard request, including the resolved mode in the cache key.
Debug preference can be bypassed: The new lookup checks the release defaults domain before the debug domain and returns the first stored value; a release preference therefore overrides the running debug app’s preference.
Existing bundle-aware defaults pattern: Current Core code derives one application defaults domain from the bundle or containing app before opening its UserDefaults suite, including debug-bundle handling.
Findings 2 actionable findings [P2] Resolve the active app’s defaults domain
[P2] Align serve --help with the new identity default
Security Needs attention Keep debug privacy preferences isolated: Returning the release-domain value before consulting the debug domain means a debug dashboard can reveal identity details the debug app is configured to redact.

How this fits together

The bundled CLI serves a dashboard snapshot that the built-in web UI polls. Each request resolves an identity mode, uses it in the response-cache key, and returns account data to the dashboard.

flowchart LR
A[App privacy preference] --> B[Serve request]
C[Optional identity flag] --> D[Identity mode resolver]
B --> D
D --> E[Dashboard cache key]
E --> F[Snapshot builder]
F --> G[Web dashboard]
D --> H[Full or redacted identity]
Loading

Before merge

  • Resolve the active app’s defaults domain (P2) - The new release-first lookup returns com.steipete.codexbar as soon as it has a stored value. A debug bundle with its own privacy setting can therefore ignore that setting and serve unredacted identities; derive the one matching app domain before reading the preference, as the existing bundle-aware Core preference path does.
  • Align `serve --help` with the new identity default (P2) - The changed absent-flag behavior is preference-dependent, but the built-in help still promises full emails by default. This leaves the CLI’s own contract inconsistent with the docs and can surprise callers that receive redacted snapshots; update and cover the help text.
  • Resolve security concern: Keep debug privacy preferences isolated - Returning the release-domain value before consulting the debug domain means a debug dashboard can reveal identity details the debug app is configured to redact.
  • Resolve merge risk (P1) - A debug app can serve full email local parts despite its own privacy setting when the release defaults domain contains a conflicting value; the unchanged help text also misstates the behavior existing CLI users receive.
  • Complete next step (P2) - Both blockers have a narrow, source-proven repair and focused validation path on this PR branch.

Findings

  • [P2] Resolve the active app’s defaults domain — Sources/CodexBarCLI/CLIHelpers.swift:202-208
  • [P2] Align serve --help with the new identity default — Sources/CodexBarCLI/CLIHelp.swift:218-219
  • [low] Keep debug privacy preferences isolated — Sources/CodexBarCLI/CLIHelpers.swift:202
Agent review details

Security

Needs attention: The debug-domain fallback can defeat an explicit privacy preference and expose email local parts in dashboard output.

Review metrics

Metric Value Why it matters
Changed surface 7 files; production +142/-65, tests +61 The change spans serving behavior, defaults lookup, documentation, and focused tests, so its CLI contract and preference boundary need coherent coverage.

Root-cause cluster

Relationship: canonical
Canonical: #2967
Summary: This is the active maintainer-refresh implementation replacing a closed, unmerged conflicting branch.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Repair defaults selection and command help (recommended)
    Resolve one defaults domain from the invoked or containing app, then test debug and release selection and align the built-in help with the documented behavior.
  2. Pause the behavior change
    Keep the former full-email default until the privacy preference can be read from the correct application domain and the CLI contract is updated.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Use a bundle-aware single-domain defaults resolver, add debug/release and help-text regression tests, and retain explicit --identity precedence plus identity-specific cache keys.

Technical review

Best possible solution:

Use the active app’s single bundle-aware defaults domain, update serve --help to state the implicit-setting behavior, and add regression coverage before merging.

Do we have a high-confidence way to reproduce the issue?

Yes. With conflicting release and debug hidePersonalInfo defaults, a debug-bundled CLI returns the release value first; codexbar serve --help directly prints the contradictory full-default claim.

Is this the best way to solve the issue?

No. Per-request resolution and cache separation are sound, but selecting the active app domain and updating the built-in command contract are required for a correct implementation.

Full review comments:

  • [P2] Resolve the active app’s defaults domain — Sources/CodexBarCLI/CLIHelpers.swift:202-208
    The new release-first lookup returns com.steipete.codexbar as soon as it has a stored value. A debug bundle with its own privacy setting can therefore ignore that setting and serve unredacted identities; derive the one matching app domain before reading the preference, as the existing bundle-aware Core preference path does.
    Confidence: 0.98
  • [P2] Align serve --help with the new identity default — Sources/CodexBarCLI/CLIHelp.swift:218-219
    The changed absent-flag behavior is preference-dependent, but the built-in help still promises full emails by default. This leaves the CLI’s own contract inconsistent with the docs and can surprise callers that receive redacted snapshots; update and cover the help text.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 2f951e278d27.

Labels

Label changes:

  • add P2: The PR changes a user-facing privacy behavior but the demonstrated failures are bounded to dashboard defaults selection and CLI documentation.
  • add merge-risk: 🚨 compatibility: An existing invocation without --identity changes from a documented full-email default to a preference-dependent result.
  • add merge-risk: 🚨 security-boundary: Incorrect debug-domain selection can defeat the app’s privacy choice and return unredacted email local parts in dashboard output.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This is maintainer-authored work, so the external-contributor real-behavior-proof gate does not apply.

Label justifications:

  • P2: The PR changes a user-facing privacy behavior but the demonstrated failures are bounded to dashboard defaults selection and CLI documentation.
  • merge-risk: 🚨 compatibility: An existing invocation without --identity changes from a documented full-email default to a preference-dependent result.
  • merge-risk: 🚨 security-boundary: Incorrect debug-domain selection can defeat the app’s privacy choice and return unredacted email local parts in dashboard output.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This is maintainer-authored work, so the external-contributor real-behavior-proof gate does not apply.

Evidence

Security concerns:

  • [low] Keep debug privacy preferences isolated — Sources/CodexBarCLI/CLIHelpers.swift:202
    Returning the release-domain value before consulting the debug domain means a debug dashboard can reveal identity details the debug app is configured to redact.
    Confidence: 0.96

Acceptance criteria:

  • [P1] make check.
  • [P1] make test.
  • [P1] Add focused tests that distinguish release and debug defaults domains.
  • [P1] Assert CodexBarCLI.serveHelp describes the preference-dependent absent-flag behavior.

What I checked:

Likely related people:

  • steipete: Introduced the built-in serve dashboard and its identity option, and owns the current maintainer refresh. (role: dashboard feature owner and recent area contributor; confidence: high; commits: e795d349cb84, 8cf897bc9a44, 0e453c4a5b2a; files: Sources/CodexBarCLI/CLIServeCommand.swift, Sources/CodexBarCLI/CLIHelp.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Select the defaults suite from the active or containing app and cover conflicting debug/release values.
  • Update serve --help and add a focused expectation alongside the existing CLI help tests.
  • Run make check and make test after the corrections.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit b9ccac3 into main Aug 16, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants