Skip to content

refactor(difc): extract EvaluateCoarseAccess to eliminate duplicated Phase 2 logic#6639

Merged
lpcox merged 4 commits into
mainfrom
copilot/duplicate-code-resolution
May 28, 2026
Merged

refactor(difc): extract EvaluateCoarseAccess to eliminate duplicated Phase 2 logic#6639
lpcox merged 4 commits into
mainfrom
copilot/duplicate-code-resolution

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 28, 2026

The DIFC Phase 2 coarse-grained check block (EvaluateIsAllowedShouldBypassCoarseDeny → handle denial) was duplicated across internal/server/unified.go and internal/proxy/handler.go, with the proxy version already diverging (missing FormatViolationError and RecordError).

Changes

  • internal/difc/pipeline_decisions.go — New CoarseCheckOutcome type (CoarseAllowed / CoarseBypassForRead / CoarseDenied) and EvaluateCoarseAccess helper that encapsulates the full Phase 2 decision; returns the typed outcome plus the raw EvaluationResult so each caller can format its own denial response (MCP error vs HTTP 403)

  • internal/server/unified.go and internal/proxy/handler.go — Replace their local Evaluate + nested-if blocks with a switch on EvaluateCoarseAccess

  • internal/difc/pipeline_decisions_test.goTestEvaluateCoarseAccess covering all three outcomes

// Before (duplicated in two places, already diverged):
result := evaluator.Evaluate(agentLabels.Secrecy, agentLabels.Integrity, resource, operation)
if !result.IsAllowed() {
    if difc.ShouldBypassCoarseDeny(operation) { /* log + continue */ } else { /* block */ }
}

// After (shared decision, caller-specific response):
coarseOutcome, result := difc.EvaluateCoarseAccess(evaluator, agentLabels.Secrecy, agentLabels.Integrity, resource, operation)
switch coarseOutcome {
case difc.CoarseAllowed:      // log allowed
case difc.CoarseBypassForRead: // log + continue to Phase 3
case difc.CoarseDenied:        // caller-specific block response
}

Copilot AI changed the title [WIP] Fix duplicate code pattern in DIFC access check logic refactor(difc): extract EvaluateCoarseAccess to eliminate duplicated Phase 2 logic May 28, 2026
Copilot finished work on behalf of lpcox May 28, 2026 15:56
Copilot AI requested a review from lpcox May 28, 2026 15:56
@lpcox lpcox marked this pull request as ready for review May 28, 2026 16:18
Copilot AI review requested due to automatic review settings May 28, 2026 16:18
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 28, 2026

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Extracts the duplicated DIFC Phase 2 coarse-grained access check logic from internal/server/unified.go and internal/proxy/handler.go into a single shared helper EvaluateCoarseAccess in internal/difc/pipeline_decisions.go, returning a typed CoarseCheckOutcome so each caller still formats its own denial response (MCP error vs HTTP 403). This reduces drift between the server and proxy pipelines (the proxy version was already missing FormatViolationError / span error recording).

Changes:

  • Add CoarseCheckOutcome enum and EvaluateCoarseAccess helper centralizing Evaluate + ShouldBypassCoarseDeny.
  • Replace duplicated Phase 2 if/else blocks in unified.go and proxy/handler.go with a typed switch on the new outcome.
  • Add TestEvaluateCoarseAccess covering allowed/bypass-for-read/denied-write/denied-readwrite outcomes.
Show a summary per file
File Description
internal/difc/pipeline_decisions.go Introduces CoarseCheckOutcome and EvaluateCoarseAccess helper.
internal/difc/pipeline_decisions_test.go Adds table-driven test for the new helper across the three outcomes.
internal/server/unified.go Switches Phase 2 to use the shared helper; inadvertently swaps tracing.RecordSpanError for inline calls referencing an unimported codes package.
internal/proxy/handler.go Switches Phase 2 to the shared helper via a switch on the typed outcome.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 4/4 changed files
  • Comments generated: 1

Comment thread internal/server/unified.go Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox lpcox merged commit 7da2492 into main May 28, 2026
16 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-resolution branch May 28, 2026 16:27
Copilot stopped work on behalf of lpcox due to an error May 28, 2026 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] Duplicate Code Pattern: DIFC Phase 2 Coarse-Grained Access Check Block

3 participants