[Rul1an]

Adds a "Known issues" section under [Unreleased] for 6.16.0. It covers the decision outcome label on plain MCP proxy allows: such an allow is recorded as obligation_skipped from the obligation_executor even when no obligation exists. The note tells readers how to interpret those records: when the same record has obligation_skipped_present: false, it is a plain policy allow.

The fix is #3406. It changes a public enum, so it is parked for the next major release. That decision is recorded on #2959 (#2959 (comment)).

Docs only, one file (CHANGELOG.md).

• Head SHA: 4f64c4042ec27377b135799c68d2695907bac73e
• Builder: Claude Code (claude/ branch). Review: pending; one non-building review is needed on this head.

🤖 Generated with Claude Code

Updated to head dfd64d6e939fc411d3d890971852def453527978 (merge of main after #3408); the review on 4f64c4042ec27377b135799c68d2695907bac73e carries per the checker.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Rul1an/assay
• Branch: claude/changelog-policy-allow-known-issue
• File: .coderabbit.yaml
• Commit: 4f64c40
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9c07b52d-1743-44da-88b4-339c9491177f

📥 Commits

Reviewing files that changed from the base of the PR and between 62f857e and 4f64c40.

📒 Files selected for processing: 1

• CHANGELOG.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Summary

Summary by CodeRabbit

• Documentation
  • Added a known-issue note explaining that an MCP proxy decision labeled obligation_skipped may still represent a plain policy allow when obligation_skipped_present is false.
  • The note clarifies that the label affects a public enum and is tracked for a future major release.

The Unreleased changelog adds a Known issues entry about MCP proxy decision records. It explains how to interpret an allow with no obligation and tracks the public enum correction for the next major release.

Changes: MCP proxy decision records

Layer / File(s)	Summary
Document decision record interpretation CHANGELOG.md	Adds guidance that a record labeled obligation_skipped should be read as a plain policy allow when obligation_skipped_present is false. It tracks the public enum correction for the next major release.

Priority: ⬇️ Low

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

Change: Other

Possibly related PRs

• Rul1an/assay#3406: Implements the decision-record behavior described in the changelog entry.

Suggested labels: area:docs

Suggested reviewers: codex

Merge Risk: ⚪ Minimal · up to 4f64c

This documentation-only change introduces no identified merge-blocking risk.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly identifies the changelog update and the documented obligation_skipped label issue for plain allows.
Docstring Coverage		No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description states the PR adds a Known issues note to CHANGELOG.md under [Unreleased] for 6.16.0. It describes the mislabeled MCP proxy allow records (obligation_skipped from obligation_executor),…

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

Full details: Description check

Explanation

The description states the PR adds a Known issues note to CHANGELOG.md under [Unreleased] for 6.16.0. It describes the mislabeled MCP proxy allow records (obligation_skipped from obligation_executor), the interpretation rule using obligation_skipped_present: false, and the deferred fix (#3406) that changes a public enum for the next major release. This matches the raw summary, so the description is clearly related to the changeset.

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

[Rul1an]

Non-building review of head 4f64c4042ec27377b135799c68d2695907bac73e. Parent 62f857ea0d6f41ee3b1ab324ff27bc5b41743fe9 is main. The diff is CHANGELOG.md only. I did not write this commit, and I did not run cargo.

I was one of the read-only consultants on the release decision this note records. On the #3406 shape, the consultation chose a new PolicyAllow variant, a basis bump, and leaving stored logs unchanged, and a later review of an earlier #3406 head came back blocked on the semver gate. I did not author #2959, #3406, or this changelog. The sentences below are checked against the code on this head.

The sentence that might mislead

CHANGELOG.md:16-19 says an allow that had no obligation is labeled decision_outcome_kind: obligation_skipped with decision_origin: obligation_executor, and that when that same record has obligation_skipped_present: false, read it as a plain policy allow.

The case against the second sentence is that the flag is not "this was a plain allow". refresh_fulfillment_normalization sets it from the outcome list alone (crates/assay-core/src/mcp/decision_next/normalization.rs:72-76): it is true when any outcome status is Skipped. An allow that had obligations and applied them also has the flag false, because those outcomes are Applied.

The handler emits that shape. test_event_contains_required_fields (crates/assay-core/tests/decision_emit_invariant/emission.rs:313-398) is a tool-call allow: typed_decision is AllowWithObligations, the obligations list is non-empty, the outcome is an applied log, decision_outcome_kind is ObligationApplied, decision_origin is ObligationExecutor, and obligation_skipped_present is Some(false). test_allow_with_warning_emits_log_obligation_outcome (crates/assay-core/src/mcp/tool_call_handler/tests/emission.rs:80-105) is the same warning path and asserts the flag false. A deny with an applied outcome does it too (crates/assay-core/tests/fulfillment_normalization.rs:50-52). I did not execute these tests; the assertions are in the source on this head.

Error outcomes are the same trap from the other side. classify_decision_outcome takes ObligationError before the residual arm (crates/assay-core/src/mcp/decision/outcome_convergence.rs:67-71). An allow with an error outcome is not labeled obligation_skipped, and if nothing was skipped the flag is still false.

A constructed record wears AllowWithObligations and still falls into the residual bucket. allow_event (crates/assay-core/tests/replay_diff_contract.rs:10-34) sets typed_decision: AllowWithObligations on a default context (empty obligations, empty outcomes) and the normalizer reports ObligationSkipped. with_policy_context refreshes again (crates/assay-core/src/mcp/decision_next/builder.rs:311). I did not run it. An empty outcome list is what refresh_fulfillment_normalization turns into obligation_skipped_present: Some(false). A loose reading of the changelog would call that record a plain policy allow while its typed decision says otherwise.

Why the sentence still holds

The second sentence does not say the flag alone means a plain allow. "The same record" is the record in the previous sentence: one already labeled obligation_skipped from obligation_executor.

On an Allow, that kind is only the residual arm, and the skipped flag is an unused parameter (outcome_convergence.rs:44, outcome_convergence.rs:66-82). An applied outcome takes ObligationApplied. An error outcome takes ObligationError. The emission record above is the counterexample to the flag, and it is also why the kind splits them: its kind is ObligationApplied, so it is not the record the sentence is about.

Inside the residual bucket the flag is the right split. The only statuses are Applied, Skipped, and Error (crates/assay-core/src/mcp/decision_next/event_types.rs:125-129). Residual kind plus obligation_skipped_present: false means the outcome list is empty. On the proxy path that list is execute_log_only of the obligation list (crates/assay-core/src/mcp/tool_call_handler/evaluate.rs:82-83, crates/assay-core/src/mcp/proxy/decisions.rs:109-110), one outcome per obligation (crates/assay-core/src/mcp/obligations.rs:21-111). An empty outcome list is an empty obligation list. A real skip (restrict_scope, redact_args, approval_required, or an unknown type left at Skipped) keeps the residual kind and sets the flag true, so the sentence does not call it a plain allow.

The replay fixture does not break that. Its obligations vec is empty; only the typed enum says AllowWithObligations. The policy engine does not emit that pair. AllowWithWarning always attaches a legacy_warning (crates/assay-core/src/mcp/policy/contracts.rs:59-61), and execute_log_only turns that into an applied log (obligations.rs:59-74), which classifies as ObligationApplied. I did not find a production constructor that sets typed_decision to AllowWithObligations with an empty outcome list. The assignments under crates/assay-core/src/mcp are finalize_evaluation (crates/assay-core/src/mcp/policy/engine_next/diagnostics.rs:15-16), the handler copy (crates/assay-core/src/mcp/tool_call_handler/emit.rs:57), and the proxy copy (decisions.rs:101). PolicyDecision::Allow itself maps to typed Allow with an empty obligation vec (contracts.rs:55-58).

For a record this tree labels obligation_skipped, obligation_skipped_present: false is a plain policy allow. The first sentence is the same fact: a no-obligation allow takes the residual arm and origin ObligationExecutor (outcome_convergence.rs:77-81). That is the behavior [Unreleased] will ship in 6.16.0. Workspace Cargo.toml is still 6.15.0; this head's parent is main.

The rest of the note

The new block is ### Known issues (CHANGELOG.md:15). The file's other level-3 labels of this kind are ### Known follow-ups (CHANGELOG.md:2911) and ### Known issue with the ... (CHANGELOG.md:2988, CHANGELOG.md:3100). Same heading level, same "Known …" wording. The plural matches Known follow-ups rather than the singular incident headings.

#3406 is the parked fix. It adds public DecisionOutcomeKind::PolicyAllow. The enum is re-exported (crates/assay-core/src/mcp/decision.rs:35) and is not #[non_exhaustive] (outcome_convergence.rs:5-14). That is the public-enum change the note names.

"Tracked for the next major release" does not date that major, does not call 6.16.0 a major, and does not put the fix in [Unreleased]. The comment the PR cites exists: #2959 (comment) (Rul1an, 2026-10-23). It records #3406 as the first accepted break, says the next release stays on 6.x, and parks the fix until a later major. The changelog does not go past that.

CI at this head

Required contexts on main: CI, host-capability-check, lane-check/proof, review-record-check.

• host-capability-check: pass
• lane-check/proof: pass
• review-record-check: fail, because no record was on this head yet. Outside the changelog text.
• CI: pending. Pending is not green. Outside this docs diff.

Verdict: READY.

[Rul1an]

{
  "schema": "assay.review-record.v0",
  "head_sha": "4f64c4042ec27377b135799c68d2695907bac73e",
  "builder": {"agent": "claude", "instance": "changelog-policy-allow-known-issue"},
  "reviewer": {"agent": "cursor-grok", "instance": "review-3407-4f64c404", "github_login": "Rul1an"},
  "review_completed": true,
  "verdict": "READY",
  "findings": [
    {
      "id": "F1",
      "summary": "obligation_skipped_present false is not by itself a plain allow: an applied AllowWithObligations record carries that flag with kind obligation_applied. Bound to a record already labeled obligation_skipped, the flag false means an empty outcome list, which this tree's proxy path emits only for an allow with no obligation. The changelog sentence has that antecedent, so it holds for the 6.16.0 line.",
      "disposition": "verified"
    }
  ],
  "no_findings": false,
  "independence": {"did_not_build": true, "did_not_author_governing_spec": true}
}

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.