docs: remove reference to merged upstream json-patch PR in jsonx#1275
docs: remove reference to merged upstream json-patch PR in jsonx#1275telegraphchi wants to merge 2 commits into
Conversation
The comment referenced evanphx/json-patch#158, which has since been merged. ORY's closed-reference notifier flags merged/closed PRs, so the link was reported as a stale reference (ory#1254). Replace it with a description of the actual behavior: only "add", "remove", and "replace" operations are accepted; "move", "copy", and "test" are intentionally rejected. This is durable and no longer points at a closed reference. Closes ory#1254 Signed-off-by: Mohamad Reza Chegini <mrchcoin@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA clarifying inline comment is added to ChangesApplyJSONPatch operation support documentation
Docker builder dependency manifests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@oryx/jsonx/patch.go`:
- Around line 63-64: Update the API contract documentation for the JSONPatch.Op
field in the openapix/jsonpatch.go file (around line 15) to clarify that only
the three operations "add", "remove", and "replace" are supported, rather than
documenting all six RFC 6902 operations as valid. This will align the API
documentation with the actual implementation behavior defined in the opAllowList
and error handling in patch.go, preventing confusion for API clients about which
operations are actually supported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2f47d92a-25f1-46aa-b5ff-6b7eaf8d422c
📒 Files selected for processing (1)
oryx/jsonx/patch.go
| // Only "add", "remove", and "replace" operations are supported; "move", | ||
| // "copy", and "test" are intentionally rejected. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the API contract file
echo "=== Checking oryx/openapix/jsonpatch.go ==="
if [ -f "oryx/openapix/jsonpatch.go" ]; then
wc -l oryx/openapix/jsonpatch.go
echo "--- Lines 1-50 of oryx/openapix/jsonpatch.go ---"
head -50 oryx/openapix/jsonpatch.go | cat -n
else
echo "File not found at oryx/openapix/jsonpatch.go"
find . -name "jsonpatch.go" -type f
fiRepository: ory/oathkeeper
Length of output: 1606
🏁 Script executed:
# Read the implementation file to verify opAllowList and error handling
echo "=== Checking oryx/jsonx/patch.go ==="
if [ -f "oryx/jsonx/patch.go" ]; then
wc -l oryx/jsonx/patch.go
echo "--- Lines 1-80 of oryx/jsonx/patch.go ---"
head -80 oryx/jsonx/patch.go | cat -n
else
echo "File not found at oryx/jsonx/patch.go"
find . -name "patch.go" -type f
fiRepository: ory/oathkeeper
Length of output: 2968
🏁 Script executed:
# Verify that there are no other usages or configurations that might support move/copy/test
echo "=== Searching for move/copy/test operation references ==="
rg "(move|copy|test)" oryx/jsonx/ -i --context=2
echo ""
echo "=== Checking if there are any comments or issues referencing the RFC 6902 restriction ==="
rg "RFC 6902|move|copy" --type go -iRepository: ory/oathkeeper
Length of output: 38146
Document mismatch between API contract and implementation.
The new comment accurately documents what the code does. However, it reveals a cross-file contract/behavior mismatch: the API contract in oryx/openapix/jsonpatch.go (line 15) documents the JSONPatch.Op field as supporting "add", "remove", "replace", "move", "copy", and "test" per RFC 6902. Yet the implementation intentionally rejects move/copy/test operations (via opAllowList at lines 17–21 and the error at line 66).
API clients reading the JSONPatchDocument documentation would expect those six operations to be valid, but ApplyJSONPatch will reject them with "unsupported operation" errors. This mismatch could break integrations or confuse new API users.
Update the API contract in oryx/openapix/jsonpatch.go to clarify that only "add", "remove", and "replace" are actually supported.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oryx/jsonx/patch.go` around lines 63 - 64, Update the API contract
documentation for the JSONPatch.Op field in the openapix/jsonpatch.go file
(around line 15) to clarify that only the three operations "add", "remove", and
"replace" are supported, rather than documenting all six RFC 6902 operations as
valid. This will align the API documentation with the actual implementation
behavior defined in the opAllowList and error handling in patch.go, preventing
confusion for API clients about which operations are actually supported.
The replace directives in go.mod point to ./middleware/rpctest and
./oryx. go mod download needs the go.mod and go.sum for each replaced
module to resolve dependencies, but only oryx/go.mod was copied and
oryx/go.sum was being written to the wrong path (proto/go.sum).
Add the missing middleware/rpctest/go.{mod,sum} COPY steps and fix
the oryx/go.sum destination so the scanner Docker build succeeds.
1 participant
What
Replace the stale upstream reference in
oryx/jsonx/patch.goflagged by #1254.Why
The comment pointed to
evanphx/json-patch#158, which has since been merged. ORY's Closed Reference Notifier flags merged and closed PRs alike, so it was reported as a stale reference (#1254).How
Replace the link with a durable description of the actual behaviour: only
add,remove, andreplaceoperations are accepted;move,copy, andtestare intentionally rejected (matchingopAllowList). This is a comment-only change — no behaviour change.gofmtandgo vet ./jsonx/are clean; the package compiles.Closes #1254
Summary by CodeRabbit
rpctestcomponent, improving dependency resolution consistency during builds.