fix: correct integrity level descriptions and auto-enable cli-proxy for reactions#26154
fix: correct integrity level descriptions and auto-enable cli-proxy for reactions#26154
Conversation
…or reactions Three changes: 1. Fix incorrect min-integrity: unapproved descriptions in the maintaining-repos guide. The guide incorrectly described unapproved as allowing 'all community content' including 'everyone', which is the behavior of min-integrity: none. Fixed to accurately describe unapproved as including CONTRIBUTOR and FIRST_TIME_CONTRIBUTOR while filtering out FIRST_TIMER and NONE association. 2. Compiler: implicitly enable cli-proxy when integrity-reactions feature flag is set. Reaction-based integrity decisions require the proxy to identify reaction authors, so cli-proxy must be active. This avoids requiring users to set both features.integrity-reactions: true and features.cli-proxy: true manually. 3. Docs: rewrite integrity-reactions section to describe compiler defaults rather than showing explicit frontmatter for endorsement-reactions, disapproval-reactions, endorser-min-integrity, and disapproval-integrity. Note availability in v0.68.2 and clarify that reactions only work in proxy mode (not gateway mode). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Since the compiler now implicitly enables cli-proxy when integrity-reactions: true is set, the warning about proxy mode is unnecessary — reactions are guaranteed to run through the proxy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes integrity filtering documentation inaccuracies and updates the compiler to better support reaction-based integrity decisions by ensuring CLI-proxy-related components are included when integrity-reactions is enabled.
Changes:
- Corrects
min-integrity: unapproveddescriptions in the maintainer guide. - Updates Docker image collection and CLI-proxy “needed” logic to consider
integrity-reactions. - Adds unit tests covering implicit CLI-proxy enablement behavior.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/docker.go |
Pulls the cli-proxy AWF image when integrity-reactions (or cli-proxy) is enabled. |
pkg/workflow/compiler_difc_proxy.go |
Treats integrity-reactions as implying CLI-proxy is needed (when firewall + version support). |
pkg/workflow/compiler_difc_proxy_test.go |
Adds test cases for implicit CLI-proxy enablement via integrity-reactions. |
docs/src/content/docs/guides/maintaining-repos.md |
Fixes unapproved integrity descriptions and rewrites the reactions section to describe compiler defaults. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
docs/src/content/docs/guides/maintaining-repos.md:229
- This section states the compiler will automatically enable the CLI proxy and that a warning is logged when
integrity-reactionsis enabled in gateway mode. With the current code changes, onlyisCliProxyNeeded/image pulling considerintegrity-reactions; other behavior is still gated byfeatures.cli-proxy, and gateway-mode warnings currently only trigger when reaction fields are explicitly present in the tool config. Either adjust the docs to match actual behavior, or expand the compiler/runtime sointegrity-reactionstruly forces proxy mode (and emits the described warning when it can’t).
The compiler handles the rest — when `integrity-reactions: true` is set, it automatically:
- Enables the CLI proxy (`cli-proxy: true`), which is required for reaction-based integrity decisions
- Injects default endorsement reactions: `THUMBS_UP`, `HEART`
- Injects default disapproval reactions: `THUMBS_DOWN`, `CONFUSED`
- Uses `endorser-min-integrity: approved` (only reactions from owners, members, and collaborators count)
- Uses `disapproval-integrity: none` (a disapproval reaction demotes content to `none`)
These defaults mean that when a trusted member (owner, member, or collaborator) adds a 👍 or ❤️ reaction to an issue or comment, the item's integrity is promoted to `approved` — making it visible to agents using `min-integrity: approved`. Conversely, a 👎 or 😕 reaction from a trusted member demotes the item to `none`.
See the [Integrity Filtering Reference](/gh-aw/reference/integrity/) for complete configuration details.
## Scaling Strategies
pkg/workflow/compiler_difc_proxy.go:361
isCliProxyNeedednow treatsintegrity-reactionsas implicitly enabling the CLI proxy, but the rest of the compilation path still gates CLI-proxy behavior onfeatures.cli-proxyonly (e.g., AWF args--difc-proxy-host/--difc-proxy-ca-cert, skipping GitHub MCP server registration, gh CLI guidance, GH_TOKEN exclusion). As-is, a workflow with onlyintegrity-reactions: truewill start the host proxy step but AWF will not start/connect the cli-proxy sidecar, so reaction-based integrity decisions still won't be effective. Consider introducing a single "effectiveCliProxyEnabled" check (cli-proxy || integrity-reactions) and using it everywhere the cli-proxy flag is currently checked, or actually injectingcli-proxy: trueintoWorkflowData.Featuresduring compilation whenintegrity-reactionsis enabled.
cliProxyEnabled := isFeatureEnabled(constants.CliProxyFeatureFlag, data)
integrityReactionsEnabled := isFeatureEnabled(constants.IntegrityReactionsFeatureFlag, data)
if !cliProxyEnabled && !integrityReactionsEnabled {
return false
}
if integrityReactionsEnabled && !cliProxyEnabled {
difcProxyLog.Print("integrity-reactions enabled: implicitly enabling CLI proxy")
}
if !isFirewallEnabled(data) {
return false
}
firewallConfig := getFirewallConfig(data)
if !awfSupportsCliProxy(firewallConfig) {
difcProxyLog.Printf("Skipping CLI proxy: AWF version too old")
return false
}
return true
- Files reviewed: 4/4 changed files
- Comments generated: 2
| // Add cli-proxy sidecar container when the cli-proxy is needed (explicitly via | ||
| // cli-proxy feature flag, or implicitly via integrity-reactions feature flag) | ||
| // and the AWF version supports it. Without this, --skip-pull causes AWF to fail | ||
| // because the cli-proxy image was never pulled. | ||
| if isFeatureEnabled(constants.CliProxyFeatureFlag, workflowData) && awfSupportsCliProxy(firewallConfig) { | ||
| cliProxyNeeded := isFeatureEnabled(constants.CliProxyFeatureFlag, workflowData) || | ||
| isFeatureEnabled(constants.IntegrityReactionsFeatureFlag, workflowData) | ||
| if cliProxyNeeded && awfSupportsCliProxy(firewallConfig) { |
There was a problem hiding this comment.
collectDockerImages uses the same cli-proxy || integrity-reactions condition as isCliProxyNeeded, but other runtime wiring for cli-proxy is still gated solely on features.cli-proxy (AWF flags, env exclusions, etc.). Until those other call sites also treat integrity-reactions as enabling cli-proxy, this can lead to pulling the cli-proxy image even though the cli-proxy sidecar won't actually be started/used. Align the condition across the whole cli-proxy pipeline (ideally via a shared helper) so image pulling matches actual runtime usage.
| func TestIsCliProxyNeeded_IntegrityReactionsImplicitEnable(t *testing.T) { | ||
| awfVersion := "0.25.20" | ||
|
|
||
| tests := []struct { | ||
| name string |
There was a problem hiding this comment.
This test calls isFeatureEnabled via isCliProxyNeeded, which can read GH_AW_FEATURES from the environment. To keep the test hermetic (and avoid failures when GH_AW_FEATURES is set in a developer’s shell/CI), explicitly clear or set GH_AW_FEATURES (e.g., t.Setenv("GH_AW_FEATURES", "")) at the start of the test.
…pages The integrity-reactions feature (features.integrity-reactions: true, available from gh-aw v0.68.2) was documented in the maintaining-repos guide by #26154 but was missing from the reference pages that it points to for "complete configuration details". - reference/integrity.md: add four reaction fields to configuration table (endorsement-reactions, disapproval-reactions, endorser-min-integrity, disapproval-integrity); add "Promoting and demoting items via reactions" subsection; add a reactions example - reference/frontmatter.md: add integrity-reactions under Feature Flags with a cross-reference to the integrity reference Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Three related fixes for integrity filtering documentation and compiler behavior:
1. Fix incorrect
min-integrity: unapproveddescriptionsThe maintaining-repos guide (merged in #26073) incorrectly described
unapprovedas allowing "all community content — first-time contributors, external users, everyone." That is the behavior ofmin-integrity: none.Fixed three locations to accurately describe
unapprovedas includingCONTRIBUTORandFIRST_TIME_CONTRIBUTORwhile filtering outFIRST_TIMERandNONEassociation users.2. Compiler: implicitly enable
cli-proxywhenintegrity-reactions: trueReaction-based integrity decisions require the CLI proxy to identify reaction authors. Previously, users had to set both
features.integrity-reactions: trueandfeatures.cli-proxy: truemanually. The compiler now automatically enables the CLI proxy when integrity-reactions is enabled.Changes:
isCliProxyNeeded()incompiler_difc_proxy.go: also returns true whenintegrity-reactionsfeature flag is setcollectDockerImages()indocker.go: includes cli-proxy image when eithercli-proxyorintegrity-reactionsis enabledcompiler_difc_proxy_test.go3. Docs: describe compiler defaults for integrity-reactions (available in v0.68.2)
Rewrote the "Reactions as Trust Signals" section to describe what the compiler injects automatically rather than showing explicit frontmatter for
endorsement-reactions,disapproval-reactions,endorser-min-integrity, anddisapproval-integrity. Users only need:The compiler handles the rest:
THUMBS_UP,HEARTTHUMBS_DOWN,CONFUSEDendorser-min-integrity: approved(MCPG default)disapproval-integrity: none(MCPG default)Also added a clear warning that reactions only work in proxy mode — gateway mode cannot identify reaction authors.
Test Plan
TestIsCliProxyNeeded_IntegrityReactionsImplicitEnable(6 cases) — all passTestCollectDockerImages_CliProxy(4 cases) — still passTestHasDIFCProxyNeeded(10 cases) — still pass-racemake recompile: 191/191 workflows compiled, no lock file changes (existing workflows don't use integrity-reactions)