Repository navigation
Conversation
…ason A call refused by a profile's tier cap, deny rule or unannotated-tool policy was written to the audit log as authz deny / reason other, while the activity log carried block_reason profile_tier|profile_rule| profile_unannotated. The typed cause reached the funnel but not auditAuthz. Pass it through: the three tool-policy causes are now authz reasons (profile_tier, profile_rule, profile_unannotated, disclosed:true), in the top-level gates, direct mode and replay (one funnel) and in the nested code_execution path, which used to write token_scope / disclosed:false for a refusal the script reads in full. profile_scope is unchanged; profile_code_execution and profile_management are not audit reasons (built-in invocations are not audited per call). Schema enum, audit-log docs, the Spec 107 contract and CHANGELOG updated. Fixes smart-mcp-proxy#1703
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Dumbris
left a comment
Dumbris
left a comment
There was a problem hiding this comment.
Verified on a merge with main: build, CI lint config, targeted race run of the audit/profile/code_execution/direct/replay tests; cross-reviewed clean. Three specific reasons, profile_scope unchanged, nested code_execution path folded in, both schema copies updated identically, as discussed on #1703. The earlier red E2E job was the main-branch flake fixed in #1733.
|
Merged, and #1703 is closed with it. Thank you, @james-crowley! The audit stream now says why a profile refused a call ( Contributions like this are what keep MCPProxy moving, and they matter a lot to the project. One small ask, only if you feel like it: a short public note about your change (a tweet, a LinkedIn post, a line in a community you are part of) helps more people discover MCPProxy. No pressure at all, and I would be glad to repost it. |
## Description `TestInformationalRescan_BusyServerIsRetriedNotDropped` (Spec 118, #1689) failed on `main` in 4 of the last 8 unit-test runs and was the only red check on contributor PRs #1727, #1725 and #1715. **Cause.** `fireInformationalRescan` deletes the server's entry from `infoRescanTimers` when the debounce timer fires and re-creates it only after the gates and the claim ran. On every busy fire there is a short window in which "is a timer armed" is false. The test sampled the map at one fixed instant (`time.Sleep(4 * debounce)`), which under CI load lands in that window. Reproduced locally: 1 failure in 150 runs with `GOMAXPROCS=1` under 2×ncpu CPU burners. **Fix (test only).** Read `infoRescanSeq` under the mutex before scheduling; the change itself takes `before+1`, so `require.Eventually(rescanSeq("srv") > before+1)` is a timing-free proof that a busy fire re-armed the timer. The "no scan attempted on top of a claimed slot" and "a busy slot is not a failed attempt" assertions are unchanged. **Verification.** 400 runs of the fixed test under the same CPU contention: 0 failures. `go test -race -count=3 -run TestInformationalRescan ./internal/server/` green. `golangci-lint` with the CI v2 config clean. Cross-reviewed with opencode (gpt-6.1-sol), two rounds: round 1 found the baseline read could hit the same gap (fixed by reading the counter before scheduling), round 2 CLEAN. No production code changes. ## Second flake, same PR `TestRefreshConcurrency_ScansDuringASlowSelfCheckSeeOldOrNew` (from #1720) failed the darwin-amd64 Build Binaries job on this PR: `119.6ms is not less than 109.7ms (baseline median 1.19ms)`. Both refresh-concurrency guards compared the single worst of 300 scans against `50*median+50ms`, which one GC pause on a macOS runner exceeds. They now bound p90 relative to the baseline and keep an absolute 2 s ceiling on the worst scan (no scheduler stall reaches it, any wait behind the gate or a fetch deadline does). 30 runs under CPU contention: 0 failures. opencode Sol 6.1 round 1 flagged that p90 alone could hide one scan absorbing a timed-out wait; the absolute ceiling is the answer to that.
Description
Fixes #1703. A call refused by a profile's tier cap, deny rule or unannotated-tool policy was audited as
authz deny/reason: other, while the activity log carriedblock_reason: profile_tier|profile_rule|profile_unannotated. The typed cause reachedemitActivityPolicyDecisionWithBlockReasonbut neverauditAuthz.authzreasons (profile_tier,profile_rule,profile_unannotated,disclosed: true) through the one funnel, so top-levelcall_tool_*, direct mode and replay all get them.code_executionpath usedtoken_scopewithdisclosed: falsefor a refusal the script reads in full. It now usesreport.BlockReasonfor the same three reasons. Its other refusals are unchanged.profile_scopeis unchanged.profile_code_executionandprofile_managementare not audit reasons (built-in invocations never reach the audit stream), as discussed in the issue. They still map tootherif they ever arrive.telemetry.BlockReason*is not touched: the availability counters keep their fixed enum.docs/schemas/audit-line-v1.schema.jsonand the Spec 107 contract schema (the schema tests check both), plusaudit-log.md, the contract doc and the CHANGELOG. This is an additive change to thereasonenum, so a consumer that validates audit lines against it must accept the new members; the CHANGELOG says so.Testing
New tests (
internal/server/audit_profile_reason_test.go): a table for the mapping, top-level and nested refusals for each of the three reasons (validated against the contract schema), and an allowed call that is stillallow. The top-level tests fail without the fix (other).go test ./internal/audit ./internal/jsruntime ./internal/telemetrypass. Ininternal/serverthe audit, profile, code-execution, direct, replay, activity and scope tests pass exceptTestCodeExecClientModeE2E/client_mode_when_daemon_running, which also fails on a cleanmainin my environment (daemon not ready after 15s). I'm relying on CI for the rest of the package.