Add read-only GitHub Issues access to agent enclaves - #55531
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bcc38c7d-af99-4f49-8c17-b56cbb4515c5
There was a problem hiding this comment.
Pull request overview
Adds isolated, read-only GitHub Issues access for agent enclaves through a dedicated mcpg proxy.
Changes:
- Adds
issues-read-v1schemas, validation, and version gates. - Generates proxy policy, lifecycle, credential isolation, and tests.
- Documents routes, DIFC behavior, and dependency requirements.
Show a summary per file
| File | Description |
|---|---|
.changeset/enclave-github-issues-profile.md |
Records the new profile. |
.github/aw/enclaves.md |
Adds authoring guidance. |
actions/setup/sh/start_enclave_github_proxy.sh |
Starts and configures the proxy. |
actions/setup/sh/stop_enclave_github_proxy.sh |
Cleans up proxy resources. |
docs/src/content/docs/reference/enclaves.md |
Documents profile behavior. |
docs/src/content/docs/reference/glossary.md |
Updates enclave terminology. |
pkg/constants/version_constants.go |
Defines dependency minimums. |
pkg/parser/schema_test.go |
Tests frontmatter validation. |
pkg/parser/schemas/main_workflow_schema.json |
Adds user-facing schema syntax. |
pkg/workflow/awf_env.go |
Excludes proxy handoff variables. |
pkg/workflow/compiler_yaml_ai_execution.go |
Adds proxy cleanup lifecycle. |
pkg/workflow/enclave_github_proxy.go |
Builds policy and lifecycle steps. |
pkg/workflow/enclave_github_proxy_test.go |
Tests proxy integration. |
pkg/workflow/enclaves.go |
Adds configuration and validation. |
pkg/workflow/enclaves_test.go |
Tests AWF configuration output. |
pkg/workflow/mcp_setup_generator.go |
Starts the proxy during MCP setup. |
pkg/workflow/schemas/awf-config.schema.json |
Adds AWF schema support. |
schema-demos/schema-demo-enclaves.md |
Demonstrates the new syntax. |
Review details
Suppressed comments (1)
actions/setup/sh/start_enclave_github_proxy.sh:66
- A cancelled prior run can leave
proxy-tls/ca.crthere. Because the readiness probe usescurl -k, it can accept the new proxy while retaining the stale CA, after which AWF receives a CA that cannot authenticate the proxy. Remove the previous container and log/TLS directory before recreating it.
mkdir -p "$MCP_LOG_DIR"
chmod 700 "$MCP_LOG_DIR"
docker rm -f "$CONTAINER_NAME" >/dev/null 2>&1 || true
- Files reviewed: 18/18 changed files
- Comments generated: 2
- Review effort level: Balanced
|
/matt |
|
/review |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Request changes
This introduces two blocking regressions: the new repo-limit validation rejects valid mixed script + agent.github.cli enclave configs, and the enclave GitHub proxy teardown can be skipped when later host-side steps fail.
Blocking themes
- The
issues-read-v1non-public repository limit is being enforced against the wrong scope, so existing mixed-enclave workflows break as soon as they opt into the new profile. - The proxy cleanup path is not robust against downstream failures, which leaves the capability handoff and proxy state alive longer than the design claims.
🔎 Code quality review by PR Code Quality Reviewer · pi · gpt54 · 9.7 AIC · ⌖ 7.12 AIC · ⊞ 7K
Comment /review to run again
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design and /tdd — overall LGTM with two minor observations (no blocking issues).
📋 Key Themes & Highlights
Key Themes
- Architecture is clean and well-scoped: the PAT/capability key never reaches the AWF sandbox or agent, the proxy runs in bridge mode with no published host port, and the stop step always clears the key from
GITHUB_ENV. The security boundary is deliberately layered and the test suite enforces the contract. - Version gating is conservative: provisional minimums (AWF v0.28.6, MCPG v0.4.11) are separated from global defaults until release artifacts exist. The
validateEnclaveGitHubIssuesVersionsfunction correctly uses the default MCPG version as the fallback when none is specified — meaning omittingsandbox.mcp.versionfails validation. - Two minor observations posted as inline comments:
enclaveGitHubIssuesOperationsas a package-level slice makes profile-to-operations lookup implicit; worth a map when a second profile arrives.- A test case for the nil-MCP-config rejection path would complete the version-gate coverage.
Positive Highlights
- ✅
TestEnclaveGitHubProxyScriptsEnforceDedicatedBridgeContractis an excellent contract-enforcement test — it pins security-critical shell invariants (no-phost port, bridge mode, capability masking ordering) directly in Go. - ✅
TestCompileEnclaveGitHubProxyLifecycleverifies end-to-end compilation ordering and exclusion of all handoff vars from the AWF command line. - ✅
effectivePrimaryGitHubIntegrityFloorcorrectly prefersParsedToolsover raw map access and falls back toapproved— the priority chain is explicit and tested. - ✅ Refactoring
validateEnclavesConfigintovalidateEnclaveEntry/validateEnclaveRepositories/validateEnclaveGitHubIssuesVersionsmeaningfully improves testability and readability.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet46 · 78.7 AIC · ⌖ 10.2 AIC · ⊞ 7.6K
Comment /matt to run again
|
Please do one focused follow-up pass:
Run: https://github.com/github/gh-aw/actions/runs/32787277683
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed in efa862c. I added focused regression coverage for mixed-enclave repository-limit scoping and for unconditional enclave GitHub proxy teardown ( |
|
Please do one focused follow-up pass:
Run: https://github.com/github/gh-aw/actions/runs/32795790586
|
Follow-up done. I replied in the blocking github-actions review threads with concrete fix references to efa862c for both items ( |
Follow-up complete on current head |
|
Please do one focused follow-up pass:
Run: https://github.com/github/gh-aw/actions/runs/32907942669
|
Follow-up complete on head |
Merge current main, set the first supported enclave GitHub Issues releases to AWF v0.28.8 and mcpg v0.4.11, and refresh generated workflow image pins. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bcc38c7d-af99-4f49-8c17-b56cbb4515c5
|
@copilot address all review feedback |
Done on current head |
|
Please do one focused follow-up pass:
Run: https://github.com/github/gh-aw/actions/runs/32957620001
|
Current PR state checked on head |
|
Please do one focused follow-up pass:
Run: https://github.com/github/gh-aw/actions/runs/32962488926
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Completed. I ran |
|
@copilot resolve the merge conflicts on this branch. |
Done — merge conflicts on this branch have been resolved. I merged |
Classify the compiler-owned awf-enclave route as deferred during the eager gateway functionality check so AWF can attach its backend and perform bounded readiness without weakening ordinary required-server checks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bcc38c7d-af99-4f49-8c17-b56cbb4515c5
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bcc38c7d-af99-4f49-8c17-b56cbb4515c5
Summary
enclaves[].agent.github.cli: issues-read-v1syntax to the user and AWF schemasSmoke-run fix
The gh-aw-firewall smoke run
32928731346exposed an ordering bug: the compiler's eager Start MCP Gateway functionality check treated the AWF-ownedawf-enclavebackend like an already-running required server. mcpg correctly returned retryable HTTP 503 until AWF attached the late backend, but the eager check failed before AWF could launch.This PR now classifies only the compiler-owned exact server name
awf-enclaveas deferred for that eager check when enclaves are enabled. It remains required in mcpg configuration, the authenticated route remains fail-closed, arbitrary/user-selected server names cannot be deferred, and every ordinary required server failure remains fatal. AWF still owns boundedinitializeplustools/listreadiness after attachment.Validation
go test ./pkg/workflow -run 'TestGenerateEnclaveGatewayContract|TestCompileEnclaveStartupOrdering' -count=1bash actions/setup/sh/check_mcp_servers_test.sh: 30/30 passedmake shellcheck-setup-shmake fmtmake buildmake recompilegit diff --checkrequired:falsemake agent-report-progresspasses workflow drift, formatting, build, Go lint, action shell lint, schema freshness, and impacted Go tests. Its final aggregate result remains blocked by the repository's existing function-length custom-linter baseline. After merging currentmain, impacted setup-JS validation also requires newly added TypeScript dependencies; local restoration was blocked by the configured package feed/network (vite@8.2.2unavailable), not by this change.The external smoke-workflow compile also reports existing
container_pin_not_foundentries for the AWFv0.28.8enclave-agent and enclave-mcp-server images; this does not affect the deferred-check behavior.Dependencies
This is dependency layer 3. The required gh-aw-firewall/AWF and gh-aw-mcpg changes are merged and released as AWF
v0.28.8and mcpgv0.4.11; this PR consumes those published contracts and artifacts.