feat: add integrity-reactions feature flag for MCPG reaction-based integrity promotion/demotion#25948
feat: add integrity-reactions feature flag for MCPG reaction-based integrity promotion/demotion#25948
integrity-reactions feature flag for MCPG reaction-based integrity promotion/demotion#25948Conversation
Add new feature flag `integrity-reactions` that, when enabled, injects `endorsement-reactions` and `disapproval-reactions` fields into the MCPG allow-only integrity policy. Requires MCPG >= v0.2.18. Changes: - Add IntegrityReactionsFeatureFlag constant - Add MCPGIntegrityReactionsMinVersion = "v0.2.18" constant - Add new fields to GitHubToolConfig struct - Parse new reaction fields in tools_parser.go - Add mcpgSupportsIntegrityReactions() version gate helper - Add injectIntegrityReactionFields() helper for both code paths - Inject reactions into gateway allow-only policy (mcp_renderer_github.go) - Inject reactions into DIFC proxy policy (compiler_difc_proxy.go) - Add validateIntegrityReactions() validation - Wire up validation in both compiler paths - Update JSON schema with new fields - Add comprehensive unit tests Agent-Logs-Url: https://github.com/github/gh-aw/sessions/24601d6f-99dd-4b19-ac56-7c90f187f8e9 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
…mprove function doc Agent-Logs-Url: https://github.com/github/gh-aw/sessions/24601d6f-99dd-4b19-ac56-7c90f187f8e9 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
integrity-reactions feature flag for MCPG reaction-based integrity promotion/demotion
There was a problem hiding this comment.
Pull request overview
Adds an integrity-reactions feature flag to support maintainer reaction-based integrity promotion/demotion by injecting reaction-related fields into MCPG guard policy output, gated to MCPG >= v0.2.18.
Changes:
- Added new GitHub tool config fields (
endorsement-reactions,disapproval-reactions,disapproval-integrity,endorser-min-integrity) plus parsing and schema updates. - Implemented version-gated injection of these fields into rendered MCP gateway guard-policies and DIFC proxy policy JSON.
- Added validation + tests for the feature flag, MCPG version gate, and allowed enum values.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/tools_validation.go | Adds validation for integrity-reactions configuration (feature flag, version gate, enums). |
| pkg/workflow/tools_validation_test.go | Adds unit tests for version gating, validation, and DIFC proxy injection behavior. |
| pkg/workflow/tools_types.go | Extends GitHubToolConfig with reaction-based integrity fields. |
| pkg/workflow/tools_parser.go | Parses new reaction-based integrity fields from tools.github config. |
| pkg/workflow/mcp_renderer_github.go | Injects reaction fields into explicit allow-only guard policy before rendering. |
| pkg/workflow/mcp_github_config.go | Adds injection helper + MCPG version support check using semver compare. |
| pkg/workflow/compiler_string_api.go | Runs integrity-reactions validation during string-based workflow parsing. |
| pkg/workflow/compiler_orchestrator_workflow.go | Runs integrity-reactions validation during file-based workflow parsing. |
| pkg/workflow/compiler_difc_proxy.go | Extends DIFC proxy policy builder to accept workflow data/config and inject reaction fields. |
| pkg/workflow/compiler_difc_proxy_test.go | Updates DIFC proxy policy JSON tests for new function signature. |
| pkg/parser/schemas/main_workflow_schema.json | Adds schema properties/enums for the new GitHub reaction-based guard-policy fields. |
| pkg/constants/version_constants.go | Adds MCPGIntegrityReactionsMinVersion = v0.2.18. |
| pkg/constants/feature_constants.go | Adds IntegrityReactionsFeatureFlag = "integrity-reactions". |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 13/13 changed files
- Comments generated: 5
| // Validate integrity-reactions feature configuration | ||
| var gatewayConfig *MCPGatewayRuntimeConfig | ||
| if workflowData.SandboxConfig != nil { | ||
| gatewayConfig = workflowData.SandboxConfig.MCP | ||
| } | ||
| if err := validateIntegrityReactions(workflowData.ParsedTools, workflowData.Name, workflowData, gatewayConfig); err != nil { | ||
| return nil, fmt.Errorf("%s: %w", cleanPath, err) | ||
| } |
There was a problem hiding this comment.
validateIntegrityReactions is executed before imported features are merged (MergeFeatures happens later in this function). If a workflow enables integrity-reactions via an import, this validation will incorrectly fail because isFeatureEnabled() won’t see the merged flag yet. Move this validation to after the features merge (or validate against the merged feature set).
| // Validate integrity-reactions feature configuration | ||
| var gatewayConfig *MCPGatewayRuntimeConfig | ||
| if workflowData.SandboxConfig != nil { | ||
| gatewayConfig = workflowData.SandboxConfig.MCP | ||
| } | ||
| if err := validateIntegrityReactions(workflowData.ParsedTools, workflowData.Name, workflowData, gatewayConfig); err != nil { | ||
| return nil, fmt.Errorf("%s: %w", cleanPath, err) | ||
| } |
There was a problem hiding this comment.
validateIntegrityReactions runs before features from imports are merged (MergeFeatures is later). This can reject workflows that enable integrity-reactions through an imported workflow because isFeatureEnabled() won’t observe the merged feature flags yet. Run this validation after feature merging (or validate against the merged feature map).
| if strings.EqualFold(version, "latest") { | ||
| return true | ||
| } | ||
|
|
There was a problem hiding this comment.
mcpgSupportsIntegrityReactions() claims non-semver versions should return false, but it unconditionally calls semverutil.Compare(version, minVersion). semverutil.Compare delegates to x/mod/semver.Compare without validating inputs, so non-semver strings (e.g. branch names) may compare unpredictably and could incorrectly pass the gate. Add an explicit semverutil.IsValid(version) check (after handling "latest") and return false when invalid.
| if !semverutil.IsValid(version) { | |
| return false | |
| } |
pkg/workflow/tools_validation.go
Outdated
| // Reaction fields require min-integrity to be set | ||
| if (hasEndorsementReactions || hasDisapprovalReactions) && github.MinIntegrity == "" { | ||
| toolsValidationLog.Printf("endorsement-reactions/disapproval-reactions without min-integrity in workflow: %s", workflowName) | ||
| return errors.New("invalid guard policy: 'endorsement-reactions' and 'disapproval-reactions' require 'github.min-integrity' to be set") |
There was a problem hiding this comment.
validateIntegrityReactions currently allows setting 'disapproval-integrity' and/or 'endorser-min-integrity' without 'github.min-integrity'. In that case no allow-only guard policy is rendered, so these fields are silently ignored (they’re only injected into an existing allow-only policy). Either require github.min-integrity whenever any integrity-reactions field is set, or ensure a guard policy is created/injected in the guard-from-step path so the config actually takes effect.
| // Reaction fields require min-integrity to be set | |
| if (hasEndorsementReactions || hasDisapprovalReactions) && github.MinIntegrity == "" { | |
| toolsValidationLog.Printf("endorsement-reactions/disapproval-reactions without min-integrity in workflow: %s", workflowName) | |
| return errors.New("invalid guard policy: 'endorsement-reactions' and 'disapproval-reactions' require 'github.min-integrity' to be set") | |
| // Integrity reaction fields require min-integrity to be set so the allow-only guard policy is rendered | |
| if (hasEndorsementReactions || hasDisapprovalReactions || hasDisapprovalIntegrity || hasEndorserMinIntegrity) && github.MinIntegrity == "" { | |
| toolsValidationLog.Printf("integrity reaction fields without min-integrity in workflow: %s", workflowName) | |
| return errors.New("invalid guard policy: 'endorsement-reactions', 'disapproval-reactions', 'disapproval-integrity', and 'endorser-min-integrity' require 'github.min-integrity' to be set") |
| // Reaction fields require the integrity-reactions feature flag | ||
| if !isFeatureEnabled(constants.IntegrityReactionsFeatureFlag, data) { | ||
| toolsValidationLog.Printf("Reaction fields present but integrity-reactions feature flag not enabled in workflow: %s", workflowName) | ||
| return errors.New("invalid guard policy: 'endorsement-reactions', 'disapproval-reactions', 'disapproval-integrity', and 'endorser-min-integrity' require the 'integrity-reactions' feature flag to be enabled. Add 'features: integrity-reactions: true' to your workflow") |
There was a problem hiding this comment.
The error message suggests adding "features: integrity-reactions: true", which is not valid YAML for this repo’s frontmatter style (it should be nested under features with proper indentation). Consider updating the message to show a correct YAML snippet (e.g. features:\n integrity-reactions: true) and include the full key path (tools.github.*) for clarity.
| return errors.New("invalid guard policy: 'endorsement-reactions', 'disapproval-reactions', 'disapproval-integrity', and 'endorser-min-integrity' require the 'integrity-reactions' feature flag to be enabled. Add 'features: integrity-reactions: true' to your workflow") | |
| return errors.New("invalid guard policy: 'tools.github.endorsement-reactions', 'tools.github.disapproval-reactions', 'tools.github.disapproval-integrity', and 'tools.github.endorser-min-integrity' require the 'integrity-reactions' feature flag to be enabled. Add the following to your workflow frontmatter:\nfeatures:\n integrity-reactions: true") |
🧪 Test Quality Sentinel ReportTest Quality Score: 90/100✅ Excellent test quality
Test Classification Details
Flagged Tests — Requires ReviewNo tests are flagged for behavioral quality issues. i️ Test Inflation Note (
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Generated by Design Decision Gate workflow. Records the decision to introduce the integrity-reactions feature flag with a semver version gate (MCPG >= v0.2.18) and a shared injection helper across all policy code paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Commit pushed:
|
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (662 new lines in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: The draft captures the following key design decisions identified in the diff:
What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. Why ADRs MatterADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
Note 🔒 Integrity filter blocked 1 itemThe following item were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
- Move validateIntegrityReactions after MergeFeatures in both compiler_string_api.go and compiler_orchestrator_workflow.go so imported feature flags are visible during validation - Add semverutil.IsValid() guard in mcpgSupportsIntegrityReactions to safely handle non-semver strings (branch names, etc.) - Extend min-integrity requirement to all 4 integrity-reactions fields - Fix error message to show correct YAML multi-line format - Update tests to reflect new behavior Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c9ae237b-f229-4918-98e7-7d057c6618ca Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Review of PR #25948 — integrity-reactions feature flagOverall AssessmentSolid implementation that follows existing patterns well. The Issues
Upcoming ChangePer user request,
This means enabling |
- endorsement-reactions defaults to [THUMBS_UP, HEART] when integrity-reactions feature flag is enabled - disapproval-reactions defaults to [THUMBS_DOWN, CONFUSED] - Reaction-based integrity is only enforced in proxy mode (DIFC/CLI proxy), not MCP gateway mode, because the GitHub MCP server protocol does not expose reaction author information - Compiler emits a warning when reactions are configured with the gateway path - Validation now requires min-integrity when feature flag is enabled (even without explicit reaction lists, since defaults will be injected) - Schema updated: removed minItems constraint, added default values - Updated all type docs and schema descriptions to clarify proxy-only Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@pelikhan please take a look; feature flag to use maintainers' reactions on github.com to promote or demote objects' integrity when cli-proxy is enabled. |
|
Hey
Verdict: 🟢 Aligned — looks ready for maintainer review! The implementation faithfully executes the plan from #25947: constants, parsing, validation with semver-gated version check, proxy-only injection with gateway warning, schema updates, and an ADR. Backward compatibility is preserved (feature flag defaults off,
|
Adds
integrity-reactionsfeature flag that injectsendorsement-reactionsanddisapproval-reactionsinto the MCPG allow-only policy, enabling maintainer reaction-based integrity promotion/demotion (gh-aw-mcpg#3618). Version-gated to MCPG >= v0.2.18.Reaction-based integrity is only enforced in proxy mode (DIFC proxy / CLI proxy). In MCP gateway mode, reaction authors cannot be identified from the GitHub MCP server protocol, so reactions are ignored for integrity decisions. The compiler emits a warning when reactions are configured with the gateway path.
New frontmatter syntax
Enabling
integrity-reactions: truewithmin-integrityset is sufficient — all four reaction fields are optional and receive sensible defaults at compile time.Changes
IntegrityReactionsFeatureFlag = "integrity-reactions"andMCPGIntegrityReactionsMinVersion = "v0.2.18"GitHubToolConfig(EndorsementReactions,DisapprovalReactions,DisapprovalIntegrity,EndorserMinIntegrity) parsed intools_parser.goDefaultEndorsementReactions = ["THUMBS_UP", "HEART"]andDefaultDisapprovalReactions = ["THUMBS_DOWN", "CONFUSED"]applied automatically when the feature flag is enabled but lists are not explicitly setinjectIntegrityReactionFields()helper called in the proxy code path only:compiler_difc_proxy.go) —getDIFCProxyPolicyJSON()now acceptsdata+gatewayConfigto support injection with defaultsmcp_renderer_github.gono longer injects reactions; instead emits a compile-time warning when reactions are configured with the gateway path, explaining that reaction authors cannot be identified from the GitHub MCP servermcpgSupportsIntegrityReactions(gatewayConfig)uses semver comparison; default MCPG version (v0.2.17) is below the minimum, so existing workflows are unaffectedvalidateIntegrityReactions()enforces: feature flag required when reaction fields are set, MCPG >= v0.2.18,min-integrityrequired when feature flag is enabled (even without explicit reaction lists since defaults will be injected), validReactionContentenum values, valid integrity level stringsendorsement-reactions,disapproval-reactions,disapproval-integrity,endorser-min-integrityadded tomain_workflow_schema.jsonwith enum constraints, default values, and proxy-only documentationBackward compatibility
Feature flag defaults off.
make recompileproduces no changes to existing.lock.ymlfiles.Closes #25947