Repatriate single-consumer provider-bridge SDK exports to their owning plugins - #1729
Merged
Conversation
Executes audit item 2 (Surface size) of the @get-bb/plugin-sdk/provider-bridge entry in docs/api_to_audit.md: - extractEnvOverrides and getMessageContentTypes move from the bridge kit into bb-plugin-provider-claude-code (their only consumer), with the env-override round-trip test. - The pending-interaction-normalization module (and test) moves from the bridge kit into bb-plugin-provider-codex; the SDK surface gains pendingInteractionRequestedPermissionProfileSchema (the domain composite the normalizer validates against) in its place. - cloneReasoningEfforts moves from @bb/domain into claude-code's model catalog (its only consumer; domain stays types/schemas). The other named candidates stay: the Claude mock-CLI traffic config, the claudeTaskTool* schemas, the ACP reasoning/permission CLI schemas, and the workflow snapshot types all have core consumers (agent-runtime, server, thread-view, host-daemon-contract, config, app), and buildEditDiff, completeStartedToolItem, and decodeToolCallResponsePayload are used inside the kit itself. Surface: 197 -> 194 names; bundled provider-bridge d.ts 6615 -> 6498 lines. No wire shapes change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🚨 SLOP COP 🚨 · I am SlopCop. I am reviewing this pull request for security, code quality, performance, architecture, and end-to-end behavior. |
resolveProviderWorkflowsEnabled applied the claudeCodeWorkflowsDisabled app setting to every provider that passes the supportsWorkflows capability policy, so the first third-party provider declaring supportsWorkflows: true would silently inherit a Claude-named user preference. The setting now only affects providerId "claude-code"; other providers' answer comes from the declared capability alone. Behavior-preserving today: grepping provider declarations confirms provider-claude-code is the only provider declaring supportsWorkflows: true (provider-codex, provider-acp, provider-pi, the echo example, and the ACP tier all declare false). Fixes #1722 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![bb-slop-cop[bot]](https://avatars.githubusercontent.com/in/4577281?s=60&v=4){kind=small}
| pendingInteractionFileSystemPermissionsSchema, | ||
| pendingInteractionMacOsPermissionsSchema, | ||
| pendingInteractionNetworkPermissionsSchema, | ||
| pendingInteractionRequestedPermissionProfileSchema, |
There was a problem hiding this comment.
🚨 slopcop/review — Avoid a replacement single-consumer SDK export
This new public schema has one source consumer: the moved Codex normalizer. It replaces one single-consumer helper with another public name.
The local transform already creates the complete typed output. Please return its parsed result or compose a local schema from existing public component schemas.
Then remove this export and rebuild the declarations.
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Plain-English summary: This change moves four shared helpers into the two provider plugins that use them. It reduces the published bridge SDK without runtime changes.
I found no security, performance, or correctness issue.
Findings:
- Low:
pendingInteractionRequestedPermissionProfileSchemais a new public SDK export with only one Codex consumer. This partly replaces the public surface that this pull request removes. Keep the final validation local to Codex and remove this export. - Non-blocking refactor:
model-list.tsstill copies reasoning efforts with the same map operation. Reuse the new localcloneReasoningEffortshelper there.
Validation:
- All 740 focused tests passed across the domain, SDK, bridge protocol, Codex, and Claude packages.
- All 11 focused Turbo test and typecheck tasks passed.
- Both provider conformance suites passed.
- All GitHub checks passed.
- Browser QA did not apply because this change has no user interface, route, or visible workflow.
I found no blocking issue. The SDK export concern is small, but it conflicts with the stated surface reduction goal.
…ssful build
Two defects in dev-loop build status handling:
- Every dev build problem was labeled "frontend bundle build failed:",
including HOST build failures. setDevBuildProblem now takes the build
kind ("frontend" | "host") and labels the problem accordingly; the two
targets keep independent problem slots so a frontend success cannot
hide a host failure or vice versa.
- The problem was sticky: a later successful host rebuild through the
enable/load path (loadHostArtifactCandidate) never cleared it, so a
transient dev-loop failure stayed in plugin status until the next
source change. Both load-path builds (buildPluginApp in
loadAppBundleCandidate, buildPluginHost in loadHostArtifactCandidate)
now clear their own target's problem on success; the dev-loop callers
already cleared on success and now do so per target.
The unit test constructs the plugin runtime directly (in-memory SQLite,
stub hub) and covers per-target labeling, independent clearing, and the
problem surviving later status updates until cleared. End-to-end
coverage of the enable-path clear would require real host bundle builds
in the test, so that path is exercised only through the existing
builtin dev-loop tests.
Fixes #1725
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SlopCop review nit on #1729: buildDiscoveredModel inlined the same spread-map the relocated helper encodes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ymichael
added a commit
that referenced
this pull request
Aug 18, 2026
…registry, repatriate single-consumer helpers (#1742) De-overfits `bridge-kit` following a two-reviewer adversarial audit of every module and export, calibrated on one ruling: **GOOD** = code encoding bb-side invariants (canonical event/item semantics, wire contracts both sides validate); **BAD** = a frame hosting per-provider knowledge injected through generics, callbacks, or config objects. ~80% of the kit survived the audit as genuine invariants (notably `turn-state`, whose suspect generic-plus-hooks shape is driven identically by all three consumers); this PR removes the part that didn't. One commit per finding group: 1. **Unwind `buildToolUseItem`** — the router was an if-chain each bridge could write itself, inverted into the kit via name-sets + parser callbacks + an escape hatch supplied by exactly one provider. The invariant constructors stay (`buildFileChangeItem`, new `buildGenericToolCallItem` fallback); claude-code and pi own explicit switches, preserving null-parse→generic fallback and parent-id threading. 2. **Split `bridge-session-registry`** — only 2 of 4 bridges used it, and over half its option surface existed for one consumer each (`nextToolCallRequestId`, `resolveAdditionalPendingWork`, `stopSession` claude-code-only; `TCloseResult` pi-only). The genuinely shared kernel survives as `createPendingToolCallTracker` (flat requestId→resolver map — also removes the per-response all-sessions scan); session orchestration inlined into the two bridges. **Reviewer note:** claude-code's interactive requests previously shared the tool-call id counter; they now use string-prefixed ids (`interaction-N`), keeping the id spaces collision-free. Wire-compatible: JSON-RPC ids are `string | number` and both ends treat them opaquely. 3. **One decoder for canonical tool-call requests** — `decodeNativeProviderToolCallRequest` had zero consumers anywhere (deleted); `bridge-protocol-adapter` reimplemented `decodeNormalizedProviderToolCallRequest` inline (now delegates); `providerToolCallResponseSchema` privatized. 4. **Repatriations** — `buildShellEnvironmentPolicyConfig` → provider-codex (its `shell_environment_policy.set.*` namespace is codex-native config); claude-code stops round-tripping env vars through codex-style keys and carries a plain map under its own bag key (both ends verified inside the plugin — nothing crosses the daemon wire schema). `diffCumulativeText` → pi, its only consumer. 5. **Dead surface + trims** — `finishOpenProviderTurn` (only caller was a test) deleted; two per-provider policy booleans removed from the good constructors (`completeWebItems`, `preserveUndefinedToolCallFields`); the universally-identical `drainAcceptedUserMessages` call folded into the turn-state core; `CounterScopedItemIdState`'s lying `reasoning*` field names renamed (compaction ids flow through them too); `contentWrapperSchema` and dead arg interfaces un-exported. Includes the `docs/api_to_audit.md` record and regenerated bundled types. ## Numbers - Published surface: 196 → 192 names (−7 removed, +3 added: the two kept constructors and the tracker). - 35 files, +972/−908 (+825/−738 excluding regenerated bundled types). Insertions exceed deletions by design: the audit trades kit generics and single-consumer options for deliberate, explicit per-consumer code, plus ~200 lines of new tests (tracker + moved diff tests). ## Verification - Typecheck green via turbo: provider-bridge-protocol, plugin-sdk, agent-runtime, provider-claude-code, provider-codex, provider-acp, @bb/server. - Tests green via turbo (post-rebase onto the merged #1729): 74 + 98 + 316 + 260 + 164 + 144 + 1722 across those packages. - No wire-shape changes, no protocol or SDK version bumps, pi's daemon-bundled path untouched. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > AGENT GENERATED: by Claude Fable 5 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Executes the follow-up recorded in
docs/api_to_audit.mdunder the@get-bb/plugin-sdk/provider-bridgeaudit (item 2, surface size): repatriate single-consumer bridge-kit exports to their owning plugins and shrink the published surface. Pure move/deletion refactor — no behavior change, no wire shapes, no version bumps.What moved (each verified single-consumer)
extractEnvOverrides(bridge-kitadapter-utils.ts) →plugins/provider-claude-code/src/bridge/env-overrides.ts, with its round-trip test.getMessageContentTypes(bridge-kitprovider-visibility-helpers.ts) → module-local inplugins/provider-claude-code/src/visibility.ts.pending-interaction-normalization.ts(whole module + test) →plugins/provider-codex/src/; its only consumer was codex'sinteractive-requests.ts. The surface gainedpendingInteractionRequestedPermissionProfileSchema(the domain composite the normalizer parses against; its component siblings were already exported).cloneReasoningEfforts→ out of@bb/domainintoplugins/provider-claude-code/src/model-catalog.ts— both consumers are claude-code, and a helper function in@bb/domainviolated its types/schemas-only rule.What deliberately stayed
The audit item's originally named candidates turned out to have core consumers, so they stay; the sweep covered every export on the surface:
claudeCodeMockCliTrafficConfigSchema+ default: type/default consumed by@bb/agent-runtimeand the server.claudeTaskToolNameSchema/claudeTaskToolOutputSchema: share a contract file with exports@bb/thread-viewrenders.acpNativeReasoningSchema,acpPermissionCliSchema,acpReasoningCliSchema: parsed byhost-daemon-contractand@bb/config.docs/api_to_audit.mditem 2 is updated to record this ground truth so the stabilization audit starts from facts.Numbers and verification
bundled-types/bb-plugin-sdk-provider-bridge.d.ts6,615 → 6,498 lines. Net −112 lines.provider-bridge-protocol,plugin-sdk,domain, all four provider plugins, and downstreamagent-runtime/thread-view/server/config/host-daemon-contract.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Also in this PR: two small follow-up fixes from the #1640 issue sweep
Each is its own commit and closes its issue on merge:
resolveProviderWorkflowsEnabledapplied theclaudeCodeWorkflowsDisabledapp setting to every workflows-capable provider; it is now scoped toproviderId === "claude-code". Behavior-preserving today (claude-code is the only provider declaringsupportsWorkflows: true), with a server test proving a non-claude workflows-capable provider ignores the setting while claude-code honors it.frontend bundle build failed:(even for host build failures) and stuck after a later successful rebuild through the enable/load path. Problems are now stored per build target (frontend/host) with matching labels, and each build path clears its own target on success. Unit test covers per-target labels, independent clearing, and persistence-until-cleared.Verification for the full stack:
@bb/serversuite 183 files / 1721 tests green, plus typecheck via turbo.🤖 Generated with Claude Code