Skip to content

feat(compile): add required_awf_mounts to CompilerExtension for Lean runtime#354

Merged
jamesadevine merged 8 commits intomainfrom
feat/awf-mounts-lean-runtime
Apr 29, 2026
Merged

feat(compile): add required_awf_mounts to CompilerExtension for Lean runtime#354
jamesadevine merged 8 commits intomainfrom
feat/awf-mounts-lean-runtime

Conversation

@jamesadevine
Copy link
Copy Markdown
Collaborator

@jamesadevine jamesadevine commented Apr 29, 2026

Summary

AWF replaces $HOME with an empty directory overlay for security, only mounting specific known subdirectories (.cargo, .rustup, .npm, etc.). When the Lean runtime is enabled, the toolchain installed at $HOME/.elan/ was not mounted into the AWF container, causing lean, lake, and elan binaries to be missing inside the chroot — regardless of PATH configuration.

This PR extends CompilerExtension with a new required_awf_mounts() method, following the same pattern as required_hosts(){{ allowed_domains }}. The Lean extension uses this to declare --mount "$HOME/.elan:$HOME/.elan:ro" on the AWF invocation, making the elan toolchain accessible inside the chroot.

Changes

New trait method: required_awf_mounts()

  • src/compile/extensions/mod.rs — Added required_awf_mounts() -> Vec<String> to CompilerExtension with empty default, plus macro delegation in extension_enum!
  • Returns mount specs in AWF --mount format (host_path:container_path[:mode])

Lean runtime implementation

  • src/runtimes/lean/extension.rs — Returns ["$HOME/.elan:$HOME/.elan:ro"] to mount the elan toolchain read-only
  • src/runtimes/lean/mod.rs — Updated comments: mount is now primary mechanism, symlinks are defense-in-depth fallback

Generator + template wiring

  • src/compile/common.rs — New generate_awf_mounts() function that collects mounts from all extensions and formats as --mount CLI flags
  • src/data/base.yml{{ awf_mounts }} marker in agent AWF invocation
  • src/data/1es-base.yml{{ awf_mounts }} marker in agent AWF invocation
  • src/compile/standalone.rs — Wires awf_mounts into extra_replacements
  • src/compile/onees.rs — Wires awf_mounts into extra_replacements

Tests

  • Extension tests: test_lean_required_awf_mounts, test_default_required_awf_mounts_empty
  • Generator tests: test_generate_awf_mounts_no_extensions, test_generate_awf_mounts_with_lean

Documentation

  • docs/extending.md — Added required_awf_mounts() to trait reference
  • docs/template-markers.md — Documented {{ awf_mounts }} marker
  • docs/runtimes.md — Updated Lean section to describe AWF mount behavior

How it works

When Lean is enabled via runtimes: lean: true, the compiled pipeline's AWF invocation now includes:

sudo -E "$(Pipeline.Workspace)/awf/awf" \
  --allow-domains "..." \
  --skip-pull \
  --env-all \
  --enable-host-access \
  --mount "$HOME/.elan:$HOME/.elan:ro" --container-workdir "..." \
  ...

When no extensions declare mounts, the marker expands to empty (no behavioral change to existing pipelines).

…runtime

AWF replaces $HOME with an empty directory overlay for security,
only mounting specific known subdirectories (.cargo, .rustup, etc.).
The Lean toolchain installed at $HOME/.elan/ was not mounted,
causing lean/lake/elan binaries to be missing inside the chroot.

Add 
equired_awf_mounts() to the CompilerExtension trait so
extensions can declare Docker volume mounts needed inside the AWF
chroot. The Lean extension returns $HOME/.elan:C:\Users\devinejames/.elan:ro
to mount the elan toolchain read-only.

The compiler collects mounts from all extensions via
generate_awf_mounts() and injects them as --mount flags on
the AWF invocation through a new {{ awf_mounts }} template
marker in both standalone and 1ES base templates.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread docs/runtimes.md Outdated
- Adds Lean-specific domains to the network allowlist: `elan.lean-lang.org`, `leanprover.github.io`, `lean-lang.org`
- Symlinks lean tools into `/tmp/awf-tools/` for AWF chroot compatibility
- Mounts `$HOME/.elan` into the AWF container via `--mount` flag so the elan toolchain is accessible inside the chroot (AWF replaces `$HOME` with an empty overlay for security)
- Symlinks lean tools into `/tmp/awf-tools/` as a defense-in-depth fallback
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot let's drop the symlinks from the compiler extension. Not necessary.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — removed the symlink loop and the echo "Lean tools symlinked..." line from the install script in e241b35. The module doc comment and generate_lean_install doc were also updated to drop the symlink references.

Comment thread src/data/base.yml Outdated
--skip-pull \
--env-all \
--container-workdir "{{ working_directory }}" \
{{ awf_mounts }}--container-workdir "{{ working_directory }}" \
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot shouldn't awf mounts be on a different line?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4915eae{{ awf_mounts }} now expands each mount as its own --mount "spec" \ continuation line (followed by a newline and appropriate indentation), so --container-workdir always starts on its own line.

Comment thread src/data/base.yml Outdated
--skip-pull \
--env-all \
--container-workdir "{{ working_directory }}" \
{{ awf_mounts }}--container-workdir "{{ working_directory }}" \
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot awf mounts is not required for threat detection!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in 4915eae{{ awf_mounts }} is now only in the agent job AWF invocation; the detection job in both base.yml and 1es-base.yml no longer has the marker.

Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/3352a7f0-905a-491f-a9df-3aefb8ffec4b

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Comment thread src/runtimes/lean/extension.rs Outdated
vec![generate_lean_install(&self.config)]
}

fn required_awf_mounts(&self) -> Vec<String> {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot formaliza this as a struct that can be serialized and deserialized from a docker format string (like this)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 4915eae — introduced AwfMount struct with Display/FromStr (Docker bind-mount format host:container[:mode]) and custom Serialize/Deserialize that round-trips through the format string. required_awf_mounts() now returns Vec<AwfMount> and LeanExtension uses AwfMount::new("$HOME/.elan", "$HOME/.elan", Some("ro")).

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 29, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dev.azure.com
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-74f9fa9b6a384fc1 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-74f9fa9b6a384fc1 --hash-style=gnu --as-needed -shared -fuse-ld=lld -z caf9ice5zh2amovswktjgo.1yne3uz.rcgu.o uy5wa9brrg5qmk3kmb4sxz.1yne3uz.rcgu.o 5pxp63k02bekmehtfb933l.1yne3uz.rcgu.o v61i2jyg4dka72fcagcfj5.1yne3uz.rcgu.o 5olzwfuu9j5ltdkuaxq2xy.1yne3uz.rcgu.o nnloo0ffhzni85q3u5asfc.1yne3uz.rcgu.o 4xkip4k6f1jqgyl9ozhm15.1yne3uz.rcgu.o bpa44udb8ji0t39rrhu1vs.1yne3uz.rcgu.o ke8kkd5bjqlcahao8q6y1u.1yne3uz.rcgu.o h08h11zcv4ox87lt89k6nj.1yne3uz.rcgu.o 4fvw3780bc2we7ve8hxwjv.1yne3uz.rcgu.o 93yitj74s5g2twe8340smo.1yne3uz.rcgu.o 8c7wx7vgxwicru565qse1f.1yne3uz.rcgu.o hjscpt6ckgnf9mc4po43w6.1yne3uz.rcgu.o (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@github-actions
Copy link
Copy Markdown
Contributor

🔍 Rust PR Review

Summary: Core implementation looks solid, but there's an accidentally committed internal file that must be removed, a doc/code discrepancy in the mount formatting, and the Lean integration test doesn't assert the new behavior.

Findings

🐛 Bugs / Logic Issues

  • s360-breeze-mcp.md (entire file) — This file was accidentally committed and is completely unrelated to this PR. It's internal Microsoft S360 MCP Server documentation containing API endpoint URLs, Azure AD application IDs (6833b4aa..., 08654c87...), tenant IDs, and email addresses. It should be removed from this branch before merge.

  • docs/template-markers.md vs src/compile/common.rs — The docs say each mount is formatted as --mount "spec" \ with trailing backslash for shell line continuation, implying each mount gets its own line. The actual implementation joins them with a single space and produces --mount "a" --mount "b" --container-workdir "..." \ — all on one line. Functionally fine for bash, but the docs are misleading and multiple mounts would create an unusually long single line. Either fix the docs to reflect the inline-concatenation approach, or fix the generator to emit \\\n between flags.

  • tests/compiler_tests.rs:test_lean_runtime_compiled_output — The existing Lean E2E integration test doesn't assert that --mount "$HOME/.elan appears in the compiled output. A unit test covers generate_awf_mounts in isolation, but there's no end-to-end verification that the marker is actually wired and replaced in the final YAML. Worth adding an assertion like assert!(compiled.contains("--mount"), ...) alongside the existing elan.lean-lang.org check.

🔒 Security Concerns

  • src/compile/common.rs:generate_awf_mounts — Mount specs from extensions are embedded directly into the generated shell script without any validation or sanitization. The current Lean extension uses a hardcoded string so there's no immediate risk, but the trait API (required_awf_mounts() -> Vec<String>) accepts arbitrary strings. A future extension that derives a mount path from front-matter input could introduce shell injection (e.g., a spec containing " --some-other-flag). Consider adding a validator in generate_awf_mounts that checks specs match the expected path:path[:mode] format (similar to how validate.rs guards other user-controlled values).

✅ What Looks Good

  • The trait extension pattern (required_awf_mounts() with empty default) is clean and consistent with required_hosts() / required_pipeline_vars().
  • The macro delegation in extension_enum! correctly forwards to the inner type — no missed match arms.
  • The Lean extension correctly uses $HOME shell variables (not compile-time paths), allowing AWF to expand them at runtime.
  • Both base templates (standalone + 1ES) and both pipeline stages (agent + detection) are updated consistently.
  • New unit tests cover the happy path and the no-extensions case.

Generated by Rust PR Reviewer for issue #354 · ● 663.8K ·

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Rust PR Review

Summary: Looks good overall — clean extension pattern, consistent wiring across both targets. One doc/code contradiction to fix before merging.

Findings

🐛 Bugs / Logic Issues

  • docs/runtimes.md:32 vs src/runtimes/lean/mod.rs:90-107 — The docs state "Symlinks lean tools into /tmp/awf-tools/ as a defense-in-depth fallback" but the symlink loop was removed from generate_lean_install(). This is a direct contradiction introduced by this PR. Either the fallback is intentionally dropped (and the docs bullet should be removed) or the symlink loop should be kept. The PR description's own summary says "symlinks are defense-in-depth fallback" while the code removes them — which is the intended behaviour?

⚠️ Suggestions

  • src/compile/common.rs:generate_awf_mounts() — When multiple mounts are declared (possible in future), they're joined with a single space and placed inline before --container-workdir. This is valid bash, but consider formatting each with a trailing \\\n + indent to match the surrounding shell line-continuation style. Not blocking, just a style heads-up for when the second runtime ships.

  • src/runtimes/lean/extension.rs — The mount spec "$HOME/.elan:$HOME/.elan:ro" is hardcoded in extension code (not user-supplied), so no injection risk today. If required_awf_mounts() is ever wired to accept user-provided paths (e.g., a mounts: front matter field), those values must go through src/sanitize.rs / src/validate.rs to prevent path traversal or injection into the AWF invocation.

✅ What Looks Good

  • Extension trait default returns vec![] — zero-cost opt-out for all existing extensions.
  • extension_enum! macro delegation is correctly updated — no match-arm gaps.
  • Template marker placed correctly in both base.yml and 1es-base.yml, in both agent and detection stages, and wired identically in standalone.rs and onees.rs.
  • Tests cover both the Lean case (mount present, read-only, .elan path) and the default case (empty).
  • generate_awf_mounts correctly produces an empty string (not " " or "\n") when no extensions declare mounts — existing pipeline output is unaffected.

Generated by Rust PR Reviewer for issue #354 · ● 206.2K ·

…emove from detection job

Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/5651c5dd-be03-4f3b-86d5-4f925a895a21

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 29, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dev.azure.com
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-74f9fa9b6a384fc1 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-74f9fa9b6a384fc1 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-27869e84769d9fe3.0inlkr/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-504a1b3a667b9e83.0xcuhgynym2z2e0w34kz681ff.1ea5eea.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-27869e84769d9fe3.0k5mu7/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-504a1b3a667b9e83.0ym7k8bxy7m34anrpw48qzcgw.1ea5eea.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-27869e84769d9fe3.0lifbe/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-504a1b3a667b9e83.0znry6dy9dgams4tihylzv7yp.1ea5eea.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-27869e84769d9fe3.0mdbpq/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-504a1b3a667b9e83.10zr1nj6761mjfqotrch4xxja.1ea5eea.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-27869e84769d9fe3.0ofh9x/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-504a1b3a667b9e83.15jjgd4ecnrhc7mhf792dl544.1ea5eea.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-27869e84769d9fe3.0r0xpe/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-504a1b3a667b9e83.17iipxrk15umntt1deaykbzk1.1ea5eea.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-27869e84769d9fe3.0te7z7/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-504a1b3a667b9e83.1chvjo7uu3ng22szjkxdip59q.1ea5eea.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-27869e84769d9fe3.0voszj/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-504a1b3a667b9e83.1qe1xfoys7nujer0ff33atx0f.1ea5eea.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-27869e84769d9fe3.100lbi3qzu�� /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-27869e84769d9fe3.121fs9a42ecdbuek1srzl5dex.0m14flw.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-27869e84769d9fe3.16wz4y5ciiaqn2f2uia9mc7me.0m14flw.rcgu.o g/de�� lib/rustlib/x86_--error-format=json lib/rustlib/x86_--json=diagnostic-rendered-ansi,artifacts,future-incompat bin/rustc res-0d1f7d618793cc res-0d1f7d618793-m64 res-0d1f7d618793/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/rustcUMSov9/symbols.o bin/rustc (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Comment thread src/compile/extensions/mod.rs Outdated
/// Corresponding path inside the container.
pub container_path: String,
/// Optional mount mode (e.g. `"ro"` for read-only, `"rw"` for read-write).
pub mode: Option<String>,
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot enum for mount mode, please

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in d7f3055 — added AwfMountMode enum with ReadOnly (ro) and ReadWrite (rw) variants, each with Display/FromStr/Serialize/Deserialize. AwfMount.mode is now Option<AwfMountMode> and LeanExtension uses AwfMountMode::ReadOnly. Parsing an unknown mode string now returns a descriptive error.

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 29, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • spsprodeus21.vssps.visualstudio.com
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-74f9fa9b6a384fc1 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-74f9fa9b6a384fc1 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.1m4pmtwus1f4yl6otbbj3f5m5.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.1zszbv0jadgx2dht1k8z0gd0k.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.2492jbbba4vngtgf6umbjzmvy.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.2o0tkk0q1zu072jiej4qvy3pk.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.2op8dxzuyjo7narrtpmkc43uq.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.2vclwbhszma3ro3lt8fno97af.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.30j7hz107t5sz8jx5sxgzdx5o.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.3bn5cth9nu1e73ttmbez4bkkc.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a.24x�� /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a.2eic40pkfx5wikhd73nzwyiy4.1ul0bdp.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a.2s1x3nf0f4deblv1kjt23ls7h.1ul0bdp.rcgu.o lap_�� lap_derive.40302fb925fcb60e-cgu.13.rcgu.o lap_derive.40302fb925fcb60e-cgu.14.rcgu.o f/dyn-clone-1.0.-Wl,-Bstatic s/zerovec_derivecc s/zerovec_derive-Wl,--version-script=/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/rustcotNL/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.028suq3upi66gm39ouv590ni0.0f0futi.rcgu.o s/zerovec_derive-Wl,--no-undefined-version known-linux-gnu/-m64 (dns block)
  • spsprodweu4.vssps.visualstudio.com
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-74f9fa9b6a384fc1 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-74f9fa9b6a384fc1 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.1m4pmtwus1f4yl6otbbj3f5m5.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.1zszbv0jadgx2dht1k8z0gd0k.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.2492jbbba4vngtgf6umbjzmvy.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.2o0tkk0q1zu072jiej4qvy3pk.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.2op8dxzuyjo7narrtpmkc43uq.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.2vclwbhszma3ro3lt8fno97af.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.30j7hz107t5sz8jx5sxgzdx5o.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.3bn5cth9nu1e73ttmbez4bkkc.0f0futi.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a.24x�� /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a.2eic40pkfx5wikhd73nzwyiy4.1ul0bdp.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a.2s1x3nf0f4deblv1kjt23ls7h.1ul0bdp.rcgu.o lap_�� lap_derive.40302fb925fcb60e-cgu.13.rcgu.o lap_derive.40302fb925fcb60e-cgu.14.rcgu.o f/dyn-clone-1.0.-Wl,-Bstatic s/zerovec_derivecc s/zerovec_derive-Wl,--version-script=/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/rustcotNL/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-b2d9d78aa816a53c.028suq3upi66gm39ouv590ni0.0f0futi.rcgu.o s/zerovec_derive-Wl,--no-undefined-version known-linux-gnu/-m64 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Move {{ awf_mounts }} to its own template line so replace_with_indent
handles indentation automatically. When no mounts exist, emit a bare
bash continuation marker (\) to preserve the surrounding \-chain.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Rust PR Review

Summary: Looks good overall — clean trait design, well-tested, correct bash continuation logic. One documentation bug and one PR description inaccuracy to resolve.

Findings

🐛 Bugs / Logic Issues

  • docs/runtimes.md documents removed behavior — The updated runtimes.md adds the line "Symlinks lean tools into /tmp/awf-tools/ as a defense-in-depth fallback", but the PR completely removes the symlink loop from generate_lean_install(). The symlinks no longer exist in the generated pipeline; only the --mount path remains. The doc should drop that line. The code comment in lean/mod.rs is already correct — only docs/runtimes.md is wrong.

⚠️ Suggestions

  • PR description says "both AWF invocations (agent + detection)" but both template files only received the marker in the agent-job section. The detection job at lines ~481 (base.yml) and ~512 (1es-base.yml) does not get {{ awf_mounts }}, which is correct (and matches docs/template-markers.md saying "agent job only"). Just a misleading PR description — no code change needed, but worth noting.

  • AwfMount::from_str single-segment input — a bare string with no : (e.g. "elan") falls through to the _ bail arm, which is correct. Worth adding a test for this edge case to lock the contract.

✅ What Looks Good

  • The \ fallback in generate_awf_mounts when mounts are empty is clever: replace_with_indent produces \ which is a valid bash line-continuation, keeping the surrounding \-chain intact.
  • AwfMount::from_str correctly uses splitn(3, ':') so paths like $HOME/.elan (no colons) parse without ambiguity.
  • Serde round-trip test covers the serialize/deserialize impls cleanly.
  • Removing the fragile symlink loop in favour of an explicit --mount flag is the right call; no user-visible regression since the Lean toolchain becomes more reliably accessible inside the chroot.
  • Trait default required_awf_mounts() -> vec![] means zero impact on existing extensions with no code changes needed.

Generated by Rust PR Reviewer for issue #354 · ● 413.4K ·

jamesadevine and others added 2 commits April 29, 2026 15:52
Always store an explicit AwfMountMode instead of Option<AwfMountMode>.
Parsing 'host:container' without a mode suffix now defaults to ReadOnly
(secure default). Display/Serialize always emit the mode suffix so
generated AWF flags are self-documenting.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… test

The symlink loop was removed from generate_lean_install() but the doc
still referenced it. Also adds a test for single-segment AwfMount parse
input to lock the error contract.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Rust PR Review

Summary: Looks good overall — clean trait extension, solid test coverage for new types, and the template/indent wiring is correct. One integration test gap and a PR description inaccuracy are worth addressing.

Findings

🐛 Bugs / Logic Issues

  • tests/compiler_tests.rs:2692test_lean_runtime_compiled_output doesn't verify the --mount flag
    The existing Lean integration test compiles a full pipeline but never asserts that --mount "$HOME/.elan" appears in the output. If generate_awf_mounts were accidentally dropped from extra_replacements, or the {{ awf_mounts }} marker were removed from the template, this test would still pass. A single assertion like assert!(compiled.contains("--mount"), "should contain AWF mount for elan") would close that gap.

  • docs/template-markers.md vs PR description — "agent job only" vs "both invocations"
    template-markers.md correctly documents {{ awf_mounts }} as replacing in "the agent job AWF invocation only (not the detection job)". The PR description says "both AWF invocations (agent + detection)". The templates (base.yml:313, 1es-base.yml:342) confirm only the agent job gets the marker — the detection job's AWF invocation at base.yml:474 and 1es-base.yml:507 has no {{ awf_mounts }}. The code is right; the PR description is wrong. Worth correcting to avoid confusion for future reviewers.

⚠️ Suggestions

  • src/compile/extensions/mod.rsAwfMount fields are pub with no path validation
    host_path and container_path are written directly into a bash double-quoted string (--mount "spec" \). Today all mounts come from hardcoded required_awf_mounts() implementations, so there's no user-controlled input path — low risk for now. If this becomes user-configurable in the future (e.g., front-matter mounts: field), a simple check that paths don't contain " or \n should be added to AwfMount::new or FromStr.

  • src/compile/extensions/tests.rs:47 — test name test_awf_mount_display_no_mode is misleading
    The test creates a mount with AwfMountMode::ReadOnly and verifies the display — there's no "no mode" state. Renaming to test_awf_mount_display_readonly_mode or similar would reduce confusion.

✅ What Looks Good

  • replace_with_indent is correctly used for all extra_replacements (confirmed in common.rs:1874), so multi-mount indentation is properly handled without any special casing.
  • The \ fallback when no mounts are present preserves the bash \-continuation chain cleanly — verified against both templates.
  • AwfMountMode and AwfMount serde round-trip is well-tested, including the 2-part (no explicit mode) parse case.
  • Removing the symlink workaround in generate_lean_install is the right call now that the mount provides direct access.
  • The trait default (vec![]) and extension_enum! macro delegation follow the established pattern for required_hosts() exactly.

Generated by Rust PR Reviewer for issue #354 · ● 847.8K ·

@jamesadevine jamesadevine merged commit f6f437b into main Apr 29, 2026
15 checks passed
@jamesadevine jamesadevine deleted the feat/awf-mounts-lean-runtime branch April 29, 2026 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants