refactor: compute sink-visibility at runtime instead of compile time#44245
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #44245 does not have the 'implementation' label and has ≤100 new lines of code in business logic directories (78 additions detected, threshold is 100). |
|
✅ PR Code Quality Reviewer completed the code quality review. |
There was a problem hiding this comment.
Pull request overview
This PR refactors MCP gateway guard-policy generation so sink-visibility is resolved at workflow runtime (via a GitHub Actions expression) instead of being computed at compile time via gh api, avoiding stale visibility and removing a compile-time network dependency.
Changes:
- Removed compile-time repository visibility plumbing (
WorkflowData.RepositoryVisibilityand related compiler wiring). - Updated guard-policy derivation to emit
sink-visibilityas a runtime expression referencingsteps.determine-automatic-lockdown.outputs.visibility. - Updated generated workflow lock files and golden test fixtures to reflect the runtime
sink-visibilityexpression.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/workflow_data.go | Removes RepositoryVisibility from WorkflowData. |
| pkg/workflow/workflow_builder.go | Drops compile-time visibility resolution during workflow data construction. |
| pkg/workflow/mcp_github_config.go | Emits runtime sink-visibility expression in derived write-sink policies; updates derivation call sites. |
| pkg/workflow/compiler_orchestrator_workflow_test.go | Removes test that asserted compile-time visibility resolution into workflow data. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-copilot.golden | Updates expected rendered guard-policies to include runtime sink-visibility. |
| .github/workflows/workflow-skill-extractor.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/workflow-normalizer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/workflow-health-manager.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/weekly-editors-health-check.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/visual-regression-checker.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/video-analyzer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/update-astro.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/unbloat-docs.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/uk-ai-operational-resilience.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/ubuntu-image-analyzer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/tidy.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/test-workflow.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/test-quality-sentinel.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/test-project-url-default.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/test-dispatcher.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/terminal-stylist.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/technical-doc-writer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/super-linter.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/sub-issue-closer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/step-name-alignment.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/stale-pr-cleanup.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/spec-librarian.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/spec-extractor.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/spec-enforcer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-workflow-call.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-workflow-call-with-inputs.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-update-cross-repo-pr.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-test-tools.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-temporary-id.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-service-ports.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-project.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-pi.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-otel-backends.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-multi-pr.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-create-cross-repo-pr.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-copilot-sub-agents.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-copilot-sdk.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-copilot-arm.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-codex.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-ci.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/smoke-call-workflow.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/slide-deck-maintainer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/security-compliance.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/schema-feature-coverage.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/schema-consistency-checker.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/research.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/repository-quality-improver.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/repo-tree-map.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/repo-audit-analyzer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/release.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/refactoring-cadence.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/python-data-charts.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/pr-description-caveman.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/pdf-summary.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/outcome-collector.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/objective-impact-report.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/notion-issue-summary.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/necromancer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/metrics-collector.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/mergefest.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/linter-miner.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/lint-monster.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/layout-spec-maintainer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/jsweep.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/instructions-janitor.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/hourly-ci-cleaner.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/gpclean.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/go-logger.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/glossary-maintainer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/github-remote-mcp-auth-test.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/github-mcp-tools-report.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/functional-pragmatist.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/firewall.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/firewall-escape.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/example-permissions-warning.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/example-failure-category-filter.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/eslint-refiner.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/eslint-monster.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/eslint-miner.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/duplicate-code-detector.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/draft-pr-cleanup.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/docs-noob-tester.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/dictation-prompt.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/dev.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/dev-hawk.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/designer-drift-audit.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/design-decision-gate.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/deployment-incident-monitor.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/dependabot-repair.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/dependabot-go-checker.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/dependabot-burner.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/delight.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/dead-code-remover.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-workflow-updater.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-windows-terminal-integration-builder.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-testify-uber-super-expert.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-syntax-error-quality.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-spdd-spec-planner.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-skill-optimizer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-sentrux-report.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-semgrep-scan.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-security-red-team.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-security-observability.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-secrets-analysis.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-safeoutputs-git-simulator.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-safe-output-integrator.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-repo-chronicle.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-regulatory.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-performance-summary.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-observability-report.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-model-resolution.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-model-inventory.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-mcp-concurrency-analysis.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-max-ai-credits-test.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-malicious-code-scan.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-geo-optimizer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-formal-spec-verifier.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-firewall-report.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-file-diet.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-fact.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-experiment-report.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-credit-limit-test.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-compiler-threat-spec-optimizer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-compiler-quality.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-community-attribution.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-cli-tools-tester.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-cli-performance.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-choice-test.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-cache-strategy-analyzer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-byok-ollama-test.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-assign-issue-to-user.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-architecture-diagram.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/daily-ambient-context-optimizer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/craft.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/copilot-pr-prompt-analysis.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/copilot-pr-nlp-analysis.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/copilot-opt.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/copilot-cli-deep-research.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/copilot-centralization-optimizer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/copilot-centralization-drilldown.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/copilot-agent-analysis.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/constraint-solving-potd.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/codex-github-remote-mcp-test.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/code-simplifier.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/code-scanning-fixer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/cli-consistency-checker.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/ci-doctor.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/ci-coach.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/changeset.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/breaking-change-checker.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/brave.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/bot-detection.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/blog-auditor.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/avenger.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/artifacts-summary.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/architecture-guardian.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/archie.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/approach-validator.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/agentic-token-trend-audit.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/agentic-token-optimizer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/agentic-token-audit.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/agent-performance-analyzer.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
| .github/workflows/ace-editor.lock.yml | Recompiled lock content to use runtime sink-visibility expression (and updated MCP config heredoc delimiter). |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 265/265 changed files
- Comments generated: 0
- Review effort level: Low
🧪 Test Quality Sentinel Report🔶 Test Quality Score: 62/100 — Acceptable
📊 Metrics (7 table-driven test cases modified)
Detailed Test Analysis
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Test Quality Sentinel: 62/100. The PR simplifies tests by removing compile-time visibility verification, but does not add comprehensive runtime verification. The refactor shifts responsibility from compile time to runtime, but test coverage for the runtime behavior is incomplete. Consider adding integration tests for the determineAutomaticLockdown step to verify visibility detection and guard policy enforcement at runtime.
|
@copilot run pr-finisher skill |
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design and /tdd — requesting changes on two correctness gaps.
📋 Key Themes & Highlights
Key Themes
- Stale integration test: The
"No auto detection when guard policy explicitly configured"case ingithub_lockdown_autodetect_test.gostill assertsexpectAutoDetectionStep: false, but the PR's central change (remove the early-return guard) means the step now always runs. This test actively contradicts the new invariant and is only caught on integration builds. - Missing unit tests for new helpers:
serializeEnvStringValueandquoteYAMLEnvValue(theGH_AW_GITHUB_REPOSbugfix) have no unit tests. These are non-trivial serialisation edge cases (embedded single quotes, non-string slices) with no regression guard. - Duplicated runtime expression literal: The sentinel string
guardExprSentinel + "${{ toJSON(steps.determine-automatic-lockdown.outputs.visibility) }}"is copy-pasted in two production call sites. The test layer already definessinkVisibilityExpras a const — the same pattern should be applied in production code.
Positive Highlights
- ✅ Eliminating the compile-time API call for visibility is the right architectural move — fresher data, faster compilation, no auth requirement at build time.
- ✅ The
guardExprSentinelpost-processing pattern is used consistently and correctly extended here. - ✅ Test suite is broadly updated to reflect the new invariant (runtime expr vs. static string) — the coverage is thorough except for the two gaps above.
- ✅ All 258 lock files regenerated cleanly and consistently, which validates the compiler-level change.
- ✅
serializeEnvStringValue/quoteYAMLEnvValuecleanly fix the YAML-sequence misparse bug.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 126.8 AIC · ⌖ 9.12 AIC · ⊞ 6.6K
Comment /matt to run again
| // serializeEnvStringValue converts a workflow config value to a string suitable for a | ||
| // YAML env variable. Strings are returned as-is; slices are JSON-encoded so that | ||
| // e.g. ["github/gh-aw", "github/*"] becomes `["github/gh-aw","github/*"]`. | ||
| func serializeEnvStringValue(v any) string { |
There was a problem hiding this comment.
[/tdd] No unit tests for the new serializeEnvStringValue and quoteYAMLEnvValue helpers. These functions handle edge cases that are easy to regress: slices with embedded single quotes, empty slices, non-string non-slice types, marshal failure paths.
💡 Suggested tests to add in a new file or alongside the function
func TestSerializeEnvStringValue(t *testing.T) {
assert.Equal(t, "myorg/repo", serializeEnvStringValue("myorg/repo"))
assert.Equal(t, `["github/gh-aw","github/*"]`, serializeEnvStringValue([]any{"github/gh-aw", "github/*"}))
}
func TestQuoteYAMLEnvValue(t *testing.T) {
assert.Equal(t, "'hello'", quoteYAMLEnvValue("hello"))
assert.Equal(t, "'it''s'", quoteYAMLEnvValue("it's")) // embedded single-quote escaped
assert.Equal(t, "'[\"a\",\"b\"]'", quoteYAMLEnvValue(`["a","b"]`))
}Without these, the bugfix for GH_AW_GITHUB_REPOS serialisation has no regression guard.
@copilot please address this.
There was a problem hiding this comment.
Fixed in 7a281dd. Added focused coverage in pkg/workflow/compiler_github_mcp_steps_test.go for serializeEnvStringValue, quoteYAMLEnvValue, and the generated lockdown-step env output.
| if sinkVisibility != "" { | ||
| writeSink["sink-visibility"] = sinkVisibility | ||
| "accept": acceptList, | ||
| "sink-visibility": guardExprSentinel + "${{ toJSON(steps.determine-automatic-lockdown.outputs.visibility) }}", |
There was a problem hiding this comment.
[/codebase-design] The sink-visibility expression is now duplicated in two places: deriveSafeOutputsGuardPolicyFromGitHub (line 508) and deriveWriteSinkGuardPolicyFromWorkflow (line 572). The tests solve this by defining sinkVisibilityExpr as a package-level const, but the production code repeats the raw string literal twice. A named constant or a sinkVisibilityRuntimeExpr variable at package scope would make future changes (e.g. a step rename) a one-line edit and make the intent self-documenting.
💡 Suggested approach
In mcp_renderer_guard.go (where guardExprSentinel lives) or mcp_github_config.go:
// sinkVisibilityRuntimeExpr is the guard-policy expression that resolves repository visibility at runtime.
const sinkVisibilityRuntimeExpr = guardExprSentinel + "${{ toJSON(steps.determine-automatic-lockdown.outputs.visibility) }}"Both call sites then use sinkVisibilityRuntimeExpr — the tests' sinkVisibilityExpr constant can alias or reuse it.
@copilot please address this.
There was a problem hiding this comment.
Fixed in 7a281dd. I extracted sinkVisibilityRuntimeExpr into production code in pkg/workflow/mcp_renderer_guard.go and both guard-policy call sites now reuse it.
There was a problem hiding this comment.
REQUEST_CHANGES — one correctness bug that must be fixed before merge; two stale artifacts that erode safety guarantees.
### Critical: TOML format silently drops `sink-visibility`
renderGuardPoliciesToml only handles []string-typed fields. The new sink-visibility field is a string (even after sentinel removal), so it is silently discarded for every engine that uses TOML config (Codex). Those workflows ship with no public/private/internal enforcement in their write-sink guard policy. The JSON path (Copilot engine) works correctly; TOML does not.
Separately: even if the TOML renderer were fixed to emit string fields, it would need a runtime-substitution mechanism for ${{ }} expressions — the TOML heredoc is written before determine-automatic-lockdown runs.
### High: Stale integration test assertion
github_lockdown_autodetect_test.go line 79 asserts expectAutoDetectionStep: false for the "guard policy explicitly configured" case. This PR removes that early-return, so the assertion is wrong. Because the test is integration-build-tag only, it won't catch regressions in CI.
### Medium: Stale comment in `mcp_environment.go`
Line 97 says "determine-automatic-lockdown step is not generated" when guardPoliciesExplicit is true — this is no longer true after this PR.
### Medium: Inconsistent YAML quoting for env vars
GH_AW_GITHUB_MIN_INTEGRITY is written without quoteYAMLEnvValue; GH_AW_GITHUB_REPOS is quoted. The inconsistency is a latent risk.
🔎 Code quality review by PR Code Quality Reviewer · 265.1 AIC · ⌖ 6.01 AIC · ⊞ 5.4K
Comment /review to run again
| @@ -75,14 +68,39 @@ func (c *Compiler) generateGitHubMCPLockdownDetectionStep(yaml *strings.Builder, | |||
| fmt.Fprintf(yaml, " GH_AW_GITHUB_MIN_INTEGRITY: %s\n", configuredMinIntegrity) | |||
There was a problem hiding this comment.
GH_AW_GITHUB_MIN_INTEGRITY written unquoted while GH_AW_GITHUB_REPOS is single-quoted: configuredRepos is passed through quoteYAMLEnvValue (line 71) but configuredMinIntegrity is not (line 68). While the current set of allowed min-integrity values is safe, the inconsistency is a latent risk if validation is ever relaxed or the pattern is copied elsewhere.
💡 Suggested fix
if configuredMinIntegrity != "" {
fmt.Fprintf(yaml, " GH_AW_GITHUB_MIN_INTEGRITY: %s\n", quoteYAMLEnvValue(configuredMinIntegrity))
}There was a problem hiding this comment.
Fixed in 7a281dd. GH_AW_GITHUB_MIN_INTEGRITY is now emitted through quoteYAMLEnvValue in generateGitHubMCPLockdownDetectionStep.
| // serializeEnvStringValue converts a workflow config value to a string suitable for a | ||
| // YAML env variable. Strings are returned as-is; slices are JSON-encoded so that | ||
| // e.g. ["github/gh-aw", "github/*"] becomes `["github/gh-aw","github/*"]`. | ||
| func serializeEnvStringValue(v any) string { |
There was a problem hiding this comment.
serializeEnvStringValue and quoteYAMLEnvValue have no unit tests: A nil input to serializeEnvStringValue will produce the string literal "null" (written as GH_AW_GITHUB_REPOS: 'null'), which the lockdown script would interpret as a literal repos value. Map or struct inputs produce a JSON object. These edge cases are unverified.
💡 Suggested fix
Add a unit test covering at least: nil input, empty slice, []string{"a", "b"}, plain string, and a string containing a single quote. For example:
func TestSerializeEnvStringValue(t *testing.T) {
assert.Equal(t, "myvalue", serializeEnvStringValue("myvalue"))
assert.Equal(t, `["a","b"]`, serializeEnvStringValue([]string{"a", "b"}))
assert.Equal(t, "null", serializeEnvStringValue(nil)) // document the behavior
assert.Equal(t, "[]", serializeEnvStringValue([]string{}))
}
func TestQuoteYAMLEnvValue(t *testing.T) {
assert.Equal(t, `'foo'`, quoteYAMLEnvValue("foo"))
assert.Equal(t, `'it''s'`, quoteYAMLEnvValue("it's"))
assert.Equal(t, `'["a","b"]'`, quoteYAMLEnvValue(`["a","b"]`))
}There was a problem hiding this comment.
Fixed in 7a281dd. The new unit tests cover plain strings, slices, empty slices, nil, embedded single quotes, and the rendered YAML env values.
There was a problem hiding this comment.
Review: refactor: compute sink-visibility at runtime instead of compile time
The refactor is sound and correctly eliminates the compile-time gh api call for repository visibility. The lockdown detection step now always runs when the GitHub tool is present, and both deriveSafeOutputsGuardPolicyFromGitHub and deriveWriteSinkGuardPolicyFromWorkflow consistently emit the guardExprSentinel-prefixed runtime expression for sink-visibility. The fallback in determine_automatic_lockdown.cjs (emits "unknown" on error) ensures the field is always populated.
Two issues in changed code worth addressing:
-
GH_AW_GITHUB_MIN_INTEGRITYis emitted inconsistently (blocking): the PR addsserializeEnvStringValue+quoteYAMLEnvValueforGH_AW_GITHUB_REPOSbut the parallelGH_AW_GITHUB_MIN_INTEGRITYstill uses rawfmt.Sprintf("%v", v)and no quoting. Ifmin-integrityever carries a non-scalar value this path will produce broken YAML — for symmetry it should use the same helpers. (See inline comment.) -
Minor nit on
valindefaultbranch ofserializeEnvStringValue(non-blocking):valshadowsvin thedefaultcase but equalsv(any).json.Marshal(val)vsjson.Marshal(v)makes no difference here; usingvdirectly would be clearer. (See inline comment.)
What the PR gets right
- Removing
RepositoryVisibilityfromWorkflowDataeliminates a compile-time network call and prevents stale cached values. - Always generating the lockdown detection step regardless of guard-policy configuration ensures
visibilityoutput is always available for the runtime expression. serializeEnvStringValue+quoteYAMLEnvValuecorrectly fix theGH_AW_GITHUB_REPOSYAML serialization bug for slice values.- Tests are updated to match the new runtime expression constant, and the removed
TestBuildInitialWorkflowData_SetsRepositoryVisibilityForGitHubTooltest is correctly deleted since that code path no longer exists. - All 258 lock files are consistently recompiled.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 167.8 AIC · ⌖ 6.15 AIC · ⊞ 4.8K
| @@ -75,14 +68,39 @@ func (c *Compiler) generateGitHubMCPLockdownDetectionStep(yaml *strings.Builder, | |||
| fmt.Fprintf(yaml, " GH_AW_GITHUB_MIN_INTEGRITY: %s\n", configuredMinIntegrity) | |||
There was a problem hiding this comment.
Inconsistency: GH_AW_GITHUB_MIN_INTEGRITY is not quoted, but GH_AW_GITHUB_REPOS is
This PR introduces quoteYAMLEnvValue to wrap GH_AW_GITHUB_REPOS in single-quotes so YAML cannot misinterpret JSON arrays as sequences. However, GH_AW_GITHUB_MIN_INTEGRITY is emitted without quoting, and configuredMinIntegrity is still set via fmt.Sprintf("%v", v) rather than serializeEnvStringValue.
Although min-integrity is always a string today, the asymmetry is a latent footgun. For consistency and safety the same pattern should be applied:
configuredMinIntegrity = serializeEnvStringValue(v)
// ...
fmt.Fprintf(yaml, " GH_AW_GITHUB_MIN_INTEGRITY: %s\n", quoteYAMLEnvValue(configuredMinIntegrity))@copilot please address this.
There was a problem hiding this comment.
Fixed in 7a281dd. min-integrity now follows the same path as repos: serializeEnvStringValue at read time and quoteYAMLEnvValue when rendered to YAML.
| func serializeEnvStringValue(v any) string { | ||
| switch val := v.(type) { | ||
| case string: | ||
| return val |
There was a problem hiding this comment.
val is declared but only used in one branch of the type switch
In the case string branch, val is used correctly (return val). However, in the default branch, the function uses v (the original any) for json.Marshal(val) — val in the default branch is any, same as v. This is harmless but the shadowing may cause confusion. Consider:
case string:
return val
default:
encoded, err := json.Marshal(v) // use v directly; val == v hereNot blocking, just a minor clarity nit. @copilot please address this.
There was a problem hiding this comment.
Fixed in 7a281dd. I switched the fallback marshal branch to use v directly so the type-switch shadowing is clearer.
|
@copilot run pr-finisher skill |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🎉 This pull request is included in a new release. Release: |
Previously,
sink-visibilityin the mcpg guard policy config was a static string (e.g."public") baked in at compile time viagh api. This could go stale and required a network call during compilation.Changes
RepositoryVisibilityfromWorkflowDataand thecomputeRepositoryVisibility()call inworkflow_builder.gosink-visibility: BothderiveSafeOutputsGuardPolicyFromGitHubandderiveWriteSinkGuardPolicyFromWorkflownow emit:guardExprSentinelpost-processing patterngenerateGitHubMCPLockdownDetectionStepno longer skips when explicit guard policies are configured — the step must run to produce thevisibilityoutput regardlessGH_AW_GITHUB_REPOS:allowed-repos: [...]was being serialized viafmt.Sprintf("%v", slice)→[github/gh-aw github/*], which YAML parses as a sequence. AddedserializeEnvStringValue(JSON-encodes slices) andquoteYAMLEnvValue(single-quotes YAML scalars) to produce valid output