refactor(arch): finish standardplugins split — remove pluginreg/standardbundle shim and types.go alias by matdev83 · Pull Request #117 · matdev83/go-llm-interactive-proxy

matdev83 · 2026-07-08T09:40:45Z

Completes the remaining Phase 3 registry-split cleanup from the architecture review resolution plan.

What changed

Deleted the transitional internal/pluginreg/standardbundle/ shim.
Deleted internal/standardplugins/types.go and qualified all registry types as pluginreg.X (Registry, BackendFactory, BackendSecurityProfile, FrontendMount, FeatureFactory, etc.).
Switched runtimebundle/bootstrap_plan.go to call standardplugins.InstallStandardBundleOn directly.
Updated pluginreg package doc to clarify it is the registry mechanism only.
Updated all standardplugins tests to use pluginreg.Registry directly.
Relaxed internal/archtest/guardrails_test.go so that runtimebundle/bootstrap_plan.go is the single allowed composition-root call site for InstallStandardBundleOn.

Verification

go test ./internal/archtest/... ./internal/standardplugins/... ./internal/infra/runtimebundle/... ./internal/core/config/... passes.
No behavior change; this is purely boundary/ownership cleanup.

This is the first of several PRs splitting the post-review cleanup changes on the local working tree.

…ardbundle shim and types.go alias Complete the Phase 3 registry split by removing the remaining transitional indirection: - Delete internal/pluginreg/standardbundle/ and switch the composition root (runtimebundle bootstrap_plan.go) to standardplugins.InstallStandardBundleOn. - Delete internal/standardplugins/types.go and qualify all registry types as pluginreg.X (Registry, BackendFactory, BackendSecurityProfile, etc.). - Update pluginreg package doc to state it is the registry mechanism only. - Update all standardplugins tests to use pluginreg.Registry directly. - Allow runtimebundle/bootstrap_plan.go as the single composition-root call site for InstallStandardBundleOn in the architecture guardrail test. No behavior change; purely boundary/ownership cleanup. Co-authored-by: Cursor <cursoragent@cursor.com>

coderabbitai · 2026-07-08T09:41:18Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4485b1c2-4f2f-40fd-b1d2-e76dbff8df58

📥 Commits

Reviewing files that changed from the base of the PR and between 29d8b1a and 77288e2.

📒 Files selected for processing (32)

internal/archtest/guardrails_test.go
internal/core/config/effective_default_route.go
internal/infra/runtimebundle/bootstrap_plan.go
internal/infra/runtimebundle/options.go
internal/pluginreg/main_test.go
internal/pluginreg/reg.go
internal/pluginreg/standardbundle/install.go
internal/pluginreg/standardbundle/install_test.go
internal/pluginreg/standardbundle/main_test.go
internal/standardplugins/backend_model_inventory_config_test.go
internal/standardplugins/backend_prefix_inventory_test.go
internal/standardplugins/backend_security_test.go
internal/standardplugins/backends_openaicodex_test.go
internal/standardplugins/bedrock_acp_install_regression_test.go
internal/standardplugins/build_backend_test.go
internal/standardplugins/credentials.go
internal/standardplugins/custom_backends.go
internal/standardplugins/custom_compatible_build_test.go
internal/standardplugins/feature_yaml_test.go
internal/standardplugins/hosted_backend_build_test.go
internal/standardplugins/llamacpp_build_test.go
internal/standardplugins/lmstudio_build_test.go
internal/standardplugins/localstub_build_test.go
internal/standardplugins/ollama_build_test.go
internal/standardplugins/opencode_backends_test.go
internal/standardplugins/registry_zero_test.go
internal/standardplugins/resolve_upstream_api_keys_test.go
internal/standardplugins/standard_bundle_posture_test.go
internal/standardplugins/standard_bundle_test.go
internal/standardplugins/standard_table.go
internal/standardplugins/types.go
internal/standardplugins/vllm_build_test.go

💤 Files with no reviewable changes (4)

internal/pluginreg/standardbundle/main_test.go
internal/pluginreg/standardbundle/install.go
internal/standardplugins/types.go
internal/pluginreg/standardbundle/install_test.go

📜 Recent review details

⏰ Context from checks skipped due to timeout. (1)

GitHub Check: qa

🧰 Additional context used

📓 Path-based instructions (4)

**/*.go

📄 CodeRabbit inference engine (Custom checks)

**/*.go: For server, CLI, worker, or network Go code, ensure context.Context is propagated correctly, cancellation is respected, and new goroutines cannot leak indefinitely.
Do not make accidental public API breaks in Go code: under pkg/** or anywhere exported Go identifiers are changed, warn if the PR changes exported types, function signatures, error behavior, JSON fields, CLI flags, config keys, or documented behavior without clearly explaining the compatibility impact.

Files:

internal/pluginreg/main_test.go
internal/infra/runtimebundle/options.go
internal/pluginreg/reg.go
internal/core/config/effective_default_route.go
internal/standardplugins/standard_bundle_posture_test.go
internal/standardplugins/localstub_build_test.go
internal/standardplugins/credentials.go
internal/standardplugins/vllm_build_test.go
internal/standardplugins/registry_zero_test.go
internal/standardplugins/resolve_upstream_api_keys_test.go
internal/standardplugins/backend_prefix_inventory_test.go
internal/standardplugins/feature_yaml_test.go
internal/standardplugins/lmstudio_build_test.go
internal/standardplugins/bedrock_acp_install_regression_test.go
internal/standardplugins/custom_backends.go
internal/standardplugins/llamacpp_build_test.go
internal/standardplugins/build_backend_test.go
internal/standardplugins/custom_compatible_build_test.go
internal/infra/runtimebundle/bootstrap_plan.go
internal/standardplugins/backend_security_test.go
internal/standardplugins/ollama_build_test.go
internal/standardplugins/standard_bundle_test.go
internal/archtest/guardrails_test.go
internal/standardplugins/hosted_backend_build_test.go
internal/standardplugins/opencode_backends_test.go
internal/standardplugins/backends_openaicodex_test.go
internal/standardplugins/backend_model_inventory_config_test.go
internal/standardplugins/standard_table.go

⚙️ CodeRabbit configuration file

**/*.go: Review as production Go code. Prioritize correctness, race conditions, goroutine leaks, context cancellation, timeout handling, error wrapping, nil-pointer risks, resource cleanup, defer placement, API compatibility, interface design, dependency boundaries, and testability. Avoid generic style comments when gofmt/golangci-lint already covers the issue.

Files:

internal/pluginreg/main_test.go
internal/infra/runtimebundle/options.go
internal/pluginreg/reg.go
internal/core/config/effective_default_route.go
internal/standardplugins/standard_bundle_posture_test.go
internal/standardplugins/localstub_build_test.go
internal/standardplugins/credentials.go
internal/standardplugins/vllm_build_test.go
internal/standardplugins/registry_zero_test.go
internal/standardplugins/resolve_upstream_api_keys_test.go
internal/standardplugins/backend_prefix_inventory_test.go
internal/standardplugins/feature_yaml_test.go
internal/standardplugins/lmstudio_build_test.go
internal/standardplugins/bedrock_acp_install_regression_test.go
internal/standardplugins/custom_backends.go
internal/standardplugins/llamacpp_build_test.go
internal/standardplugins/build_backend_test.go
internal/standardplugins/custom_compatible_build_test.go
internal/infra/runtimebundle/bootstrap_plan.go
internal/standardplugins/backend_security_test.go
internal/standardplugins/ollama_build_test.go
internal/standardplugins/standard_bundle_test.go
internal/archtest/guardrails_test.go
internal/standardplugins/hosted_backend_build_test.go
internal/standardplugins/opencode_backends_test.go
internal/standardplugins/backends_openaicodex_test.go
internal/standardplugins/backend_model_inventory_config_test.go
internal/standardplugins/standard_table.go

**/*

📄 CodeRabbit inference engine (Custom checks)

Do not introduce hardcoded credentials, API keys, tokens, private keys, passwords, production secrets, or sensitive internal URLs.

Files:

internal/pluginreg/main_test.go
internal/infra/runtimebundle/options.go
internal/pluginreg/reg.go
internal/core/config/effective_default_route.go
internal/standardplugins/standard_bundle_posture_test.go
internal/standardplugins/localstub_build_test.go
internal/standardplugins/credentials.go
internal/standardplugins/vllm_build_test.go
internal/standardplugins/registry_zero_test.go
internal/standardplugins/resolve_upstream_api_keys_test.go
internal/standardplugins/backend_prefix_inventory_test.go
internal/standardplugins/feature_yaml_test.go
internal/standardplugins/lmstudio_build_test.go
internal/standardplugins/bedrock_acp_install_regression_test.go
internal/standardplugins/custom_backends.go
internal/standardplugins/llamacpp_build_test.go
internal/standardplugins/build_backend_test.go
internal/standardplugins/custom_compatible_build_test.go
internal/infra/runtimebundle/bootstrap_plan.go
internal/standardplugins/backend_security_test.go
internal/standardplugins/ollama_build_test.go
internal/standardplugins/standard_bundle_test.go
internal/archtest/guardrails_test.go
internal/standardplugins/hosted_backend_build_test.go
internal/standardplugins/opencode_backends_test.go
internal/standardplugins/backends_openaicodex_test.go
internal/standardplugins/backend_model_inventory_config_test.go
internal/standardplugins/standard_table.go

internal/**

⚙️ CodeRabbit configuration file

internal/**: Focus on package boundaries, hidden coupling, unexported API design, concurrency safety, deterministic behavior, and whether logic belongs in this internal package.

Files:

internal/pluginreg/main_test.go
internal/infra/runtimebundle/options.go
internal/pluginreg/reg.go
internal/core/config/effective_default_route.go
internal/standardplugins/standard_bundle_posture_test.go
internal/standardplugins/localstub_build_test.go
internal/standardplugins/credentials.go
internal/standardplugins/vllm_build_test.go
internal/standardplugins/registry_zero_test.go
internal/standardplugins/resolve_upstream_api_keys_test.go
internal/standardplugins/backend_prefix_inventory_test.go
internal/standardplugins/feature_yaml_test.go
internal/standardplugins/lmstudio_build_test.go
internal/standardplugins/bedrock_acp_install_regression_test.go
internal/standardplugins/custom_backends.go
internal/standardplugins/llamacpp_build_test.go
internal/standardplugins/build_backend_test.go
internal/standardplugins/custom_compatible_build_test.go
internal/infra/runtimebundle/bootstrap_plan.go
internal/standardplugins/backend_security_test.go
internal/standardplugins/ollama_build_test.go
internal/standardplugins/standard_bundle_test.go
internal/archtest/guardrails_test.go
internal/standardplugins/hosted_backend_build_test.go
internal/standardplugins/opencode_backends_test.go
internal/standardplugins/backends_openaicodex_test.go
internal/standardplugins/backend_model_inventory_config_test.go
internal/standardplugins/standard_table.go

**/*_test.go

⚙️ CodeRabbit configuration file

**/*_test.go: Review tests for meaningful assertions, table-driven coverage, race-prone tests, t.Parallel misuse, nondeterminism, leaked goroutines, real network or filesystem dependencies, fragile sleeps, and missing edge cases. Prefer testing observable behavior over implementation details.

Files:

internal/pluginreg/main_test.go
internal/standardplugins/standard_bundle_posture_test.go
internal/standardplugins/localstub_build_test.go
internal/standardplugins/vllm_build_test.go
internal/standardplugins/registry_zero_test.go
internal/standardplugins/resolve_upstream_api_keys_test.go
internal/standardplugins/backend_prefix_inventory_test.go
internal/standardplugins/feature_yaml_test.go
internal/standardplugins/lmstudio_build_test.go
internal/standardplugins/bedrock_acp_install_regression_test.go
internal/standardplugins/llamacpp_build_test.go
internal/standardplugins/build_backend_test.go
internal/standardplugins/custom_compatible_build_test.go
internal/standardplugins/backend_security_test.go
internal/standardplugins/ollama_build_test.go
internal/standardplugins/standard_bundle_test.go
internal/archtest/guardrails_test.go
internal/standardplugins/hosted_backend_build_test.go
internal/standardplugins/opencode_backends_test.go
internal/standardplugins/backends_openaicodex_test.go
internal/standardplugins/backend_model_inventory_config_test.go

🧠 Learnings (1)

📚 Learning: 2026-07-01T22:57:42.953Z

Learnt from: matdev83
Repo: matdev83/go-llm-interactive-proxy PR: 101
File: pkg/lipsdk/scope/context.go:0-0
Timestamp: 2026-07-01T22:57:42.953Z
Learning: In this repository, when defining unexported Go `context` key constants of type `ctxKey int` (e.g., `const ( kFoo ctxKey = iota + N )`), preserve the `iota + <offset>` pattern and keep the existing `<offset>` values rather than simplifying to plain `iota`. These per-package offsets are part of the repo-wide convention to avoid key collisions across packages, and each such constant set should include a short explanatory comment (for example: “offset avoids collision with other packages' context keys”).

Applied to files:

internal/pluginreg/main_test.go
internal/infra/runtimebundle/options.go
internal/pluginreg/reg.go
internal/core/config/effective_default_route.go
internal/standardplugins/standard_bundle_posture_test.go
internal/standardplugins/localstub_build_test.go
internal/standardplugins/credentials.go
internal/standardplugins/vllm_build_test.go
internal/standardplugins/registry_zero_test.go
internal/standardplugins/resolve_upstream_api_keys_test.go
internal/standardplugins/backend_prefix_inventory_test.go
internal/standardplugins/feature_yaml_test.go
internal/standardplugins/lmstudio_build_test.go
internal/standardplugins/bedrock_acp_install_regression_test.go
internal/standardplugins/custom_backends.go
internal/standardplugins/llamacpp_build_test.go
internal/standardplugins/build_backend_test.go
internal/standardplugins/custom_compatible_build_test.go
internal/infra/runtimebundle/bootstrap_plan.go
internal/standardplugins/backend_security_test.go
internal/standardplugins/ollama_build_test.go
internal/standardplugins/standard_bundle_test.go
internal/archtest/guardrails_test.go
internal/standardplugins/hosted_backend_build_test.go
internal/standardplugins/opencode_backends_test.go
internal/standardplugins/backends_openaicodex_test.go
internal/standardplugins/backend_model_inventory_config_test.go
internal/standardplugins/standard_table.go

🔇 Additional comments (28)

internal/standardplugins/backend_model_inventory_config_test.go (1)

9-9: LGTM!

Also applies to: 18-18, 36-36, 64-64, 75-75, 103-103, 114-114, 142-142, 153-153

internal/standardplugins/backend_prefix_inventory_test.go (1)

8-15: LGTM!

Also applies to: 28-28, 63-67

internal/standardplugins/backend_security_test.go (1)

8-35: LGTM!

Also applies to: 44-71, 88-112

internal/standardplugins/registry_zero_test.go (1)

10-25: LGTM!

Also applies to: 34-34, 45-45, 64-64, 75-75

internal/standardplugins/resolve_upstream_api_keys_test.go (1)

7-8: LGTM!

Also applies to: 206-206

internal/standardplugins/standard_bundle_posture_test.go (1)

6-6: LGTM!

Also applies to: 17-17, 29-29, 38-38, 57-57

internal/standardplugins/standard_bundle_test.go (1)

8-8: LGTM!

Also applies to: 18-33

internal/standardplugins/backends_openaicodex_test.go (1)

15-15: LGTM!

Also applies to: 67-77, 116-133, 159-168, 193-202, 223-232, 253-262, 283-292, 315-324, 345-354, 386-395

internal/standardplugins/bedrock_acp_install_regression_test.go (1)

7-7: LGTM!

Also applies to: 50-50, 60-60, 72-72

internal/standardplugins/localstub_build_test.go (1)

6-10: LGTM!

Also applies to: 12-29, 31-33

internal/standardplugins/ollama_build_test.go (1)

11-11: LGTM!

Also applies to: 20-34, 43-43, 75-75, 88-88, 109-109, 118-118, 143-143, 158-158

internal/standardplugins/opencode_backends_test.go (1)

13-13: LGTM!

Also applies to: 89-108, 133-133, 188-194

internal/standardplugins/vllm_build_test.go (1)

10-37: LGTM!

Also applies to: 59-59, 71-71

internal/standardplugins/build_backend_test.go (1)

9-33: LGTM!

internal/standardplugins/custom_compatible_build_test.go (1)

13-21: LGTM!

Also applies to: 69-69, 130-130, 181-181, 225-225, 261-261, 285-285

internal/standardplugins/feature_yaml_test.go (1)

9-15: LGTM!

internal/standardplugins/hosted_backend_build_test.go (1)

12-12: LGTM!

Also applies to: 22-22, 35-35, 46-46, 55-55, 75-75, 84-84, 116-116, 125-125, 137-137, 161-161

internal/standardplugins/llamacpp_build_test.go (1)

10-37: LGTM!

Also applies to: 59-59, 71-71

internal/standardplugins/lmstudio_build_test.go (1)

10-37: LGTM!

Also applies to: 59-59, 71-71

internal/pluginreg/main_test.go (1)

9-22: LGTM!

internal/pluginreg/reg.go (1)

1-3: LGTM!

internal/standardplugins/standard_table.go (1)

8-8: LGTM!

Also applies to: 51-58, 62-72, 74-81, 86-97, 100-121, 166-243, 246-273

internal/standardplugins/credentials.go (1)

6-6: LGTM!

Also applies to: 40-40

internal/standardplugins/custom_backends.go (1)

12-12: LGTM!

Also applies to: 225-225

internal/infra/runtimebundle/bootstrap_plan.go (1)

17-17: LGTM!

Also applies to: 128-128

internal/archtest/guardrails_test.go (1)

187-190: LGTM!

Also applies to: 205-207, 221-221
internal/core/config/effective_default_route.go (1)
7-7: 🎯 Functional Correctness

No change needed for the DefaultWireModel comment. standardplugins.DefaultWireModel is a function in internal/standardplugins/default_wire_model.go, so the wording is accurate.
			> Likely an incorrect or invalid review comment.
internal/infra/runtimebundle/options.go (1)
38-38: 🎯 Functional Correctness

standardplugins.DefaultWireModel is still defined in internal/standardplugins/default_wire_model.go, so the BuildOptions.WireModel comment is current.
			> Likely an incorrect or invalid review comment.

📝 Walkthrough

Summary by CodeRabbit

Refactor
- Consolidated standard plugin setup and registry handling for a more consistent startup path.
- Updated backend and feature wiring to use a single shared plugin registration approach.
Bug Fixes
- Tightened safeguards around where standard components can be installed.
- Improved consistency in backend configuration and security-related defaults.
Documentation
- Clarified inline notes to match the current plugin registration behavior.

Walkthrough

This PR relocates standard plugin distribution tables from internal/pluginreg/standardbundle into internal/standardplugins, deleting the standardbundle package and its tests, removing standardplugins' re-export aliases, and updating standard_table.go plus dependent production code, composition-root wiring, guardrail tests, and all standardplugins tests to reference pluginreg types directly.

Changes

Standard plugin registry relocation

Layer / File(s)	Summary
Remove standardbundle package and aliases `internal/pluginreg/standardbundle/install.go`, `.../install_test.go`, `.../main_test.go`, `internal/standardplugins/types.go`	Deletes InstallOn/Bundle/BackendBundle and their tests; removes re-exported Registry/BackendFactoryDeps/credential constants from standardplugins.
Rewire standard_table.go to pluginreg `internal/standardplugins/standard_table.go`	Install functions and registration struct fields now use `pluginreg.Registry`, `pluginreg.BackendFactory`, `pluginreg.BackendSecurityProfile`, and related pluginreg-qualified types/constants.
Supporting production files `internal/standardplugins/credentials.go`, `internal/standardplugins/custom_backends.go`	Switch `EffectiveAPIKeys` and `BackendFactoryDeps{}` calls to `pluginreg`-qualified equivalents.
Composition-root wiring and docs `internal/infra/runtimebundle/bootstrap_plan.go`, `.../options.go`, `internal/core/config/effective_default_route.go`, `internal/pluginreg/reg.go`	Bootstrap now calls `standardplugins.InstallStandardBundleOn`; comments updated to reference `standardplugins.DefaultWireModel` and the new package split.
Guardrail allowlist `internal/archtest/guardrails_test.go`	Allowlists `bootstrap_plan.go` as the only composition-layer file permitted to install the standard bundle.
Backend/build test updates `internal/standardplugins/*_test.go`	All standardplugins tests (openaicodex, bedrock/acp, llamacpp, lmstudio, localstub, ollama, opencode, vllm, custom-compatible, hosted, registry_zero, posture, bundle, feature_yaml, resolve_upstream_api_keys) switch to `pluginreg.NewRegistry()`, `pluginreg.BackendFactoryDeps{}`, and `pluginreg`-qualified constants.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

matdev83/go-llm-interactive-proxy#114: Both PRs modify internal/infra/runtimebundle/bootstrap_plan.go to shift composition/bundle wiring away from pluginreg toward standardplugins/featurebundle.

🚥 Pre-merge checks | ✅ 8

✅ Passed checks (8 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the refactor and accurately mentions the standardplugins split and shim removal.
Description check	✅ Passed	The description is directly related to the changeset and summarizes the split cleanup accurately.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
No Secrets	✅ Passed	No added lines matched secret/key/token/private-key/URL patterns; changes are docs, type wiring, and registry call-site cleanup.
Context Propagation	✅ Passed	PR is a refactoring that removes the standardbundle shim and updates imports. The main code change (bootstrap_plan.go line 128) replaces standardbundle.InstallOn with standardplugins.InstallStandar...
No Accidental Public Api Break	✅ Passed	No public API break detected. All changes are confined to internal/ packages (not in pkg/). The removed standardbundle package and deleted types.go re-exports were transitional infrastructure, not...

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Append a daily-log entry recording the merge activity this session: architecture-review cleanup triaged into #117-#121, Dependabot bumps #122/#123 (with the aws-sdk-go-v2 go.sum conflict resolved by Dependabot auto-rebase), and 3 obsolete stashes dropped. No wiki page changes needed — pages are current; this is a log append only. Co-authored-by: Cursor <cursoragent@cursor.com>

matdev83 merged commit 69941c1 into main Jul 8, 2026
2 checks passed

matdev83 mentioned this pull request Jul 8, 2026

refactor(arch): make runtime.NewExecutor a strong invariant boundary — no post-construction mutation #118

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(arch): finish standardplugins split — remove pluginreg/standardbundle shim and types.go alias#117

refactor(arch): finish standardplugins split — remove pluginreg/standardbundle shim and types.go alias#117
matdev83 merged 1 commit into
mainfrom
pr1/finish-standardplugins-split

matdev83 commented Jul 8, 2026

Uh oh!

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

matdev83 commented Jul 8, 2026

What changed

Verification

Uh oh!

coderabbitai Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading