Skip to content

Add usage-updated hooks for successful refreshes - #3512

Open
guillem-gelabert wants to merge 2 commits into
steipete:mainfrom
guillem-gelabert:feat/usage-updated-hook
Open

Add usage-updated hooks for successful refreshes#3512
guillem-gelabert wants to merge 2 commits into
steipete:mainfrom
guillem-gelabert:feat/usage-updated-hook

Conversation

@guillem-gelabert

@guillem-gelabert guillem-gelabert commented Sep 8, 2026

Copy link
Copy Markdown

Problem

External hook consumers currently receive quota threshold and provider status transitions, but they cannot evaluate the latest quota headroom after an ordinary refresh. That prevents opt-in automations such as dispatching queued work before an otherwise unused quota window resets.

Change

  • Emit usage_updated after the macOS app publishes a successful, current provider refresh.
  • Emit from regular refresh publication and the selected stacked token-account and Codex-account publication paths.
  • Include positional primary and secondary usage fractions, reset times, and window cadence in minutes; omit synthetic placeholder windows.
  • Reuse the existing direct-exec hook runner and coalesce this event per provider/account for ten minutes to avoid command storms. Privacy mode passes a separate internal account discriminator to the limiter without adding it to the JSON payload, environment, arguments, or logs.
  • Document the app-only behavior and environment variables, and provide a representative CLI test event.
  • Cover payload mapping, environment export, rate limiting, synthetic filtering, regular and stacked successful-refresh boundaries, stale refresh suppression, and two-account privacy behavior through the real hook runner.

codexbar hooks watch remains transition-based and does not synthesize this snapshot event.

Validation

  • PATH=/opt/homebrew/bin:$PATH make test — 1,044 selections in 87 groups; 87 first-pass successes, 0 failures, retries, or timeouts.
  • PATH=/opt/homebrew/bin:$PATH make lint — SwiftFormat clean; SwiftLint 0 violations across 2,151 files; repository gates passed.
  • swift test --filter 'ProviderArchitectureGatekeeperTests|CLIHooksTests|HookDispatchTests|UsageUpdatedHookTests' — 62 tests passed.
  • Astra focused review of the two reported P2 paths — no Critical or Important findings.
  • CODEXBAR_SIGNING=adhoc ./Scripts/package_app.sh release — deep signature validation and packaged launch smoke checks passed.

Installed-bundle smoke

  • Installed the packaged 0.57.1 (build 141) bundle and verified its main binary SHA-256 matches the packaged binary.
  • Enabled usage_updated rules in an existing shared config without replacing other settings.
  • The installed helper's hooks test usage_updated --provider codex invoked both configured commands successfully. A local redaction command retained only field presence and cadence: primary and secondary usage/reset fields were present, cadence was 300/10080 minutes, and no account field was exported.

The installed-helper check uses the representative CLI event; it does not claim a live provider refresh.

Related to #2000, #2001, and #2536.

@clawsweeper

clawsweeper Bot commented Sep 8, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

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

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 8, 2026

@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: dfab5227c5

ℹ️ 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 thread Sources/CodexBar/UsageStore+Refresh.swift Outdated
Comment thread Sources/CodexBarCore/Hooks/HookEvent.swift
@clawsweeper

clawsweeper Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 8, 2026, 2:06 PM ET / 18:06 UTC (Revision 3).

ClawSweeper review

What this changes

Adds opt-in commands receiving primary and secondary quota snapshots after successful macOS refreshes, with account-scoped throttling, CLI sample events, documentation, and tests.

Merge readiness

Blocked before merge - 2 items remain

This remains a distinct, useful extension to existing hooks. Both prior correctness findings are addressed; the installed-helper smoke strengthens validation but does not establish the new macOS refresh-to-hook behavior.

Priority: P2
Reviewed head: 89e75d38a8d3024f54323832f82f61d98f2a2d00

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The focused patch resolves prior findings and adds substantial regression coverage, while central runtime proof remains incomplete.
Proof confidence 🦐 gold shrimp (3/6) Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the captured installed-helper smoke demonstrates CLI sample dispatch and existing-config preservation, but bypasses UsageStore's new successful-refresh publication paths; no real macOS provider refresh delivering the payload is shown. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the captured installed-helper smoke demonstrates CLI sample dispatch and existing-config preservation, but bypasses UsageStore's new successful-refresh publication paths; no real macOS provider refresh delivering the payload is shown. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 9 items Repository policy and patch scope: Read the full root AGENTS.md; no nested AGENTS.md or maintainer notes were found under the inspected source, test, docs, and agent directories. Reviewed the complete local introduced diff across all 13 files. Tests and builds were not executed under the read-only review contract.
Current main and release do not provide this event: Inspected HookEvent.swift at fetched main and v0.57.0: both expose six transition/failure events without usage_updated. The existing watch detector emits transitions rather than ordinary successful-refresh snapshots.
Prior publication finding addressed: Regular refresh publication emits after its current-generation guard; both selected stacked-account success paths now emit after their post-await guards. This matches the contributor's responses at #3512 (comment).
Findings None None.
Security None None.

How this fits together

CodexBar refreshes provider usage and publishes account snapshots to its macOS app. Its hook subsystem filters configured rules and sends event data to user-selected local commands.

flowchart TD
  A[Provider refresh results] --> B[Current successful publication]
  B --> C[Primary and secondary quota snapshot]
  C --> D[Enabled hook rules]
  D --> E[Private account rate limit]
  E --> F[Local command with JSON and environment]
Loading

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the captured installed-helper smoke demonstrates CLI sample dispatch and existing-config preservation, but bypasses UsageStore's new successful-refresh publication paths; no real macOS provider refresh delivering the payload is shown. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Complete next step (P2) - Provide after-fix evidence from a real macOS provider refresh reaching the configured command. Screenshots or recordings are welcome where useful; terminal output and logs count. Redact credentials, account details, IP addresses, and private endpoints. Update the PR body to trigger re-review; if needed, ask a maintainer to comment @clawsweeper re-review.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test growth Production +109/-10; tests +490/-22 Production growth implements the new event using existing infrastructure; most added lines cover publication and payload regressions.

Technical review

Best possible solution:

Keep snapshot delivery within the existing opt-in hook framework, preserving transition semantics and private account-scoped throttling.

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

Not applicable as a feature request; source confirms main lacks successful-refresh hook events, and both previously reported patch defects are addressed.

Is this the best way to solve the issue?

Yes: extending the existing event and runner infrastructure is a focused solution, and the documented app-only scope avoids changing headless transition behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 928166f89947.

Labels

Label changes:

  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P2: This is a bounded, opt-in automation improvement without an urgent existing-user outage.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the captured installed-helper smoke demonstrates CLI sample dispatch and existing-config preservation, but bypasses UsageStore's new successful-refresh publication paths; no real macOS provider refresh delivering the payload is shown. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Repository policy and patch scope: Read the full root AGENTS.md; no nested AGENTS.md or maintainer notes were found under the inspected source, test, docs, and agent directories. Reviewed the complete local introduced diff across all 13 files. Tests and builds were not executed under the read-only review contract. (AGENTS.md:1, 89e75d38a8d3)
  • Current main and release do not provide this event: Inspected HookEvent.swift at fetched main and v0.57.0: both expose six transition/failure events without usage_updated. The existing watch detector emits transitions rather than ordinary successful-refresh snapshots. (Sources/CodexBarCore/Hooks/HookEvent.swift:7, 928166f89947)
  • Prior publication finding addressed: Regular refresh publication emits after its current-generation guard; both selected stacked-account success paths now emit after their post-await guards. This matches the contributor's responses at Add usage-updated hooks for successful refreshes #3512 (comment). (Sources/CodexBar/UsageStore+TokenAccounts.swift:1484, 89e75d38a8d3)
  • Prior privacy-throttling finding addressed: Dispatch receives a separate private account discriminator; payload identity remains redacted independently. The new privacy test sends two account snapshots through the real runner and checks that both commands execute without CODEXBAR_ACCOUNT. (Sources/CodexBar/UsageStore+Hooks.swift:48, 89e75d38a8d3)
  • Captured installed-bundle evidence: The supplied body at sourceRevision 586f5a95be4d39cbd3f13893be5643eacadbec3ae632ed5ff41690b0985e1b58 reports installed build 141, matching packaged binary hashes, preservation of existing settings, and two successful configured commands through hooks test. It explicitly identifies this as a representative CLI event, not a live provider refresh. This improves the previous review's upgrade evidence without satisfying its remaining refresh-path proof request. (89e75d38a8d3)
  • Tests exercise publication with synthetic provider results: The new suite covers successful versus failed publication, stacked accounts, stale generations, payload fields, and privacy. The regular refresh test injects _test_providerFetchOutcomeOverride; these are useful regressions but do not demonstrate a real provider response reaching the app hook. (Tests/CodexBarTests/UsageUpdatedHookTests.swift:167, 89e75d38a8d3)

Likely related people:

  • Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Jeremy Chapeau: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

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

  • Add redacted output from the freshly built macOS app showing a successful provider refresh delivering the snapshot hook, including account-scoped privacy behavior.

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.

History

Review history (2 earlier review cycles)
  • reviewed 2026-09-08T15:24:02.585Z sha dfab522 :: needs real behavior proof before merge. :: [P2] Separate account throttling identity from redacted payload identity
  • reviewed 2026-09-08T17:40:12.242Z sha 89e75d3 :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant