Skip to content

feat(provider): add proto provider — protoCLI via ACP#3

Merged
mabry1985 merged 1 commit into
mainfrom
feat/proto-provider-2026-05-24
May 25, 2026
Merged

feat(provider): add proto provider — protoCLI via ACP#3
mabry1985 merged 1 commit into
mainfrom
feat/proto-provider-2026-05-24

Conversation

@mabry1985

Copy link
Copy Markdown

Summary

Phase 2 of the clawpatch-Quinn integration in workstacean. The existing gateway provider gives Quinn fast/cheap stateless review by inlining file contents into an HTTP prompt. proto complements it with a tool-use review path — spawns protoCLI as a live ACP agent during review so it can read additional files, run --lsp-backed code-intel queries, and invoke shell.

Wire

Uses acpx's --agent escape hatch (acpx doesn't ship a proto subcommand upstream and we don't need it to):

acpx --agent "proto --acp -m <model>" --cwd <root>
     --approve-reads --format json --json-strict --suppress-reads
     exec --file -

Same JSON-schema mechanics as the existing acpx provider, same prompt/stdin contract. acpxFailureMessage / extractAcpxJson are reused so the error shape stays consistent.

Config

Env Default Notes
CLAWPATCH_PROTO_MODEL protolabs/reasoning CLI --model overrides
CLAWPATCH_PROTO_TIMEOUT_MS (or CLAWPATCH_PROVIDER_TIMEOUT_MS) 300000 (5 min) Proto-specific wins
OPENAI_BASE_URL / OPENAI_API_KEY (proto default) Forwarded to protoCLI via env. Typically the LiteLLM gateway.

check()

Validates BOTH acpx --version AND proto --version so clawpatch doctor catches missing-CLI bootstrap failures explicitly instead of failing mid-review on token spend.

Test plan

  • 7 new unit tests on the pure helpers (protoAgentCommand, buildProtoAcpxArgs, protoTimeoutMs) + providerByName("proto") sanity
  • pnpm test --run — 731 pass / 1 pre-existing skipped (was 724)
  • pnpm typecheck clean
  • pnpm build clean
  • Live e2e via workstacean: dispatch pr_review against a non-trivial PR, force clawpatch_review to invoke with model: proto (or via env), observe protoCLI booting and tool-use findings landing in Quinn's verdict

Out of scope

  • workstacean's clawpatch_review tool defaults the provider to gateway; switching individual invocations to proto is a follow-up PR there

🤖 Generated with Claude Code

Phase 2 of the clawpatch-Quinn integration. The existing `gateway`
provider does stateless review by inlining file contents into an
HTTP prompt; `proto` complements it with a tool-use review path —
spawns protoCLI (@protolabsai/proto) as a live ACP agent so it can
read additional files, run --lsp-backed code-intel queries, and
invoke shell during review.

Wire
----

Uses acpx's `--agent` escape hatch (acpx doesn't ship a `proto`
subcommand upstream and we don't need it to):

  acpx --agent "proto --acp -m <model>" --cwd <root>
       --approve-reads --format json --json-strict --suppress-reads
       exec --file -

Same JSON-schema mechanics as the existing acpx provider; same
prompt/stdin contract. acpxFailureMessage / extractAcpxJson / etc. all
reused via dispatch through `proto` for diagnostics so the error
shape is consistent with `acpx`.

Config
------

  CLAWPATCH_PROTO_MODEL          default protolabs/reasoning. CLI
                                  --model overrides.
  CLAWPATCH_PROTO_TIMEOUT_MS     default 300000 (5 min); proto-specific
                                  wins over CLAWPATCH_PROVIDER_TIMEOUT_MS
                                  when both are set.
  OPENAI_BASE_URL / OPENAI_API_KEY  forwarded to protoCLI via env
                                    inheritance. Typically the LiteLLM
                                    gateway.

check() validates BOTH `acpx --version` AND `proto --version` so
`clawpatch doctor` catches missing-CLI bootstrap failures explicitly
instead of waiting until a review run fails on token spend.

Tests
-----

+7 unit tests on the pure helpers: protoAgentCommand (default,
options.model > env > default), buildProtoAcpxArgs (read + approve
permission modes, exec/--file/- terminator), protoTimeoutMs (default,
provider-wide fallback, proto-specific wins, garbage values fall back).
Plus providerByName("proto") sanity. 731 pass total (was 724).

Version bumped to 0.6.0.
@mabry1985 mabry1985 merged commit 82a8925 into main May 25, 2026

@mabry1985 mabry1985 left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

QA Audit — PR #3 | feat(provider): add proto provider — protoCLI via ACP

VERDICT: FAIL


CI Status

  • Analyze (actions): ✅ success
  • Review dependency changes: ✅ success
  • Analyze (typescript): ⏳ in_progress
  • test: ❌ FAILURE

Diff Review
Adds a new proto provider to protopatch that drives protoCLI via acpx's --agent escape hatch (acpx --agent "proto --acp -m <model>"). Reuses the same JSON-schema mechanics and error shape as the existing acpx provider. Includes 7 new unit tests covering protoAgentCommand, buildProtoAcpxArgs, protoTimeoutMs, and providerByName("proto"). Documentation in docs/providers.md and CHANGELOG.md are consistent with the implementation.


Observations

  • CRITICAL: test CI check is FAILING on head 9e5a126. PR cannot be merged until this is resolved. Please re-run the test suite and push a fix.
  • LOW: clawpatch_review structural review unavailable — repo not mounted in this container. Full code quality confirmed via diff review only.
  • LOW: Analyze (typescript) is still in progress — worth monitoring but not a blocker on its own.

Recommended fix: Diagnose the failing tests, push a fix, and re-run CI. The test failure is blocking auto-merge regardless of code quality.

— Quinn, QA Engineer


⚠️ Note: Unable to submit CHANGES_REQUESTED review — GitHub rejected with "Review can not request changes on your own pull request." Escalating to human review.

@mabry1985

Copy link
Copy Markdown
Author

Submitted COMMENT review on protoLabsAI/protoPatch#3. GitHub rejected the REQUEST_CHANGES action with "Review can not request changes on your own pull request" — since the author is @mabry1985 (not Quinn), this appears to be a permission issue on the repo, but I flagged it explicitly in the review body and escalated to human review. The primary finding stands: test is CRITICAL-FAILING and blocking auto-merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant