[pelikhan]

Summary

MCP clients should be able to validate workflows without permission to modify them. Previously, compile required --allow-write-tools, and --no-emit still allowed filesystem side effects.

• Make compile available by default with a dedicated internal read-only mode, automatic --no-emit --no-check-update, and a read-only tool annotation. Preserve configured actor checks for write-enabled compilation and the startup capability gate for writing tools.
• Keep ordinary CLI no-emit validation separate from the stronger MCP boundary, preserving import-cache population for validate, maintenance jobs, and label discovery, including changed upstream branch SHAs.
• Suppress lock/diagnostic output, action-pin and .gitattributes post-processing, cache writes, and temporary remote-include downloads in read-only mode. Validate newly generated YAML rather than malformed stale lock files.
• Reuse the repository-root shared cache for frontmatter imports, all body-level include expansion paths, nested includes, and workflow hashing. Reject uncached imports/includes before downloading; retain the working-directory fallback outside Git. Ref resolution may still require network reads.
• Make default container validation inspect-only in read-only mode, without Docker pulls. Preserve existing missing-image warning/error handling and trusted CLI behavior. Fixes, dry-run compilation, and external scanners still require --allow-write-tools, with shared option-gate helpers.
• Update MCP documentation, the draft ADR, and compiler threat specification v1.0.54 with six host-side compilation requirements and implementation/test mappings. Extend the dedicated bounded TLA+ model with nineteen synthetic negative controls and seven reachability witnesses.
• Refresh the replaced official TLA+ v1.8.0 asset pin from GitHub release metadata and regenerate its verification workflow without changing intent or permissions.

ADR: ADR-69510: Read-Only MCP compile by Default (Draft)

Validation

• Full parser tests and focused CLI, compiler, Docker, capability, argument, stale-lock, watch-compilation, import-cache, changed-branch, and include-expansion regressions passed.
• Real-binary MCP integration tests passed for configuration-only inspection, read-only filesystem preservation, repository-subdirectory launches, and write-enabled lock emission. The inspection test now permits read-only compile while retaining command-execution and file-preservation assertions.
• Isolated temporary-directory tests cover body-level and nested remote includes outside the repository. Fake-Docker tests cover default validation, cached/missing images, inspection failures, and preserved trusted pulling behavior.
• WASM workflow-package build, formal compiler-threat tests, workflow-security conformance, and all 17 Python model-runner tests passed.
• Pinned TLC exhausted the updated MCP baseline with 88,416 distinct states and verified all nineteen expected negative controls and seven reachability witnesses. Existing runtime and patch/bundle model suites also passed earlier in the PR. These are bounded policy checks, not proof of arbitrary filesystem behavior or compiler refinement.
• Build, standard Go lint, and schema freshness passed. The repository gate remains blocked by pre-existing custom-linter diagnostics in unchanged logic. The broad formatter also hits the existing JavaScript formatting failure; Go formatting passed and unrelated JavaScript formatting changes were removed.

All five requested inline review comments were accepted, addressed, replied to, and resolved in the final review-fix batch.

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

[github-actions]

✅ Ponytail Reviewer completed successfully!

Ponytail Reviewer

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Ponytail Reviewer · gpt56 · 8.56 AIC · ⌖ 5.3 AIC · ⊞ 14.6K · ◷

Generated by Ponytail Reviewer for #69510

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

Commit pushed: 6b4e0b4

🏗️ ADR gate enforced by Design Decision Gate 🏗️

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Test Quality Sentinel · sonnet50 · 17.1 AIC · ⌖ 14.1 AIC · ◇ 6.57 AIC · ⊞ 12.6K · ◷

🧪 Test quality analysis by Test Quality Sentinel

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

[github-actions]

🏗️ Design Decision Gate — ADR Required

This PR makes significant changes to core business logic (303 new lines in pkg/) but does not have a linked Architecture Decision Record (ADR).

📄 Draft ADR committed: docs/adr/69510-read-only-mcp-compile-by-default.md. Review and complete it before merging.

The draft records this decision: MCP compile is available by default in read-only mode (--no-emit --no-check-update, ReadOnlyHint). --no-emit becomes side-effect free (no post-processing, no .invalid.yml, read-only import cache). Fixes, dry-run, and scanners require --allow-write-tools.

🔒 This PR cannot merge until an ADR is linked in the PR body.

📋 What to do next

1. Review the draft ADR committed to your branch. It was generated from the PR diff.
2. Complete the missing sections. Add context the AI couldn't infer, refine the decision rationale, and check the listed alternatives (keep compile write-gated, add a separate validate tool, allow downloads of uncached imports).
3. Commit the finalized ADR to docs/adr/ on your branch.
4. Reference the ADR in this PR body by adding a line such as:

   ADR: ADR-69510: Read-Only MCP compile by Default

Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision.

❓ Why ADRs Matter

"AI made me procrastinate on key design decisions. Because refactoring was cheap, I could always say 'I'll deal with this later.' Deferring decisions corroded my ability to think clearly."

ADRs 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 Reference

An ADR must contain these four sections to be considered complete:

• Context: What is the problem? What forces are at play?
• Decision: What did you decide? Why?
• Alternatives Considered: What else could have been done?
• Consequences: What are the trade-offs (positive and negative)?

All ADRs are stored in docs/adr/ as Markdown files numbered by PR number.

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · pi · opus55 · 31.3 AIC · ⌖ 40.4 AIC · ⊞ 1.9K · ◷
_{Comment /review to run again}

[github-actions]

Comment Memory

Peek at saved memory (pr-code-quality-reviewer)

reviewed_at: 2026-10-16T23:29:37.888+00:00
review_event: REQUEST_CHANGES
top_themes:
  - import-cache root depends on process cwd
  - read-only compile cached-import regressions from subdirectory launches
files_reviewed:
  - pkg/cli/mcp_tools_readonly.go
  - pkg/cli/compile_pipeline.go
  - pkg/parser/import_cache.go
  - pkg/parser/remote_workflow_spec.go
  - pkg/workflow/compiler.go
  - pkg/workflow/compiler_mutators.go
  - pkg/workflow/compiler_yaml.go
comment_count: 1

Note

This comment is managed by comment memory.

Expand the saved memory block to view or edit the persistent context for this thread.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 97.6 AIC · ⌖ 5.76 AIC · ⊞ 12.7K · ◷
_{Comment /review to run again}

[github-actions]

Request changes

Read-only MCP compile still has a cache-root correctness hole: cached remote imports can fail as uncached when the server is launched from a checkout subdirectory.

Blocking theme

• The new shared import-cache path is tied to the process working directory, not the repository root, so the advertised "cached remote imports work in --no-emit" behavior is not stable outside repo-root launches.

Static review only; I did not run code or tests.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 97.6 AIC · ⌖ 5.76 AIC · ⊞ 12.7K
_{Comment /review to run again}

[github-actions]

Skills-Based Review 🧠

Applied /diagnosing-bugs and /codebase-design to the MCP compile access-control changes. Overall this is a well-scoped, well-tested change — 2 non-blocking improvement suggestions posted inline.

📋 Key Themes & Highlights

Key Themes

• Minor perf/UX gap: read-only import resolution still does a network SHA lookup before rejecting uncached imports (pkg/parser/remote_workflow_spec.go).
• Duplication risk: the read-only write-flag gate list is repeated in two places in mcp_tools_readonly.go; worth consolidating into one helper to avoid drift as new scanners are added.

Positive Highlights

• ✅ Excellent regression coverage: TestMCPServer_ReadOnlyCompilePreservesFiles exercises existing/new files x all/single-workflow x valid/invalid/oversized/cached/uncached-import combinations, snapshotting file mode/mtime/hash to prove true read-only behavior.
• ✅ ImportCache.SetReadOnly and the explicit "not cached" error give MCP clients an actionable message instead of a confusing permission or network failure.
• ✅ Compiler.writeInvalidWorkflowYAML and runPostProcessing/runPostProcessingForDirectory correctly gate all diagnostic/side-effect writes behind NoEmit, consistent with the PR's stated goal.
• ✅ Middleware change cleanly separates compile (now allowed by default, actor-checked only when write-enabled) from add/update/fix (still fully gated), matching the documented access-control model.

No blocking issues found; approving pending author's discretion on the two inline suggestions.

@copilot please address the review comments above.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 134.3 AIC · ⌖ 15.1 AIC · ⊞ 10.1K
_{Comment /matt to run again}

Comments that could not be inline-anchored

pkg/parser/remote_workflow_spec.go:29

[/diagnosing-bugs] In read-only mode, resolveWorkflowSpecSHAForCache still performs a full network SHA resolution (REST API call / git ls-remote fallback) for every uncached import before the cache.readOnly check rejects it. This defeats the goal of making read-only compile fast/offline-safe and can surface confusing network/auth errors instead of the clean not cached message when the MCP client has no network credentials.

<details>
<summary>💡 Suggested fix</summary>

Check `cache…

pkg/cli/mcp_tools_readonly.go:146

[/codebase-design] The read-only rejection list (Fix || DryRun || Zizmor || Poutine || ...) duplicates the scanner-flag list used again at line 179 (!args.DryRun && (Zizmor || Poutine || ...)). A future scanner flag added to compileArgs only needs to appear in one of these lists to silently bypass the read-only gate.

<details>
<summary>💡 Suggested fix</summary>

Extract a single helper method on compileArgs, e.g. requiresWriteTools() bool returning the OR of all write-requiring flags, and…

[Copilot]

🟡 Changes recommended

The read-only contract and existing validation paths have unresolved correctness issues.

5 open issues

• Update MCP inspect test for read-only compile support · New
• No-emit callers fail on uncached imports before cache refresh · New
• Stale malformed lock files invalidate valid no-emit compilations · New
• Read-only compilation can pull missing Docker images · New
• Remote includes bypass read-only cache and create temporary files · New

What changed in this PR

Enables MCP workflow validation without write-tool permission and tightens the compiler’s no-emit behavior.

Changes:

• Adds capability-aware compile defaults and option restrictions.
• Suppresses output writes and introduces read-only import caches.
• Updates regression tests and MCP documentation.

File	Description
pkg/​workflow/​compiler.go	Suppresses diagnostic YAML writes in no-emit mode.
pkg/​workflow/​compiler_yaml.go	Reuses the shared cache for hashing.
pkg/​workflow/​compiler_template_injection_both_paths_test.go	Tests diagnostic-file preservation.
pkg/​workflow/​compiler_mutators.go	Applies read-only import-cache mode.
pkg/​parser/​remote_workflow_spec.go	Rejects uncached read-only imports.
pkg/​parser/​import_cache.go	Adds read-only cache controls.
pkg/​parser/​import_cache_test.go	Tests cache write restrictions.
pkg/​cli/​mcp_write_tools.go	Allows default compile calls.
pkg/​cli/​mcp_write_tools_test.go	Tests capabilities and argument restrictions.
pkg/​cli/​mcp_tools_readonly.go	Configures read-only compilation defaults.
pkg/​cli/​mcp_tools_output_streams_test.go	Updates write-enabled test registration.
pkg/​cli/​mcp_tool_arguments_test.go	Updates argument-test setup.
pkg/​cli/​mcp_server.go	Propagates write capability to compilation.
pkg/​cli/​mcp_server_unit_test.go	Updates compile-tool registration.
pkg/​cli/​mcp_server_compile_test.go	Tests file preservation and enabled emission.
pkg/​cli/​mcp_server_command.go	Documents compile access controls.
pkg/​cli/​mcp_positional_args_test.go	Updates positional-argument test setup.
pkg/​cli/​mcp_compile_diagnostics_test.go	Preserves write-enabled diagnostic tests.
pkg/​cli/​mcp_actionlint_local_test.go	Updates scanner-test registration.
pkg/​cli/​compile_pipeline.go	Skips no-emit post-processing.
docs/​src/​content/​docs/​setup/​cli.md	Explains default read-only compilation.
docs/​src/​content/​docs/​reference/​tools.md	Documents write-capability requirements.
docs/​src/​content/​docs/​reference/​gh-aw-as-mcp-server.md	Describes no-emit and cached-import behavior.

_{Review effort: Max}

done

[github-actions]

🎉 This pull request is included in a new release.

Release: v0.92.5