test(mountsync): measure scoped team syncer behavior#227
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
📝 WalkthroughWalkthroughThis PR adds a new test file ChangesTeam-Scoped Syncer Integration Tests
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Relayfile Eval ReviewRun: Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesNo reviewable human-review cases captured Relayfile output. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/mountsync/team_scoped_measurement_test.go (1)
264-265: 💤 Low valueConsider allowing path-scoped
relayfile:fs:manage:grants.The production
scopeGrantsWrite(per context snippet 2) treatsmanageequivalently towritefor authorization. Rejecting allrelayfile:fs:manage:scopes—even path-scoped ones matching the assigned root—creates an inconsistency where a valid token withrelayfile:fs:manage:/project/cloud/packages/member-a/*would be rejected by the test harness but accepted by production.If this strictness is intentional for Tier-1, consider adding a comment explaining why manage scopes are excluded.
🤖 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 `@internal/mountsync/team_scoped_measurement_test.go` around lines 264 - 265, The test currently rejects any scope starting with "relayfile:fs:manage:" (case handling the strings.HasPrefix(scope, "relayfile:fs:manage:") branch) which is stricter than production's scopeGrantsWrite behavior; update the test to either (A) allow path-scoped manage grants that are scoped under the token's assigned root by performing the same path-prefix check you use for write scopes (treat "manage" the same as "write" when the remainder of the scope is a path under the assigned root), or (B) if you intentionally want the stricter policy for Tier‑1, replace this blanket rejection with a comment referencing the intentional divergence and why manage is excluded; adjust the logic in the function containing this case to mirror scopeGrantsWrite path-checking or add the explanatory comment accordingly.
🤖 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.
Nitpick comments:
In `@internal/mountsync/team_scoped_measurement_test.go`:
- Around line 264-265: The test currently rejects any scope starting with
"relayfile:fs:manage:" (case handling the strings.HasPrefix(scope,
"relayfile:fs:manage:") branch) which is stricter than production's
scopeGrantsWrite behavior; update the test to either (A) allow path-scoped
manage grants that are scoped under the token's assigned root by performing the
same path-prefix check you use for write scopes (treat "manage" the same as
"write" when the remainder of the scope is a path under the assigned root), or
(B) if you intentionally want the stricter policy for Tier‑1, replace this
blanket rejection with a comment referencing the intentional divergence and why
manage is excluded; adjust the logic in the function containing this case to
mirror scopeGrantsWrite path-checking or add the explanatory comment
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a8953ef0-ce07-4413-8664-3b26b3377e1c
📒 Files selected for processing (1)
internal/mountsync/team_scoped_measurement_test.go
|
Reviewed PR #227 and made one fix in team_scoped_measurement_test.go: the Tier-1 helper now rejects exact assigned-root write scopes without Local checks run with a temporary Go 1.22.12 toolchain: |
|
✅ pr-reviewer applied fixes — committed and pushed Reviewed PR #227 and made one fix in team_scoped_measurement_test.go: the Tier-1 helper now rejects exact assigned-root write scopes without Local checks run with a temporary Go 1.22.12 toolchain: |
There was a problem hiding this comment.
✅ pr-reviewer applied fixes — committed and pushed 0b99b84 to this PR. The notes below describe what changed.
Reviewed PR #227 and made one fix in team_scoped_measurement_test.go: the Tier-1 helper now rejects exact assigned-root write scopes without /*, because those tokens do not grant writes to child files under the syncer’s scope matching.
Local checks run with a temporary Go 1.22.12 toolchain:
go test ./internal/mountsync
go test ./internal/mountsync -run 'TestTeamTier1' -count=1 -timeout=30s
go test ./...
go build ./cmd/relayfile ./cmd/relayfile-mount ./cmd/relayfile-cli
go vet ./...
./scripts/check-contract-surface.sh
Summary
Scope label
Tier-1 GREEN means scoping-correctness only. It does NOT prove the full section-7 N-load gate. Tier-2 CF Durable Object saturation measurement remains open for <12-inflight / 429 Retry-After / no context-deadline / no object-reset.
Verification