fix: make eviction planning CLI help successful and bounded - #217
Conversation
|
Warning Review limit reachedNext included review available in 48 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthrough세 개의 cloud CLI가 standalone 도움말을 성공 처리하고, 알 수 없는 인자와 비 UTF-8 인자를 명시적 오류로 처리합니다. 새 블랙박스 테스트가 출력, 종료 상태, 오류 진단을 검증합니다. ChangesCLI 인자 계약
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to The CLI change makes sole help requests succeed and keeps invalid arguments bounded, but the tests do not verify the exact diagnostic text required for unknown and non-UTF-8 arguments. A future change could weaken those messages without failing tests, so merge is reasonable with explicit owner follow-up. Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headc05aa3715900099694ddcdc3619294c54579b920. -
Head SHA:
c05aa3715900099694ddcdc3619294c54579b920 -
Workflow run: 32148589859
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (4 files)"]
R1 --> V1["required checks"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Decision
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (4 files)"]
R1 --> V1["required checks"]
|
c05aa37 to
88ba748
Compare
|
Current head |
|
@OpenCode review current exact head |
Dismissing: this REQUEST_CHANGES was posted against commit c05aa37 where coverage-evidence failed. The PR's current head has coverage-evidence passing (verified via gh pr checks). Remaining review thread is resolved. Re-request review to re-validate.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
1 participant
Purpose
Keep the three local-eviction/destination operational CLIs bounded and honest under both ordinary and coverage-instrumented builds. Sole
--help/-hremains a successful terminal action; invalid/mixed input stays non-zero; valid native filesystem paths are not coerced through UTF-8 protocol parsing.Covered binaries:
disksage-icloud-local-eviction;disksage-incomplete-download-destination-plan;disksage-icloud-local-eviction-batch.Exact current state
3212686e7deeee63eab3fd2774fd23bfe4331d31;main:7eb131c4b4ac628226b262d6406a663fc7dc632f;Current exact-head workflows are not yet passing evidence:
32733482360— queued;32733482294— in progress;32733482430— queued;32733482467— queued.Test-first repairs
Coverage-runtime integrity
A new black-box regression builds
disksage-icloud-local-evictionanddisksage-incomplete-download-destination-planwithcargo rustc --locked --features cloud-cli ... -- --cfg coverageand requires their real terminal-help behavior. Before the production fix, both binaries compiled coverage-only empty entry points (and the local-eviction binary hid its runtime behindcfg(not(coverage))), so coverage instrumentation could avoid measuring the shipped parser/runtime. Those coverage-only shortcuts are removed; ordinary and coverage builds now execute the same production entry point.Native filesystem path admission
The existing real-binary process contract now also sends absolute Unix paths containing a non-UTF-8 byte.
--cloud-root,--path,--manifest,--record-dir,--source-root,--capacity-snapshot, and--private-outputare host filesystem values and remain nativeOsString/PathBufvalues. Textual protocol fields and option tokens still require UTF-8 and retain the existing bounded diagnostics. The regression proves native path values reach the next real runtime boundary instead of failing as*-invalid-utf8-argument.Authority boundary
No iCloud eviction authorization, double-confirmation requirement, immutable record authority, capacity admission, destination selection, incomplete-download recovery/materialization semantics, cloud/provider mutation, or source-eviction rule was weakened. The changes only remove false coverage runtime substitution and preserve valid native path operands.
Required before merge
Do not merge unless the unchanged exact head satisfies every applicable live workflow and current finding, every valid review thread is resolved, live-main ancestry remains current, review/governance requirements are satisfied, and repository-wide exact 100% owned-production statement/region-equivalent, branch, function, and line coverage is proven without exclusions or threshold weakening. Pending, queued, skipped-required, neutral-required, failed, stale, predecessor, synthetic, diagnostic-only, status-only, model-only, rate-limited, no-source-scanner, or infrastructure-only evidence is non-passing.