Skip to content

fix(delegate): guard JSON workflow against old CLI versions - #802

Closed
amh1k wants to merge 1 commit into
alibaba:mainfrom
amh1k:fix/delegate-preview-format-compat
Closed

fix(delegate): guard JSON workflow against old CLI versions#802
amh1k wants to merge 1 commit into
alibaba:mainfrom
amh1k:fix/delegate-preview-format-compat

Conversation

@amh1k

@amh1k amh1k commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes the delegation skill/CLI version mismatch where newer skill instructions invoke:

ocr delegate preview --format json

against older OCR binaries that do not support the --format flag.

This adds CLI parser regression coverage and updates both delegation skill copies to detect unsupported OCR versions and instruct users to upgrade instead of silently falling back to incompatible text output.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring (no functional changes)
  • Documentation update
  • CI / Build / Tooling

How Has This Been Tested?

  • make test passes locally
  • Manual and automated testing:
    • go test ./...
    • go test ./cmd/opencodereview -run TestDelegateFlags_RegisterJSONFormat -count=1
    • make check
    • go run ./cmd/opencodereview delegate preview --help
    • go run ./cmd/opencodereview delegate rule --help
    • ocr delegate preview --format json output validated as JSON
    • git diff --check

Checklist

  • My code follows the project's coding style (go fmt, go vet)
  • I have performed a self-review of the code
  • I have added tests that prove the fix is effective
  • New and existing unit tests pass locally
  • I have updated the documentation accordingly
  • I have signed the CLA

Related Issues

Closes #800

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

OpenCodeReview: Review skipped: no items were selected.

@lizhengfeng101

Copy link
Copy Markdown
Collaborator

Thanks for both the report and the PR, @amh1k — the diagnosis here was spot on. Your version-skew analysis (newer skill instructions reaching an older binary, rather than a missing implementation) is exactly right, and the write-up in #800 made it fast to confirm.

We're going to close this one without merging, and that's not a reflection on the change itself. As raised in #800, we want to settle a more fundamental question first: whether ocr delegate should expose a structured output mode at all. The delegation surface was designed so that its primary consumer is the outer host agent rather than a program, and that trade-off is still open. Until the protocol direction is settled, adding version guards to SKILL.md risks being reverted along with whatever we decide — and in the meantime the guard would have to be kept in sync across both skill copies and the QCA assets, which is the same drift problem that produced this issue in the first place.

The insight we do want to carry forward is the one your PR is built on: silently falling back to text output is the wrong failure mode. The downstream steps consume structured fields like reviewable_files, mode, and merge_base, and recovering those from the human-readable manifest is unreliable — so a version mismatch should fail loudly rather than quietly degrade the review. That needs an explicit answer in whatever shape the redesign takes.

We'll put together a proper design for this module and open it up for discussion — including compatibility in both directions (older CLI × newer skill, and the reverse, which today has no protection at all). We'd be glad to have you in that thread.

Thanks again for the CLA, the added test coverage, and the detailed PR description.

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.

ocr delegate preview fails with unknown flag: --format (mismatch with SKILL.md added in commit b1c7c6a)

3 participants