Skip to content

fix(archera): validate required comparison amounts - #92

Merged
cristim merged 1 commit into
mainfrom
chore/91-archera-pkg-adoption
Oct 18, 2026
Merged

cristim merged 1 commit into
mainfrom
chore/91-archera-pkg-adoption

Conversation

@cristim

@cristim cristim commented Oct 18, 2026 •

Copy link
Copy Markdown
Member

Existing Archera comparison calls now reject missing, null or negative selected_amount at every current/candidate offer by adopting the published shared-package revision 570d168c6bc7. Explicit zero and positive values remain valid. The synthetic comparison fixture now supplies the three required amounts while preserving its financial and hostile-string coverage.

Only the shared-package pin changes; all provider pins remain unchanged. This adds no listing tool and is a prerequisite for #91.

Validation includes actual in-memory MCP CallTool dispatch through the existing registered comparison tool, real client and synthetic transport: the old package incorrectly accepted nine invalid cases; all fifteen invalid/valid controls pass after adoption. Actual product NewServer catalog and purchase safeguards have separate regression coverage; no production transport seam was added.

The full MCP race suite (273 top-level tests), build, vet, pinned lint and normal hooks passed. The actual dependency delta is 49 shared-package files from MCP's older pin; the nine-package runtime import closure and its audit, recommendation and deduplication effects were reviewed. Normalized module graphs retain the same edges, and provider versions are unchanged.

All evidence uses synthetic boundaries. No server listener, vendor request, purchase, installation or deployment was performed.

Refs #91.

Summary by CodeRabbit

  • Bug Fixes
    • Comparison results now handle valid zero and positive selected amounts, while missing, null, or negative amounts return a clear error.
    • Added coverage to verify these outcomes and ensure comparison requests do not expose test-only keys.

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

  • Run on-demand review

This review includes 3 billable files and costs up to $0.75.

  • Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 55 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 82 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-mcp/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 072cd4c7-a5a1-4d97-abac-7bedaf4dfe1b
📥 Commits

Reviewing files that changed from the base of the PR and between b173b02 and 792e427.

⛔ Files ignored due to path filters: 1
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing: 3
  • go.mod
  • tools/archera_pkg_adoption_test.go
  • tools/testdata/archera_comparison_wire.json

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-mcp/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 51042f8b-8b82-4688-9e3b-c5d71a4d5444
📥 Commits

Reviewing files that changed from the base of the PR and between b173b02 and 792e427.

⛔ Files ignored due to path filters: 1
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing: 3
  • go.mod
  • tools/archera_pkg_adoption_test.go
  • tools/testdata/archera_comparison_wire.json

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

📝 Walkthrough

The cloud-commitments-go/pkg dependency advances to a newer revision. New tests cover selected_amount values across the current entry and two candidates. The comparison fixture now includes values for all three offers.

Changes: Archera package adoption

Layer / File(s) Summary
Selected amount package adoption
go.mod, tools/archera_pkg_adoption_test.go, tools/testdata/archera_comparison_wire.json
The package dependency advances to a newer revision. Tests cover missing, null, negative, zero, and positive selected_amount values. The comparison fixture adds values to the current offer and both candidates.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 792e4

No actionable issue remains from the supplied evidence; the change is ready for normal merge checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (2 skipped: 2 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: validation of required comparison amounts in Archera.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@cristim

cristim commented Oct 18, 2026

Copy link
Copy Markdown
Member Author

go441_reviewer_b

APPROVED 792e427 by go441_reviewer_b

Two adversarial code passes are clean. Mapped the committed parent/tree/four blobs to the reviewed snapshot, totaling 71 changed lines. Verified MCP's actual 49-file dependency delta, unchanged provider pins and normalized module graph, decoder bounds, exact decimals, privacy and failure output. Inspected author checks and the other reviewer's actual runtime logs; did not rerun tests. CI and normal merge gates remain separate.

@cristim

cristim commented Oct 18, 2026

Copy link
Copy Markdown
Member Author

go441_reviewer_a

APPROVED 792e427 by go441_reviewer_a

Two adversarial code passes are clean. Verified tree8c4e9fc64e0019f9d8b03cb8bce2d2f47dda9252 and all four blobs match the reviewed and independently tested snapshot. Scope: 71 changed lines. Independently replayed actual MCP protocol/client/transport and product catalog/purchase safeguards: 11 top-level tests, 31 named passes, no failures/skips/races. Exact-parent runtime proof reproduced nine intended validation failures and six valid controls. CI and normal merge gates remain separate.

@cristim cristim added priority/p2 Backlog-worthy triaged Item has been triaged severity/low Minor harm urgency/this-quarter Within the quarter impact/few Limited audience effort/m Days type/feat New capability labels Oct 18, 2026
@cristim
cristim marked this pull request as ready for review October 18, 2026 11:28
@cristim

cristim commented Oct 18, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. Your current included review allowance is based on your included PR review attempts over the past 7 days. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 55 minutes.

@cristim

cristim commented Oct 18, 2026

Copy link
Copy Markdown
Member Author

resume-cc-audit

Exact head 792e427629d0cd6cc3b44c2272249441a39818cf has both independent two-pass approvals, local runtime regression proof and green CI. CodeRabbit comment6249363398 includes a completed review with kind: reviewed coverage for this same SHA, no actionable comments and no inline findings. The combined comment retains an older rate-limit banner; the later completed review is the current evidence.

The optional docstring warning is dismissed: this changes a dependency pin and fixture and adds a focused test, without introducing public API or new tool behavior needing documentation. Redundant test docstrings would duplicate the test name and assertions. No required check or protection is bypassed.

This is only the package-adoption prerequisite; #91 remains open for the listing tool. The default-branch workflow audit found CI only on a normal merge. Release publication requires a separate v* tag, which is outside this action. A normal merge will follow a final exact-head/protection check; no listener, installation or deployment is included.

@cristim
cristim merged commit 0017ece into main Oct 18, 2026
13 checks passed
@cristim
cristim deleted the chore/91-archera-pkg-adoption branch October 18, 2026 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/m Days impact/few Limited audience priority/p2 Backlog-worthy severity/low Minor harm triaged Item has been triaged type/feat New capability urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant