fix: depth decrementing was passed by reference and all same depth ch…

[ucatbas]

…ecks were decreasing the number. passing copy of request when nested

Summary by CodeRabbit

• Bug Fixes

  • Prevented unintended mutation of incoming check requests so the original request remains unchanged during processing.
  • Adjusted depth validation so zero depth is accepted (only negative depths are rejected).

• Tests

  • Added depth-check coverage including passing and failing scenarios for multi-level permission checks to validate depth behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Adjusts check depth handling and request mutation: DirectInvoker.Check now copies the request and decrements depth on the copy; depth validation treats zero as valid (only negative depths rejected); adds tests covering 3-level depth behavior (allowed/denied cases).

Changes

Cohort / File(s)	Summary
Request mutation refactor internal/invoke/invoke.go	DirectInvoker.Check now creates a copy of the request (nextRequest) and decrements Metadata.Depth on the copy before invoking the downstream check, avoiding mutation of the original request.
Depth validation logic internal/invoke/utils.go	Changed depth validation from <= 0 to < 0 — zero depth is now considered valid; updated comment accordingly.
Depth behavior tests internal/engines/check_test.go	Added "Depth Check Sample" tests: a 3-level schema and two cases (depth=3 expected ALLOWED; depth=2 expected DENIED with ERROR_CODE_DEPTH_NOT_ENOUGH).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix: checkDepth should throw error if depth <= 0 #1989 — Modifies checkDepth in internal/invoke/utils.go; similar area but changes depth validation in the opposite direction.
• fix(depth-check): correctly handle 'depth not enough' error condition #1707 — Also touches depth-handling and Check invocation logic in internal/invoke, related to request depth decrement and validation.

Poem

I nibble through requests with tiny care, 🐇
I copy the depth, so originals spare,
Zero's now welcome — negatives flee,
Tests hop along a three-level tree,
A rabbit's small cheer for clean state harmony! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main bug fix: addressing a depth decrementing issue caused by passing by reference. The changes across all files align with this core fix.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.61%. Comparing base (af5ccc9) to head (0911720).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2740      +/-   ##
==========================================
- Coverage   82.63%   82.61%   -0.01%     
==========================================
  Files          74       74              
  Lines        8125     8125              
==========================================
- Hits         6713     6712       -1     
- Misses        893      894       +1     
  Partials      519      519              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[wied03]

mainly just questions

[wied03]

So we're using the "strict" mocking here? Where we don't just stub things, but expect things to be called in a certain order, etc. ?

[ucatbas]

yes

[wied03]

what happens if we do mutate the original?

[ucatbas]

the depth is decremented on every deeper call. previously if a lower level had multiple leaves, it ended up decreasing the depth multiple times, when it should have only decreased once

[wied03]

Did these 2 tests fail, once written, before the fixes were made in invoke.go and utils.go?

[ucatbas]

before the changes, the case that should pass was failing. i added these tests because this scenario specifically requires a depth of 3. i included both failing and passing cases to verify the behavior.