Skip to content

fix(crof): remove provider after shutdown - #3828

Closed
steipete wants to merge 1 commit into
mainfrom
triage/20260921-crof-removal
Closed

steipete wants to merge 1 commit into
mainfrom
triage/20260921-crof-removal

Conversation

@steipete

Copy link
Copy Markdown
Owner

Crof has shut down, so CodexBar should no longer offer or call its provider. Adopts @baanish's removal from #3657 on the current registry, leaving 74 providers. Removes the descriptor, implementation, bundled plugin, icon, localized settings, quota/menu/widget special cases, and three presentation settings used only by Crof. Historical changelog entries remain.

Existing Crof config entries follow the existing unknown-provider decoder: reads ignore them without writing the file, and the next save drops them while preserving supported provider settings. The isolated CLI proof preserved all 74 surviving provider entries byte-for-byte. Contributor credit is retained for Aanish Bhirud.

Generated files were refreshed with ./Scripts/regenerate-provider-manifests.sh, ./Scripts/regenerate-plugin-js.sh, python3 Scripts/generate_serve_provider_icons.py, and node Scripts/generate-llms.mjs. Icon regeneration also includes Pi's existing SVG, which was missing from the baseline embedded table. No files under Sources/CodexBarCore/Generated changed.

Validation:

  • CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --jobs 2 --filter CodexBarConfigUnknownProviderTests: baseline regression red, 2 tests failed with 3 assertions; both pass after removal.
  • CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --jobs 2 --filter 'CodexBarConfigUnknownProviderTests|ProviderPluginParityTests|UserProviderPluginPortableTests|ProviderArchitectureGatekeeperTests|WidgetProviderChoiceTests|WidgetProviderPagerTests|BurnDownWidgetConfigurationTests|ProviderWidgetAvailabilityTests|ProviderWidgetSnapshotTests|UsageStoreWidgetSnapshot.*Tests|CLISnapshotTests|CLIUnificationGoldenTests|MenuCardCompactAccountRowTests|ProviderCredentialCharacterizationTests|UsageStoreSessionQuotaTransitionTests|ProviderPluginEngineBenchmarkTests|AccountMenuLayoutPlannerTests|ProviderPresentationPolicyCharacterizationTests|MenuCardProviderRegressionTests|MenuCardModelTests|MenuDescriptor.*Tests|ProviderConfigByteStabilityTests|CLIConfigCommandTests|QuotaWarningNotificationLogicTests|SessionQuotaNotificationLogicTests': 403 app tests and 12 plugin tests; the only failing test was the expected pair of widget color hashes, captured and updated for the registry removal.
  • CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --jobs 2 --filter 'ProviderArchitectureGatekeeperTests|CodexBarConfigUnknownProviderTests|CLIServeWebUITests': 61 tests passed after updating the hashes.
  • CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 CODEXBAR_PLUGIN_BENCHMARK=1 swift test --skip-build --filter ProviderPluginEngineBenchmarkTests: 1 test passed, both engines with fixture transport.
  • Built CLI under isolated HOME, CODEX_HOME, XDG_CONFIG_HOME, and CODEXBAR_CONFIG: config dump --json --log-level warning emits Ignoring unknown provider in config, omits Crof, and does not rewrite the file. A same-value config disable --provider codex save drops Crof and leaves the canonical surviving config byte-for-byte unchanged.
  • swift build --jobs 2: Build complete!
  • make check: 0 violations, 0 serious in 2513 files; manifests, plugins, locale counts, and repository checks pass.
  • Independent autoreview: scoped-clean through P2. No live provider probes or app relaunches.

Production diff: 42 files changed, 5 insertions(+), 396 deletions(-) (net −391). Tests: 11 files changed, 88 insertions(+), 580 deletions(-). TestsPlugin: 2 files changed, 6 insertions(+), 10 deletions(-).

Follow-up: the pre-existing docs/social.png raster still says 61 providers. Its HTML source now says 74; there is no repository raster-generation script, so the image was not manually edited.

Closes #3657

Adopt #3657 on the current 74-provider registry, remove Crof-only
presentation policy, and retain the existing unknown-provider config path.
Regenerate provider manifests, bundled plugin JavaScript, embedded CLI
icons, and the documentation index with repository scripts.

Verify retired config entries are ignored without rewriting on read and
removed on save while preserving all supported provider entries.

Co-authored-by: Aanish Bhirud <47579874+baanish@users.noreply.github.com>
@clawsweeper

clawsweeper Bot commented Sep 21, 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. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 21, 2026
@clawsweeper

clawsweeper Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 21, 2026, 5:44 AM ET / 09:44 UTC.

ClawSweeper review

What this changes

Removes the retired Crof integration, its presentation special cases and assets, updates provider documentation, and adds config upgrade regression coverage.

Merge readiness

Ready for maintainer review

Keep open: Crof remains registered on main and in v0.63.0, so this retirement is still needed. No blocking defect was found, and the owner explicitly accepts the documented config-discard behavior.

Priority: P2
Reviewed head: 9909f0f8f897c6a779daf2274ec590baeb195a38

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) Focused retirement with config regression coverage, reported CLI validation, and no blocking findings.
Proof confidence 🐚 platinum hermit (4/6) Not applicable: The external-contributor proof gate does not apply to this OWNER-authored PR. Its body reports the production CLI config dump/save path preserving 74 supported entries, and the adopted contribution supplies fresh-install and upgrade terminal evidence.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The external-contributor proof gate does not apply to this OWNER-authored PR. Its body reports the production CLI config dump/save path preserving 74 supported entries, and the adopted contribution supplies fresh-install and upgrade terminal evidence.
Evidence reviewed 8 items Repository policy: Read the complete root AGENTS.md; no nested AGENTS.md or maintainer-notes directory was found. Applied provider isolation and CLI-focused validation guidance. No builds, tests, account probes, or repository edits were performed.
Pinned removal and merge ownership: Reviewed the introduced base-to-head changes. The verified test merge has the pinned main and PR head as its two parents, and its tree matches the PR head. The removal covers the enum, manifests, bundled implementation, presentation branches, assets, and corresponding tests.
Accepted upgrade consequence: The OWNER-authored PR explicitly states that Crof entries are ignored without rewriting on read and discarded on the next save. The added configuration documentation and changelog disclose this policy. This accepts the specific retirement tradeoff raised in the earlier contribution; another acceptance request is unnecessary.
Findings None None.
Security None None.

How this fits together

CodexBar’s provider registry connects saved settings and CLI selections to usage fetching and menu presentation. Removing Crof makes its saved entry pass through the existing unknown-provider handling.

flowchart TD
  A[Saved provider settings] --> B[Configuration loader]
  C[Registry without Crof] --> B
  C --> D[CLI and menu choices]
  B --> E[Supported provider settings]
  B --> F[Ignore retired Crof entry]
  E --> G[Next settings save]
  G --> H[Config without Crof]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +5/−396 lines; tests +94/−590 lines The reduction removes retired functionality and its tests while adding focused config-preservation coverage.
Provider registry 75 → 74 supported providers The reported CLI upgrade run checks preservation of all 74 surviving provider entries.

Root-cause cluster

Relationship: canonical
Canonical: #3828
Summary: This owner-authored PR explicitly adopts the earlier retirement contribution on the current registry; neither is established as merged.

Members:

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

Technical review

Best possible solution:

Retire the unavailable integration through the existing registry and config handling while preserving supported-provider settings and documenting the accepted discard-on-save policy.

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

Not applicable as a defect reproduction: this retires an integration. Source inspection establishes the registry and config effects, supported by reported CLI upgrade validation.

Is this the best way to solve the issue?

Yes. Removing the registrations and Crof-only code while reusing the established unknown-provider decoder is a narrow, maintainable retirement path.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add P2: This is bounded retirement work for one unavailable service, with no demonstrated urgent failure in surviving integrations.
  • add merge-risk: 🚨 compatibility: Existing Crof selections become invalid and saved Crof settings disappear on the next save; the owner explicitly accepts and documents this intentional tradeoff.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply to this OWNER-authored PR. Its body reports the production CLI config dump/save path preserving 74 supported entries, and the adopted contribution supplies fresh-install and upgrade terminal evidence.

Label justifications:

  • P2: This is bounded retirement work for one unavailable service, with no demonstrated urgent failure in surviving integrations.
  • merge-risk: 🚨 compatibility: Existing Crof selections become invalid and saved Crof settings disappear on the next save; the owner explicitly accepts and documents this intentional tradeoff.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply to this OWNER-authored PR. Its body reports the production CLI config dump/save path preserving 74 supported entries, and the adopted contribution supplies fresh-install and upgrade terminal evidence.

Evidence

What I checked:

  • Repository policy: Read the complete root AGENTS.md; no nested AGENTS.md or maintainer-notes directory was found. Applied provider isolation and CLI-focused validation guidance. No builds, tests, account probes, or repository edits were performed. (AGENTS.md:1, 9909f0f8f897)
  • Pinned removal and merge ownership: Reviewed the introduced base-to-head changes. The verified test merge has the pinned main and PR head as its two parents, and its tree matches the PR head. The removal covers the enum, manifests, bundled implementation, presentation branches, assets, and corresponding tests. (Sources/CodexBarCore/Providers/Providers.swift:64, 9909f0f8f897)
  • Accepted upgrade consequence: The OWNER-authored PR explicitly states that Crof entries are ignored without rewriting on read and discarded on the next save. The added configuration documentation and changelog disclose this policy. This accepts the specific retirement tradeoff raised in the earlier contribution; another acceptance request is unnecessary. (docs/configuration.md:307, 9909f0f8f897)
  • Shared config boundary and regression coverage: The unchanged decoder skips unknown providers, and the shared store reads without writing before normalizing and encoding on save. App persistence and CLI config commands use this store. The added isolated test checks unchanged disk bytes after loading and the exact surviving config after saving. (Tests/CodexBarTests/CodexBarConfigUnknownProviderTests.swift:28, 9909f0f8f897)
  • Reported runtime validation and adopted proof: The captured PR body reports an isolated built-CLI config dump/save run preserving all 74 surviving provider entries. The adopted contribution at Remove Crof provider after shutdown #3657 includes a real CLI transcript demonstrating fresh-install absence, Crof selection rejection, read-only loading, and preservation of its then-current 68 surviving entries. That older transcript is supporting evidence, not execution of this head.
  • Presentation and generated-asset scope: Base-tree searches show that only Crof enabled the three removed presentation settings. Read-only blob comparisons verified that all 23 app locale files only remove the two Crof strings, website locale changes only replace 75 with 74, and the embedded icon table only removes Crof and adds Pi’s existing SVG unchanged. (Sources/CodexBarCore/Providers/ProviderUsagePresentation.swift:259, d8d0f3394989)

Likely related people:

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

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 added a commit that referenced this pull request Sep 21, 2026
Adopt #3657 on the current 74-provider registry, remove Crof-only
presentation policy, and retain the existing unknown-provider config path.
Regenerate provider manifests, bundled plugin JavaScript, embedded CLI
icons, and the documentation index with repository scripts.

Verify retired config entries are ignored without rewriting on read and
removed on save while preserving all supported provider entries.

Co-authored-by: Aanish Bhirud <47579874+baanish@users.noreply.github.com>
(cherry picked from commit 9909f0f)
steipete added a commit that referenced this pull request Sep 21, 2026
Adopt #3657 on the current 74-provider registry, remove Crof-only
presentation policy, and retain the existing unknown-provider config path.
Regenerate provider manifests, bundled plugin JavaScript, embedded CLI
icons, and the documentation index with repository scripts.

Verify retired config entries are ignored without rewriting on read and
removed on save while preserving all supported provider entries.

Co-authored-by: Aanish Bhirud <47579874+baanish@users.noreply.github.com>
(cherry picked from commit 9909f0f)
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. P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant