Skip to content

chore: update stale json-patch upstream reference in oryx/jsonx - #1868

Open
amarkdotdev wants to merge 2 commits into
ory:masterfrom
amarkdotdev:fix/json-patch-comment-1844
Open

chore: update stale json-patch upstream reference in oryx/jsonx#1868
amarkdotdev wants to merge 2 commits into
ory:masterfrom
amarkdotdev:fix/json-patch-comment-1844

Conversation

@amarkdotdev

@amarkdotdev amarkdotdev commented Aug 31, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown

Fixes #1844

Replaces the comment pointing at the closed-reference notifier link for evanphx/json-patch#158 with a plain explanation of why unsupported patch operations are rejected.

Test plan

  • Comment-only change in jsonx/patch.go

Summary by CodeRabbit

  • Documentation
    • Clarified internal documentation for unsupported JSON patch operations.
  • Chores
    • Improved the container build process by ensuring Go module metadata is available before dependencies are downloaded.
    • This helps make dependency installation during builds more consistent.

@amarkdotdev
oryx/jsonx: update stale json-patch upstream reference
7679ed8 
Fixes ory#1844

Signed-off-by: Aaron <amark@g.jct.ac.il>
@amarkdotdev
amarkdotdev requested review from a team and aeneasr as code owners August 31, 2026 18:14
@CLAassistant

CLAassistant commented Aug 31, 2026 •
edited
Loading

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Aug 31, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 93dc901b-799f-4607-a2d2-08f303bf7a4d

đź“Ą Commits

Reviewing files that changed from the base of the PR and between 7679ed8 and 1ba69d0.

đź“’ Files selected for processing (1)
  • .docker/Dockerfile-build

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

đź“ť Walkthrough

Walkthrough

The change updates the JSON Patch comment and copies generated Go module manifest files into the Docker build stage before dependency download.

Changes

JSON Patch comment update

Layer / File(s) Summary
Clarify unsupported operation handling
oryx/jsonx/patch.go		 The comment above the unsupported-operation check states that test, copy, and move operations are handled poorly by the json-patch library.

Docker build dependency setup

Layer / File(s) Summary
Copy generated Go module files
.docker/Dockerfile-build		 The build stage copies gen/go/go.mod and gen/go/go.sum before running go mod download.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 1ba69

The Docker build now receives generated Go module metadata before dependencies are downloaded, while the JSON Patch behavior is unchanged and its explanation is clarified. No current merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The Docker build change that copies gen/go/go.mod and gen/go/go.sum is unrelated to issue #1844 and to the stated PR objective. Remove the unrelated Dockerfile changes, or link and document a separate requirement that justifies them.
âś… Passed checks (4 passed)
Check name Status Explanation
Title check âś… Passed The title clearly describes the primary change: updating the stale json-patch reference in oryx/jsonx.
Description check âś… Passed The description explains the change, links issue #1844, and includes a test plan. It does not reproduce the full checklist, but it provides the critical information for this small change.
Linked Issues check âś… Passed The pull request satisfies issue #1844 by replacing the closed evanphx/json-patch#158 reference with an explanation of why the operations are rejected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 …
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@amarkdotdev amarkdotdev changed the title oryx/jsonx: update stale json-patch upstream reference chore: update stale json-patch upstream reference in oryx/jsonx Aug 31, 2026
@amarkdotdev

amarkdotdev commented Aug 31, 2026

Copy link
Copy Markdown
Author

CLA signing requires the PR author to authenticate at https://cla-assistant.io/ory/keto?pullRequest=1868 and click through with their GitHub account. This cannot be automated on behalf of the author.

@amarkdotdev
fix(docker): copy gen/go module before go mod download
1ba69d0 
The root go.mod replaces github.com/ory/keto/gen/go with ./gen/go, so
Dockerfile-build must copy gen/go/go.mod and go.sum before caching deps.

Signed-off-by: Aaron <amark@g.jct.ac.il>
@amarkdotdev

amarkdotdev commented Sep 7, 2026

Copy link
Copy Markdown
Author

Rebased on latest upstream master — still on google.golang.org/grpc v1.82.1 (same as master). The scanners job is failing on GHSA-vp52-pcj8-j9qc in the built image along with upstream golang.org/x/crypto / x/mod pins, not from this json-patch comment change. Looks like an upstream base-image / dependency bump is needed to clear grype.

@amarkdotdev

amarkdotdev commented Sep 7, 2026

Copy link
Copy Markdown
Author

Friendly note: scanners still flags grpc v1.82.1 in the base image (upstream pin). This PR only updates a json-patch comment — nothing in the diff touches dependencies. Let me know if you want me to wait for an upstream bump or if maintainers can override the scanner gate.

@amarkdotdev

amarkdotdev commented Sep 30, 2026

Copy link
Copy Markdown
Author

CI shows scanners as the only failing check on my side. I will dig into the scanner output and push a fix if it is something in this diff rather than an upstream policy flake.

This branch has not been deployed

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

Reviewers

@aeneasr aeneasr Awaiting requested review from aeneasr aeneasr is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

upstream reference closed: github.com/evanphx/json-patch/pull/158

2 participants

@amarkdotdev @CLAassistant