Repository navigation
feat(compiler): bindings auto-injection WASI + corrección ABI tras review 4R - #20
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds shared WASI, console, and filesystem bindings for AssemblyScript, C++, and Rust. It adds Rust dependency injection, compiler binding resolution, configurable WASI mounts, filesystem examples, cache support, tests, CI updates, and documentation. ChangesBindings and filesystem mounts
Estimated code review effort: 5 (Critical) | ~90 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
scripts/test-examples.mjsOops! Something went wrong! :( ESLint: 10.7.0 TypeError: Cannot read properties of undefined (reading 'Cjs') Comment |
C++ (-nostdlib), Rust (no_std), AssemblyScript wrapper layers over raw __wasi_* syscalls: fd I/O, path ops, preopen scanning, env access.
log, warn, error, assert with auto-appended newlines. C++ (-nostdlib), Rust (no_std), AssemblyScript.
readFile, writeFile, exists, unlink, mkdir with automatic prefix-based path resolution across preopened WASI directories. C++ (-nostdlib), Rust (no_std), AssemblyScript.
cpp-saludo: now uses console::log() rust-hello: now uses console::log() mounts-demo (new C++): console::log + fs::readFile with path resolution rust-fs (new Rust): console::log + fs::read_file as-fs (new AS): console.log + fs.readFile All examples include local copies of wasi/console/fs wrappers
…ttributes C++: - __wasi_iovec_t: uint64_t buf → uint32_t buf (WASM iovec = 8 bytes) - (uint64_t)(uintptr_t) → (uint32_t)(uintptr_t) casts Rust: - #[link(wasm_import_module = "wasi_snapshot_preview1")] for extern C - #[link_name = "fd_write"] etc. for each WASI fn - Make extern fns pub for cross-module access - impl From<WasiError> for i32 for ?-operator in fs.rs Examples: - rust-hello: use wasi → mod wasi - rust-fs: add BumpAlloc #[global_allocator] for Vec - mounts-demo, rust-fs, as-fs: graceful fallback when no preopen dirs - Update expected stdout for mount examples - Rename AS import files .as.ts → .ts
C++: - __wasi_iovec_t: uint64_t buf → uint32_t buf (WASM iovec = 8 bytes) - (uint64_t)(uintptr_t) → (uint32_t)(uintptr_t) casts Rust: - #[link(wasm_import_module = "wasi_snapshot_preview1")] for extern C - #[link_name = "fd_write"] etc. for each WASI fn - impl From<WasiError> for i32
- Mueve BumpAlloc + panic_handler de main.wasm.rs a wasi.rs en rust-fs y rust-hello - console::log/warn/error ahora es infalible (void, sin Result) en rust-fs, rust-hello, y template console.rs - as-fs usa log(String.UTF8.decode()) en vez de WASI iovec manual - Limpia boilerplate: -127 líneas netas
- AssemblyScript: asc --path al directorio de bindings (src/index.ts)
- C++: -I del directorio de bindings en cmake y clang++ (cpp-strategy.ts)
- bindings/*.{ts,h,rs}: fuentes compartidas que se copian a dist en el build
- tsconfig: excluye src/bindings (son fuentes AssemblyScript, no TS a compilar)
- copy-bindings.mjs: copia src/bindings -> dist/bindings (tsc no copia assets)
- .prettierignore: bindings/*.ts usan decoradores @external de AssemblyScript
…anifest - escapeTomlPathValue + injectBindingsDependency exportadas (puras, D4) - generateTempCargoToml publico: inyecta dep del crate vendido (REQ-2), [lib] path apunta al fuente (cargo no encontraba la lib) - compile(): manifest existente backup -> inyectar -> build -> restaurar bytes en finally (Cargo.toml + Cargo.lock, D3); temp case limpia como hoy - tests: inyeccion (byte-identical, ultima, EOF, idempotente, CRLF), win32, ciclo de vida del manifest (exito y fallo), Cargo.lock creado por cargo - .gitignore: lockfile del crate vendido no se commitea
- proposal, spec, design (D1-D5) y tasks: fase 1-3 completas (PR 1) - diseño autoritativo: macro-only allocator, backup/restore byte-clean, funciones puras de inyección, deviación D5 (rust-hello SI invoca wasm_setup)
Consume el crate vendido inyectado por el compilador: wasm_setup!() instala el allocator talc y el panic handler (D5 — la rlib referencia maquinaria de alloc/panic aunque el consumidor solo use console). Se eliminan console.rs y wasi.rs locales; Cargo.lock regenerado con wasm_apps_bindings, talc 5.0.4, lock_api y allocator-api2.
Consume el crate vendido inyectado por el compilador: wasm_setup!() instala el allocator talc (REQ-4 — Vec en fs::read_file requiere allocator); mantiene extern crate alloc. Se eliminan console.rs, wasi.rs y fs.rs locales; Cargo.lock regenerado con wasm_apps_bindings, talc 5.0.4, lock_api y allocator-api2.
asc 0.28 no resuelve imports bare via --path (busca ~lib/<nombre> en la stdlib, que secuestra console y wasi). rewriteBindingImports() convierte 'console'/'fs'/'wasi' a rutas relativas con extension .ts desde el directorio del fuente, en disco y en temp. Tests: 6 unit + 1 wiring.
main.wasm.ts importa console/fs bare (rewrite en compilador los resuelve a dist/bindings). Se eliminan src/console.ts y src/fs.ts.
main.wasm.cpp incluye <console.h>/<wasi.h> resueltos via -I del compilador a dist/bindings. Se eliminan src/console.h y src/wasi.h.
main.wasm.cpp incluye <console.h>/<fs.h>/<wasi.h> resueltos via -I del compilador a dist/bindings. Se eliminan las tres copias locales.
Todos los tasks 1-5 marcados [x]: copias locales de as/cpp eliminadas, suite unit 380/380 y 12/12 ejemplos pasan sin copias locales.
- prestat: struct de 8 bytes (tag + padding + pr_name_len@4) en C, AS y Rust - makeIovec: elimina write fuera de bounds en offset 8 - path_open: oflags WASI (O_CREAT 0x01, O_TRUNC 0x08) en vez de Linux - auto-pr: polling con gh run list --workflow CI (check runs)
Los decoradores @external de AssemblyScript no son TS válido para prettier; mismo motivo que packages/compiler/src/bindings/*.ts
…ttier" This reverts commit d5e2772.
59d8ed2 to
65cece4
Compare
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @AndyTechnologies. * #20 (comment) The following files were modified: * `examples/as-fs/src/main.wasm.ts` * `examples/cpp-saludo/src/main.wasm.cpp` * `examples/mounts-demo/src/main.wasm.cpp` * `examples/rust-fs/src/main.wasm.rs` * `examples/rust-hello/src/main.wasm.rs` * `packages/compiler/src/bindings/console.h` * `packages/compiler/src/bindings/console.rs` * `packages/compiler/src/bindings/console.ts` * `packages/compiler/src/bindings/fs.h` * `packages/compiler/src/bindings/fs.rs` * `packages/compiler/src/bindings/fs.ts` * `packages/compiler/src/bindings/rust/src/console.rs` * `packages/compiler/src/bindings/rust/src/fs.rs` * `packages/compiler/src/bindings/rust/src/wasi.rs` * `packages/compiler/src/bindings/wasi.h` * `packages/compiler/src/bindings/wasi.rs` * `packages/compiler/src/bindings/wasi.ts` * `packages/compiler/src/index.ts` * `packages/compiler/src/strategies/rust-strategy.ts` * `packages/linker/templates/console.as.ts` * `packages/linker/templates/console.h` * `packages/linker/templates/console.rs` * `packages/linker/templates/fs.as.ts` * `packages/linker/templates/fs.h` * `packages/linker/templates/fs.rs` * `packages/linker/templates/wasi.as.ts` * `packages/linker/templates/wasi.h` * `packages/linker/templates/wasi.rs`
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/compiler/src/index.ts (1)
144-168: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftCopying only the entry file to
tmpDirbreaks sibling relative imports.Any source that imports
console,fsorwasinow compiles fromtmpDirinstead of its real path. Only that one file is copied. A source that also containsfrom "./math"then fails, because./mathdoes not exist in the temp tree. The comment at lines 136-138 states that compiling from the original path is what keeps relative imports resolvable.A second defect follows from line 154: if
resolvedFileNamelies outsidePROJECT_ROOT,path.relativereturns a..-prefixed path, sosrcFileescapestmpDirand thefinallycleanup at line 233 never removes it.Prefer writing the rewritten source next to the original file under a temporary name, or copy the whole source directory into
tmpDir.Run the following script to check whether any example or test source mixes binding imports with relative imports:
#!/bin/bash # Description: Find AS sources that import bindings and also use relative imports. rg -l --glob '*.wasm.ts' -e "from ['\"](console|fs|wasi)['\"]" . | while IFS= read -r f; do rg -n -e "from ['\"]\./" -e "from ['\"](console|fs|wasi)['\"]" "$f" | sed "s|^|$f:|" done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/index.ts` around lines 144 - 168, The temporary compilation path must preserve sibling relative imports and remain inside tmpDir. Update the source handling around rewriteBindingImports so rewritten files are placed next to the original under a safe temporary name or the complete source directory is copied, and ensure resolvedFileName values outside PROJECT_ROOT cannot produce ..-prefixed paths escaping tmpDir; retain cleanupTmpDir behavior for temporary artifacts.
🟡 Minor comments (12)
.prettierignore-12-15 (1)
12-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winResolve the remaining formatter failure before merge.
These patterns ignore compiler source and distribution files only. The PR objectives state that
pnpm checkstill fails on AssemblyScript linker templates. Format those templates instead of leaving the check red. Do not broaden this rule to hide unrelated files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.prettierignore around lines 12 - 15, Update the AssemblyScript linker template files referenced by the compiler formatting check so they are validly formatted and pnpm check passes. Remove or narrow the .prettierignore entries for packages/compiler/src/bindings/*.ts and packages/compiler/dist/bindings/*.ts, and do not add broader ignore patterns.openspec/changes/bindings-auto-injection/tasks.md-65-65 (1)
65-65: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDo not mark integration verification complete before it runs.
Task 5.3 claims that all 12 examples build and run. The PR objectives state that integration tests and manual validation are still pending. Keep this task unchecked until
node scripts/test-examples.mjscompletes and the result is recorded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/bindings-auto-injection/tasks.md` at line 65, Update task 5.3 in the checklist to remain unchecked until node scripts/test-examples.mjs has completed successfully; do not claim all 12 examples build and run or mark integration verification complete before recording the actual result.openspec/changes/bindings-auto-injection/design.md-37-37 (1)
37-37: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSpecify the fenced-code language.
Use
textfor the ASCII data-flow block somarkdownlintcan validate it.Suggested change
-``` +```text🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/bindings-auto-injection/design.md` at line 37, Specify the fenced-code language for the ASCII data-flow block in the design document by changing its unlabeled fence to text, preserving the block contents.Source: Linters/SAST tools
openspec/changes/bindings-auto-injection/design.md-39-44 (1)
39-44: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRemove temporary
Cargo.lockwhenhasCargoToml === false.When a temporary
Cargo.tomlis created (line 143),cargo buildat line 150 can generateCargo.lockin the source directory. ThehasCargoToml === falsecleanup path (lines 196–199) removes the temporaryCargo.tomlbut does not remove the generatedCargo.lock. In contrast, thehasCargoToml === truepath (lines 187–195) properly cleans upCargo.lockwhen it was generated during the build. Add equivalent cleanup for the temporaryCargo.lockin thehasCargoToml === falsebranch, and test cleanup after both build success and build failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/bindings-auto-injection/design.md` around lines 39 - 44, Update the hasCargoToml === false cleanup branch to remove the Cargo.lock generated alongside the temporary Cargo.toml, matching the existing restoration cleanup used when hasCargoToml === true. Ensure this cleanup runs after both successful and failed builds, and add coverage for both outcomes.openspec/changes/bindings-auto-injection/proposal.md-55-55 (1)
55-55: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAddress compatibility risk from
talc = "5"semver range.
talc = "5"permits any version from 5.0.0 to 5.999.999. The bindings crate is vendored, buttalcis fetched from crates.io during builds. Cargo.lock is not committed (excluded in .gitignore). The proposal documents this as a "Low" risk and requires verification "at apply time," but does not show automated enforcement.Implement one of the following:
- Narrow to an exact version (e.g.,
talc = "=5.0.4") that was validated during design.- Generate and commit a Cargo.lock for bindings builds to pin resolved versions.
- Automate compatibility verification to detect talc API drift before build.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/bindings-auto-injection/proposal.md` at line 55, Update the talc dependency compatibility plan in the bindings auto-injection proposal to provide automated version/API-drift enforcement instead of relying only on “verify at apply time.” Choose one supported approach: pin talc to the validated exact version, commit a Cargo.lock for bindings builds, or add a pre-build compatibility check that detects talc API drift; document the selected mechanism and change the risk assessment accordingly.packages/compiler/src/strategies/rust-strategy.ts-200-207 (1)
200-207: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winThe
throwinfinallydiscards the original error and reports a wrong message.Biome flags this as
lint/correctness/noUnsafeFinally. Two concrete defects follow:
- If the build failed and the restore also failed, the build error is lost. The caller never sees the compiler diagnostics.
- If the build succeeded and only the restore failed, the message still claims "Rust build failed", which is false.
Capture the pending error and attach it, then build the message from the actual state.
🐛 Proposed fix
- } catch (restoreErr: any) { - // El restore nunca debe enmascarar el error original del build ni dejar - // el workspace con el manifest inyectado silenciosamente (D3). - throw new CompilerError(`Rust build failed and restoring Cargo.toml/Cargo.lock also failed: ${restoreErr.message ?? restoreErr}`, { - code: 'COMPILER_ERROR', - fileName: options.fileName, - stderr: restoreErr.message ?? restoreErr, - }); + } catch (restoreErr: any) { + // El restore no debe perder el error original del build (D3). + throw new CompilerError(`Restoring Cargo.toml/Cargo.lock failed after the Rust build: ${restoreErr.message ?? restoreErr}`, { + code: 'COMPILER_ERROR', + fileName: options.fileName, + stderr: restoreErr.message ?? restoreErr, + restoreError: restoreErr.message ?? String(restoreErr), + }); }To preserve the build error, store it in the
catchblock above and include it indetailshere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/strategies/rust-strategy.ts` around lines 200 - 207, Update the Rust build cleanup flow around the restore-error catch and its enclosing finally so it never throws directly from finally. Preserve the original build error captured by the preceding catch, attach it to the restore failure details, and construct the CompilerError message based on whether the build failed, restoration failed, or both, so successful builds with restore failures are not reported as build failures.Source: Linters/SAST tools
packages/compiler/src/strategies/rust-strategy.ts-196-199 (1)
196-199: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the lockfile that cargo created in the temporary-manifest path.
When no
Cargo.tomlexisted, cargo still writesCargo.lockinto the source directory during the build. This branch deletes only the temporaryCargo.toml, so the lockfile stays in the user's tree. ThehasCargoTomlbranch already deletes an unexpected lockfile at line 194. Make the cleanup consistent.🐛 Proposed fix
} else if (fs.existsSync(cargoTomlPath)) { // Limpiar el Cargo.toml temporal que creamos fs.rmSync(cargoTomlPath, { force: true }); + // Cargo pudo crear un Cargo.lock junto al manifest temporal + if (!backupLock) fs.rmSync(cargoLockPath, { force: true }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/strategies/rust-strategy.ts` around lines 196 - 199, Update the cleanup branch around cargoTomlPath and the existing hasCargoToml lockfile removal to also delete the Cargo.lock created in the temporary-manifest path, using the same forceful cleanup behavior and preserving the existing Cargo.toml cleanup.packages/compiler/src/strategies/cpp-strategy.ts-70-70 (1)
70-70: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
CMAKE_CXX_FLAGSbreaks when the bindings path contains a space.CMake stores
CMAKE_CXX_FLAGSas one string and splits it on whitespace when it composes compiler command lines. IfBINDINGS_DIRcontains a space, the include path breaks. Package installation paths commonly contain spaces on macOS and Windows. Quote the path inside the flag value.🐛 Proposed fix
- cmakeArgs.push(`-DCMAKE_CXX_FLAGS=-I${BINDINGS_DIR}`); + cmakeArgs.push(`-DCMAKE_CXX_FLAGS:STRING=-I"${BINDINGS_DIR}"`);Setting this cache variable also replaces any
CMAKE_CXX_FLAGSthe project expects. Consider-DCMAKE_CXX_STANDARD_INCLUDE_DIRECTORIESor a CMake include-path variable instead, so project flags stay intact.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/strategies/cpp-strategy.ts` at line 70, Update the CMake argument construction near cmakeArgs to avoid embedding BINDINGS_DIR in CMAKE_CXX_FLAGS: use CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES or another dedicated CMake include-path variable, preserving existing project compiler flags and correctly handling paths containing spaces.packages/linker/templates/fs.rs-1-26 (1)
1-26: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd
extern crate allocand re-exportVecandvecto fix compilation in thisno_stdmodule.The module declares
no_stdcompatible (line 1) but usesVec<u8>(line 10) andvec without imports. Inno_stdcrates, neitherVecnorvecis in the prelude. The working version atpackages/compiler/src/bindings/rust/src/fs.rsincludes the correct setup:extern crate alloc; use alloc::vec; use alloc::vec::Vec;Add these three lines after line 4 (
use crate::wasi;). The template version andpackages/compiler/src/bindings/fs.rsboth lack these imports; the bindings/rust version already has them and compiles correctly.Secondary issue: If
fd_read()orfd_write()returns an error (lines 58, 67, 75), the?operator causes early return beforefd_close()executes. Consider wrapping these operations in scope guards or explicit error handling to ensure file descriptors close even on read/write failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/linker/templates/fs.rs` around lines 1 - 26, Add extern crate alloc plus alloc::vec and alloc::vec::Vec imports near the existing crate::wasi import so Vec and vec! compile in this no_std module. Also update the fd_read and fd_write flows in the relevant file-operation functions to ensure fd_close executes when either operation returns an error, using explicit cleanup or an appropriate scope guard while preserving error propagation.packages/compiler/src/bindings/fs.ts-79-90 (1)
79-90: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
readFiletruncates files larger than 64 KiB without an error.The buffer is fixed at 65536 bytes and only one
fdReadcall runs. A larger file returns partial content, and the caller cannot detect the truncation. A short read on a smaller file also returns less than the full content.Read in a loop until
fdReadreturns 0, and grow the result buffer. Alternatively, usepathFilestatGetto size the buffer first.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/bindings/fs.ts` around lines 79 - 90, Update readFile to read the entire opened file instead of performing a single fixed 64 KiB fdRead; loop until fdRead returns 0, growing or appending to the result buffer while preserving the existing error handling and closing the descriptor.packages/linker/templates/fs.h-8-8 (1)
8-8: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winGuard the
namespaceblock for C translation units.This template copy has the same gap as
packages/compiler/src/bindings/fs.h. Wrapnamespace fsin#ifdef __cplusplus.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/linker/templates/fs.h` at line 8, Guard the namespace fs declaration in the fs.h template with an __cplusplus preprocessor condition, matching the existing protection in the compiler binding header, so C translation units do not parse the C++ namespace block.Source: Linters/SAST tools
packages/compiler/src/bindings/fs.h-8-8 (1)
8-8: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winGuard the
namespaceblock for C translation units.
wasi.hwraps itsnamespace wasiin#ifdef __cplusplus, but this header does not. If a C translation unit includesfs.h, the compiler fails at line 8. Clang reports "unknown type name 'namespace'" for this line.🛡️ Proposed fix for C compatibility
+#ifdef __cplusplus namespace fs {Close the guard at the end of the file:
} // namespace fs +#endif🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/bindings/fs.h` at line 8, Guard the fs namespace block with an __cplusplus preprocessor check, adding the opening guard before namespace fs and closing it at the end of the header, so C translation units can include fs.h without parsing C++ namespace syntax.Source: Linters/SAST tools
🧹 Nitpick comments (6)
scripts/test-examples.mjs (1)
20-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd mounted-file integration coverage.
Extend the WASI runtime configuration to preopen
dataat/mnt/data;inherit_argv()does not create a preopen. Then add success-path assertions foras-fs,mounts-demo, andrust-fsthat expectHello from mounted directory!.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/test-examples.mjs` around lines 20 - 22, Update the WASI runtime configuration in scripts/test-examples.mjs to explicitly preopen the data directory at /mnt/data rather than relying on inherit_argv(). Add success-path assertions for as-fs, mounts-demo, and rust-fs that verify the output contains “Hello from mounted directory!”.packages/compiler/src/strategies/rust-strategy.test.ts (1)
366-441: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd lifecycle coverage for the temporary-manifest path.
These tests all start from an existing
Cargo.toml. No test covers the branch where noCargo.tomlexists. That branch is where cleanup is incomplete, as noted onpackages/compiler/src/strategies/rust-strategy.tslines 196-199. Add a test that compiles a bare.wasm.rs, lets the mocked cargo createCargo.lock, and asserts that both the temporaryCargo.tomland the createdCargo.lockare gone afterwards.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/strategies/rust-strategy.test.ts` around lines 366 - 441, Add a lifecycle test alongside the existing manifest tests for compiling a bare .wasm.rs without an initial Cargo.toml. Mock execFile to create Cargo.lock during the build, then assert after strategy.compile completes that both the temporary Cargo.toml and generated Cargo.lock are absent.packages/compiler/src/strategies/rust-strategy.ts (1)
30-34: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winEscape the TOML path value on every platform.
TOML basic strings treat
\as an escape introducer regardless of the host platform. A posix path that contains\or"therefore produces an invalid manifest whenisWin32is false. Escaping is a no-op for ordinary posix paths, so the flag adds risk without benefit. ApplyescapeTomlPathValueunconditionally and keepisWin32only if callers still need it.♻️ Proposed change
- const depLine = `wasm_apps_bindings = { path = "${isWin32 ? escapeTomlPathValue(bindingsDir) : bindingsDir}" }`; + const depLine = `wasm_apps_bindings = { path = "${escapeTomlPathValue(bindingsDir)}" }`;This change also requires updating the expectation at
packages/compiler/src/strategies/rust-strategy.test.ts:345-349.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/strategies/rust-strategy.ts` around lines 30 - 34, Update injectBindingsDependency to apply escapeTomlPathValue to bindingsDir unconditionally when constructing depLine, preventing backslashes and quotes from producing invalid TOML on any platform. Remove isWin32 from the function and its callers if it is no longer needed, and update the corresponding rust-strategy test expectation to match the always-escaped path.packages/compiler/src/bindings/fs.h (1)
8-8: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBoth
fs.hcopies share two defects: an unguardednamespaceblock andallocainside the descriptor loop.wasi.hguards its namespace with#ifdef __cplusplus, butfs.hdoes not, so a C translation unit fails at line 8.resolve_pathalso calls__builtin_allocaper iteration, and that memory is released only when the function returns.
packages/compiler/src/bindings/fs.h#L8-L8: wrapnamespace fsin#ifdef __cplusplusand close the guard after line 145.packages/linker/templates/fs.h#L8-L8: apply the same guard in this generated copy.packages/compiler/src/bindings/fs.h#L17-L55: declare one fixed-sizedirnamebuffer before the loop and skip descriptors whosepr_name_lendoes not fit.packages/linker/templates/fs.h#L17-L55: apply the same buffer change in this generated copy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/bindings/fs.h` at line 8, Update both packages/compiler/src/bindings/fs.h (lines 8-8 and 17-55) and packages/linker/templates/fs.h (lines 8-8 and 17-55): guard the fs namespace block with __cplusplus through its closing point, and in resolve_path declare one fixed-size dirname buffer before the descriptor loop, skipping descriptors whose pr_name_len exceeds the buffer capacity instead of allocating with __builtin_alloca per iteration.Source: Linters/SAST tools
packages/compiler/src/bindings/fs.ts (1)
20-38: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
resolvePathperforms a redundantfd_prestat_getper descriptor.
fdPrestatDirNameinpackages/compiler/src/bindings/wasi.tsline 136 already callsfdPrestatGet. Theprestatvalue at line 27 is used only as a null check, so each of the 61 iterations issues twofd_prestat_getcalls. Drop lines 27 and 28 and rely on thedirName == nullcheck at line 32. This also halves the heap allocations leaked bywasi.ts.♻️ Proposed refactor
for (let fd: i32 = 3; fd <= 63; fd++) { - // Get prestat - let prestat = fdPrestatGet(fd); - if (prestat == null) continue; - // Get dir name let dirName = fdPrestatDirName(fd); if (dirName == null) continue;Remove
fdPrestatGetfrom the import list at lines 3 to 6 after this change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/bindings/fs.ts` around lines 20 - 38, Remove the redundant fdPrestatGet call and prestat null check from resolvePath, relying on fdPrestatDirName’s null result to skip unavailable descriptors. Also remove fdPrestatGet from the bindings import list, leaving the existing dirName handling unchanged.packages/compiler/src/bindings/wasi.ts (1)
5-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBiome cannot parse the AssemblyScript
@externaldecorators in both files. The shared root cause is tool configuration: AssemblyScript sources use.tsextensions, so Biome and Prettier treat them as TypeScript and report parse errors on lines 5 to 45.
packages/compiler/src/bindings/wasi.ts#L5-L45: excludepackages/compiler/src/bindingsfrom the Biome configuration and from.prettierignore.packages/linker/templates/wasi.as.ts#L5-L45: excludepackages/linker/templatesin the same way, which also unblocks thepnpm checkfailure reported in the PR objectives.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/bindings/wasi.ts` around lines 5 - 45, Exclude packages/compiler/src/bindings from both Biome configuration and .prettierignore, covering the wasi.ts AssemblyScript bindings at packages/compiler/src/bindings/wasi.ts lines 5-45. Apply the same Biome and Prettier exclusions to packages/linker/templates, covering wasi.as.ts lines 5-45, so these AssemblyScript sources are not parsed as TypeScript and pnpm check succeeds.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/auto-pr.yml:
- Around line 55-63: Update the CI polling command in the workflow to query both
status and conclusion, then fail immediately whenever status is completed and
conclusion is not success. Continue polling only for non-completed runs, while
preserving the successful exit path and existing pending behavior.
In @.github/workflows/release.yml:
- Line 33: Update the release workflow’s pnpm install command to use
--frozen-lockfile instead of --no-frozen-lockfile, and commit any required
pnpm-lock.yaml changes so dependency resolution remains aligned with the
committed lockfile.
In
`@openspec/changes/bindings-auto-injection/specs/bindings-auto-injection/spec.md`:
- Line 11: Align the affected requirements in the spec with the actual binding
implementation: update REQ-1, REQ-4, and REQ-5 so the vendored crate details
match the current symbols in wasm_apps_bindings, remove the pub use
alloc::wasm_setup re-export from the crate shape, switch the internal allocation
reference to $crate::alloc::_talc_reexport, and require rust-hello to call
wasm_setup!() directly. Keep the related scenarios and tests in the same spec
consistent with these symbol and usage changes.
In `@packages/compiler/src/bindings/rust/src/fs.rs`:
- Around line 56-64: Actualiza read_file en
packages/compiler/src/bindings/rust/src/fs.rs (líneas 56-64) y
packages/compiler/src/bindings/fs.rs (líneas 52-60) para leer en un bucle,
acumulando bloques de hasta 64 KiB hasta que fd_read devuelva cero bytes;
conserva el cierre del descriptor y devuelve todos los datos leídos, no solo el
primer bloque.
In `@packages/compiler/src/bindings/rust/src/wasi.rs`:
- Around line 60-69: Corrija el layout de __wasi_filestat_t para cumplir los 64
bytes de WASI Preview1: cambie st_filetype a u8, agregue siete bytes de padding
y use st_nlink: u64 en el offset 24. Actualice stat_file para convertir
st_filetype a u32. Ajuste también RIGHTS_READ a 1 << 1 y RIGHTS_WRITE a 1 << 6
para solicitar los permisos correctos.
- Around line 169-185: Corrige las constantes RIGHTS_READ y RIGHTS_WRITE usadas
por path_open: asigna 0x02 a RIGHTS_READ y 0x40 a RIGHTS_WRITE según WASI
Preview 1. Mantén sin cambios la lógica que combina estos derechos en
rights_base.
In `@packages/compiler/src/bindings/wasi.h`:
- Around line 32-36: Align the __wasi_filestat_t definitions in
packages/compiler/src/bindings/wasi.h (32-36) and
packages/linker/templates/wasi.h (32-36) to the 64-byte WASI layout using
explicit padding. Update the corresponding Filestat structs in
packages/compiler/src/bindings/wasi.rs (55-65) and
packages/linker/templates/wasi.rs (55-65) with u8 filetype, padding, and u64
nlink, and convert filetype to u32 in stat_file. In
packages/compiler/src/bindings/wasi.ts (168-182) and
packages/linker/templates/wasi.as.ts (168-182), allocate 64 bytes, load filetype
as u8 and nlink as i64, and shift later field offsets by 8 bytes.
- Around line 110-121: Update the path_open wrappers in
packages/compiler/src/bindings/wasi.h (110-121),
packages/linker/templates/wasi.h (110-121),
packages/compiler/src/bindings/wasi.rs (165-185),
packages/linker/templates/wasi.rs (165-185),
packages/compiler/src/bindings/wasi.ts (150-166),
packages/linker/templates/wasi.as.ts (150-166), and
packages/compiler/src/bindings/rust/src/wasi.rs to map fd_read to 1 << 1 and
fd_write to 1 << 6. Remove the C++ 0x04 rights mapping, preserve O_CREAT as
oflags 0x01, and ensure path_create_file (1 << 10) is provided on the parent
dir_fd rather than the returned descriptor’s fs_rights_base.
In `@packages/compiler/src/bindings/wasi.ts`:
- Around line 69-74: The WASI wrappers leak memory allocated by makeIovec and
their result cells. Update packages/compiler/src/bindings/wasi.ts lines 69-74
and packages/linker/templates/wasi.as.ts lines 69-74 consistently, freeing each
allocated iovec and result cell before returning or replacing them with reusable
module-level scratch buffers; ensure stdoutWrite, fdWrite, fdRead, fdSeek,
fdPrestatGet, pathOpen, pathFilestatGet, environSizesGet, and clockTime no
longer retain allocations.
In `@packages/compiler/src/strategies/rust-strategy.ts`:
- Around line 125-145: Serialize Rust compilations per source directory in
RustCompilerStrategy.compile(), covering the Cargo.toml injection, cargo
invocation, and restoration/cleanup flow as one critical section. Add or reuse a
directory-keyed mutex so concurrent compilations for the same source directory
cannot overwrite manifests or remove another compilation’s Cargo.lock, while
allowing different directories to compile concurrently.
In `@packages/linker/templates/fs.rs`:
- Around line 56-61: Update both readFile implementations to read complete files
instead of performing one fixed 64 KiB read: in packages/linker/templates/fs.rs
lines 56-61, loop wasi::fd_read until it returns 0, growing buf and truncating
to the total byte count; in packages/linker/templates/fs.as.ts lines 79-89, loop
fdRead until it returns 0, accumulate the chunks in a growable buffer, and copy
the total bytes into the returned ArrayBuffer.
- Around line 56-78: Update read_file, read_file_to_buf, and write_file to
capture the fd_read or fd_write result before propagating it, always call
fd_close after the operation, then return the captured result so both success
and error paths close the descriptor.
---
Outside diff comments:
In `@packages/compiler/src/index.ts`:
- Around line 144-168: The temporary compilation path must preserve sibling
relative imports and remain inside tmpDir. Update the source handling around
rewriteBindingImports so rewritten files are placed next to the original under a
safe temporary name or the complete source directory is copied, and ensure
resolvedFileName values outside PROJECT_ROOT cannot produce ..-prefixed paths
escaping tmpDir; retain cleanupTmpDir behavior for temporary artifacts.
---
Minor comments:
In @.prettierignore:
- Around line 12-15: Update the AssemblyScript linker template files referenced
by the compiler formatting check so they are validly formatted and pnpm check
passes. Remove or narrow the .prettierignore entries for
packages/compiler/src/bindings/*.ts and packages/compiler/dist/bindings/*.ts,
and do not add broader ignore patterns.
In `@openspec/changes/bindings-auto-injection/design.md`:
- Line 37: Specify the fenced-code language for the ASCII data-flow block in the
design document by changing its unlabeled fence to text, preserving the block
contents.
- Around line 39-44: Update the hasCargoToml === false cleanup branch to remove
the Cargo.lock generated alongside the temporary Cargo.toml, matching the
existing restoration cleanup used when hasCargoToml === true. Ensure this
cleanup runs after both successful and failed builds, and add coverage for both
outcomes.
In `@openspec/changes/bindings-auto-injection/proposal.md`:
- Line 55: Update the talc dependency compatibility plan in the bindings
auto-injection proposal to provide automated version/API-drift enforcement
instead of relying only on “verify at apply time.” Choose one supported
approach: pin talc to the validated exact version, commit a Cargo.lock for
bindings builds, or add a pre-build compatibility check that detects talc API
drift; document the selected mechanism and change the risk assessment
accordingly.
In `@openspec/changes/bindings-auto-injection/tasks.md`:
- Line 65: Update task 5.3 in the checklist to remain unchecked until node
scripts/test-examples.mjs has completed successfully; do not claim all 12
examples build and run or mark integration verification complete before
recording the actual result.
In `@packages/compiler/src/bindings/fs.h`:
- Line 8: Guard the fs namespace block with an __cplusplus preprocessor check,
adding the opening guard before namespace fs and closing it at the end of the
header, so C translation units can include fs.h without parsing C++ namespace
syntax.
In `@packages/compiler/src/bindings/fs.ts`:
- Around line 79-90: Update readFile to read the entire opened file instead of
performing a single fixed 64 KiB fdRead; loop until fdRead returns 0, growing or
appending to the result buffer while preserving the existing error handling and
closing the descriptor.
In `@packages/compiler/src/strategies/cpp-strategy.ts`:
- Line 70: Update the CMake argument construction near cmakeArgs to avoid
embedding BINDINGS_DIR in CMAKE_CXX_FLAGS: use
CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES or another dedicated CMake include-path
variable, preserving existing project compiler flags and correctly handling
paths containing spaces.
In `@packages/compiler/src/strategies/rust-strategy.ts`:
- Around line 200-207: Update the Rust build cleanup flow around the
restore-error catch and its enclosing finally so it never throws directly from
finally. Preserve the original build error captured by the preceding catch,
attach it to the restore failure details, and construct the CompilerError
message based on whether the build failed, restoration failed, or both, so
successful builds with restore failures are not reported as build failures.
- Around line 196-199: Update the cleanup branch around cargoTomlPath and the
existing hasCargoToml lockfile removal to also delete the Cargo.lock created in
the temporary-manifest path, using the same forceful cleanup behavior and
preserving the existing Cargo.toml cleanup.
In `@packages/linker/templates/fs.h`:
- Line 8: Guard the namespace fs declaration in the fs.h template with an
__cplusplus preprocessor condition, matching the existing protection in the
compiler binding header, so C translation units do not parse the C++ namespace
block.
In `@packages/linker/templates/fs.rs`:
- Around line 1-26: Add extern crate alloc plus alloc::vec and alloc::vec::Vec
imports near the existing crate::wasi import so Vec and vec! compile in this
no_std module. Also update the fd_read and fd_write flows in the relevant
file-operation functions to ensure fd_close executes when either operation
returns an error, using explicit cleanup or an appropriate scope guard while
preserving error propagation.
---
Nitpick comments:
In `@packages/compiler/src/bindings/fs.h`:
- Line 8: Update both packages/compiler/src/bindings/fs.h (lines 8-8 and 17-55)
and packages/linker/templates/fs.h (lines 8-8 and 17-55): guard the fs namespace
block with __cplusplus through its closing point, and in resolve_path declare
one fixed-size dirname buffer before the descriptor loop, skipping descriptors
whose pr_name_len exceeds the buffer capacity instead of allocating with
__builtin_alloca per iteration.
In `@packages/compiler/src/bindings/fs.ts`:
- Around line 20-38: Remove the redundant fdPrestatGet call and prestat null
check from resolvePath, relying on fdPrestatDirName’s null result to skip
unavailable descriptors. Also remove fdPrestatGet from the bindings import list,
leaving the existing dirName handling unchanged.
In `@packages/compiler/src/bindings/wasi.ts`:
- Around line 5-45: Exclude packages/compiler/src/bindings from both Biome
configuration and .prettierignore, covering the wasi.ts AssemblyScript bindings
at packages/compiler/src/bindings/wasi.ts lines 5-45. Apply the same Biome and
Prettier exclusions to packages/linker/templates, covering wasi.as.ts lines
5-45, so these AssemblyScript sources are not parsed as TypeScript and pnpm
check succeeds.
In `@packages/compiler/src/strategies/rust-strategy.test.ts`:
- Around line 366-441: Add a lifecycle test alongside the existing manifest
tests for compiling a bare .wasm.rs without an initial Cargo.toml. Mock execFile
to create Cargo.lock during the build, then assert after strategy.compile
completes that both the temporary Cargo.toml and generated Cargo.lock are
absent.
In `@packages/compiler/src/strategies/rust-strategy.ts`:
- Around line 30-34: Update injectBindingsDependency to apply
escapeTomlPathValue to bindingsDir unconditionally when constructing depLine,
preventing backslashes and quotes from producing invalid TOML on any platform.
Remove isWin32 from the function and its callers if it is no longer needed, and
update the corresponding rust-strategy test expectation to match the
always-escaped path.
In `@scripts/test-examples.mjs`:
- Around line 20-22: Update the WASI runtime configuration in
scripts/test-examples.mjs to explicitly preopen the data directory at /mnt/data
rather than relying on inherit_argv(). Add success-path assertions for as-fs,
mounts-demo, and rust-fs that verify the output contains “Hello from mounted
directory!”.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6bbfcff6-a88f-49e4-947c-a0aed503471f
⛔ Files ignored due to path filters (2)
examples/rust-fs/src/Cargo.lockis excluded by!**/*.lockexamples/rust-hello/src/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (51)
.github/workflows/auto-pr.yml.github/workflows/release.yml.gitignore.prettierignoredata/greeting.txtexamples/as-fs/expected-stdout.txtexamples/as-fs/src/main.wasm.tsexamples/cpp-saludo/src/main.wasm.cppexamples/mounts-demo/expected-stdout.txtexamples/mounts-demo/src/main.wasm.cppexamples/rust-fs/expected-stdout.txtexamples/rust-fs/src/Cargo.tomlexamples/rust-fs/src/main.wasm.rsexamples/rust-hello/src/main.wasm.rsopenspec/changes/bindings-auto-injection/design.mdopenspec/changes/bindings-auto-injection/proposal.mdopenspec/changes/bindings-auto-injection/specs/bindings-auto-injection/spec.mdopenspec/changes/bindings-auto-injection/tasks.mdpackages/compiler/package.jsonpackages/compiler/scripts/copy-bindings.mjspackages/compiler/src/bindings/console.hpackages/compiler/src/bindings/console.rspackages/compiler/src/bindings/console.tspackages/compiler/src/bindings/fs.hpackages/compiler/src/bindings/fs.rspackages/compiler/src/bindings/fs.tspackages/compiler/src/bindings/rust/Cargo.tomlpackages/compiler/src/bindings/rust/src/alloc.rspackages/compiler/src/bindings/rust/src/console.rspackages/compiler/src/bindings/rust/src/fs.rspackages/compiler/src/bindings/rust/src/lib.rspackages/compiler/src/bindings/rust/src/wasi.rspackages/compiler/src/bindings/wasi.hpackages/compiler/src/bindings/wasi.rspackages/compiler/src/bindings/wasi.tspackages/compiler/src/index.test.tspackages/compiler/src/index.tspackages/compiler/src/strategies/cpp-strategy.tspackages/compiler/src/strategies/rust-strategy.test.tspackages/compiler/src/strategies/rust-strategy.tspackages/compiler/tsconfig.jsonpackages/linker/templates/console.as.tspackages/linker/templates/console.hpackages/linker/templates/console.rspackages/linker/templates/fs.as.tspackages/linker/templates/fs.hpackages/linker/templates/fs.rspackages/linker/templates/wasi.as.tspackages/linker/templates/wasi.hpackages/linker/templates/wasi.rsscripts/test-examples.mjs
| pub fn read_file(path: &[u8]) -> Result<Vec<u8>, i32> { | ||
| let r = resolve_path(path).ok_or(-1)?; | ||
| let fd = wasi::path_open(r.dir_fd, &r.relative, false, false)?; | ||
|
|
||
| // Read in chunks up to 64KB | ||
| let mut buf = vec![0u8; 65536]; | ||
| let n = wasi::fd_read(fd, &mut buf)?; | ||
| let _ = wasi::fd_close(fd); | ||
| buf.truncate(n); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Lea hasta EOF en read_file.
Cada implementación ejecuta un solo fd_read de 64 KiB. El comentario indica lectura por bloques, pero no existe un bucle. Los archivos mayores de 64 KiB se devuelven truncados.
packages/compiler/src/bindings/rust/src/fs.rs#L56-L64: acumule bloques hasta quefd_readdevuelva cero bytes.packages/compiler/src/bindings/fs.rs#L52-L60: acumule bloques hasta quefd_readdevuelva cero bytes.
📍 Affects 2 files
packages/compiler/src/bindings/rust/src/fs.rs#L56-L64(this comment)packages/compiler/src/bindings/fs.rs#L52-L60
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/bindings/rust/src/fs.rs` around lines 56 - 64,
Actualiza read_file en packages/compiler/src/bindings/rust/src/fs.rs (líneas
56-64) y packages/compiler/src/bindings/fs.rs (líneas 52-60) para leer en un
bucle, acumulando bloques de hasta 64 KiB hasta que fd_read devuelva cero bytes;
conserva el cierre del descriptor y devuelve todos los datos leídos, no solo el
primer bloque.
| pub struct __wasi_filestat_t { | ||
| pub st_dev: u64, | ||
| pub st_ino: u64, | ||
| pub st_filetype: u32, | ||
| pub st_nlink: u32, | ||
| pub st_size: u64, | ||
| pub st_atim: u64, | ||
| pub st_mtim: u64, | ||
| pub st_ctim: u64, | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
What is the WASI Preview 1 __wasi_filestat_t ABI layout and size?
💡 Result:
In WASI Preview 1, the wasi_filestat_t ABI layout is a C-compatible structure with a total size of 64 bytes and an alignment of 8 bytes [1][2]. The structure consists of eight fields, all of which are 8-byte aligned types [1][2]. The layout and byte offsets are as follows: Field | Type | Offset (Bytes) --- | --- | --- dev | wasi_device_t (u64) | 0 ino | wasi_inode_t (u64) | 8 filetype | wasi_filetype_t (u8, padded to 8 bytes) | 16 nlink | wasi_linkcount_t (u64) | 24 size | wasi_filesize_t (u64) | 32 atim | wasi_timestamp_t (u64) | 40 mtim | wasi_timestamp_t (u64) | 48 ctim | wasi_timestamp_t (u64) | 56 The structure is explicitly defined in wasi-libc and other official WebAssembly-related implementations with static assertions to ensure this exact memory layout [1][2]. While individual field sizes such as wasi_filetype_t are smaller (typically a uint8_t), the requirements of the ABI force padding to maintain 8-byte alignment for the subsequent fields in the struct [1][3][2].
Citations:
- 1: https://github.com/WebAssembly/wasi-libc/blob/2b7e73ae7ac0bad6391d89cc3274a28412243389/libc-bottom-half/headers/public/wasi/api.h
- 2: https://github.com/emscripten-core/emscripten/blob/38eedc630f17094b3202fd48ac0c2c585dbea31e/system/include/wasi/api.h
- 3: https://docs.rs/krasm/latest/src/krasm/runtime/wasi/types.rs.html
🏁 Script executed:
#!/bin/bash
set -eu
file="packages/compiler/src/bindings/rust/src/wasi.rs"
printf '%s\n' '--- target structure and surrounding declarations ---'
rg -n -A18 -B8 'struct __wasi_filestat_t|__wasi_rights|path_open|path_filestat_get' "$file"
printf '%s\n' '--- relevant Rust binding usages ---'
rg -n -A12 -B8 '__wasi_filestat_t|path_filestat_get|path_open|RIGHTS' packages/compiler/src/bindings/rust/src
printf '%s\n' '--- file inventory for ABI tests or generated bindings ---'
git ls-files 'packages/compiler/src/bindings/rust/**' | sed -n '1,160p'Repository: AndyTechnologies/wasm-apps
Length of output: 19673
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import ctypes
class Current(ctypes.Structure):
_fields_ = [
("st_dev", ctypes.c_uint64),
("st_ino", ctypes.c_uint64),
("st_filetype", ctypes.c_uint32),
("st_nlink", ctypes.c_uint32),
("st_size", ctypes.c_uint64),
("st_atim", ctypes.c_uint64),
("st_mtim", ctypes.c_uint64),
("st_ctim", ctypes.c_uint64),
]
class WasiPreview1(ctypes.Structure):
_fields_ = [
("st_dev", ctypes.c_uint64),
("st_ino", ctypes.c_uint64),
("st_filetype", ctypes.c_uint8),
("_padding", ctypes.c_uint8 * 7),
("st_nlink", ctypes.c_uint64),
("st_size", ctypes.c_uint64),
("st_atim", ctypes.c_uint64),
("st_mtim", ctypes.c_uint64),
("st_ctim", ctypes.c_uint64),
]
for cls in (Current, WasiPreview1):
print(cls.__name__, "size=", ctypes.sizeof(cls), "alignment=", ctypes.alignment(cls))
for name, _ in cls._fields_:
print(" ", name, getattr(cls, name).offset)
print("WASI rights values:")
print("RIGHTS_FD_DATASYNC =", 1 << 0)
print("RIGHTS_FD_READ =", 1 << 1)
print("RIGHTS_FD_WRITE =", 1 << 6)
print("current READ =", 0x1)
print("current WRITE =", 0x2)
PY
printf '%s\n' '--- all st_filetype consumers ---'
rg -n -A4 -B4 'st_filetype|FILETYPE_DIRECTORY|FILETYPE_REGULAR_FILE' packages/compiler/src/bindings/rust/srcRepository: AndyTechnologies/wasm-apps
Length of output: 2810
Corrija el layout de __wasi_filestat_t.
WASI Preview1 requiere 64 bytes. Use st_filetype: u8, siete bytes de padding y st_nlink: u64 en el offset 24. El layout actual ocupa 56 bytes, por lo que path_filestat_get puede escribir fuera de buf y desplazar los metadatos siguientes. Actualice también stat_file para convertir st_filetype a u32.
Además, en las líneas 169-170, defina RIGHTS_READ como 1 << 1 y RIGHTS_WRITE como 1 << 6. Los valores actuales no solicitan FD_READ ni FD_WRITE, y las operaciones de lectura y escritura pueden fallar.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/bindings/rust/src/wasi.rs` around lines 60 - 69,
Corrija el layout de __wasi_filestat_t para cumplir los 64 bytes de WASI
Preview1: cambie st_filetype a u8, agregue siete bytes de padding y use
st_nlink: u64 en el offset 24. Actualice stat_file para convertir st_filetype a
u32. Ajuste también RIGHTS_READ a 1 << 1 y RIGHTS_WRITE a 1 << 6 para solicitar
los permisos correctos.
| const RIGHTS_READ: u64 = 0x0000000000000001; | ||
| const RIGHTS_WRITE: u64 = 0x0000000000000002; | ||
|
|
||
| pub fn path_open(dir_fd: i32, path: &[u8], for_write: bool, create: bool) -> Result<i32, WasiError> { | ||
| let mut oflags: u32 = 0; | ||
| let mut rights_base: u64 = RIGHTS_READ; | ||
| if for_write { | ||
| rights_base |= RIGHTS_WRITE; | ||
| } | ||
| if create { | ||
| oflags |= 0x01; // O_CREAT | ||
| rights_base |= RIGHTS_WRITE; | ||
| } | ||
| let mut opened_fd: i32 = -1; | ||
| let ret = unsafe { | ||
| __wasi_path_open(dir_fd, 0, path.as_ptr(), path.len(), oflags, | ||
| rights_base, rights_base, 0, &mut opened_fd) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
What are the WASI Preview 1 bit values for RIGHTS_FD_READandRIGHTS_FD_WRITEpassed topath_open?
💡 Result:
In WASI Preview 1, the rights are represented as a 64-bit unsigned integer (bitmask) where each bit corresponds to a specific capability. The bit values for the requested rights are as follows: * RIGHTS_FD_READ: 2 (which is 1 << 1) [1][2] * RIGHTS_FD_WRITE: 64 (which is 1 << 6) [3][4] When using path_open, these values (or their combinations) are typically passed in the fs_rights_base and fs_rights_inheriting fields to specify the capabilities granted to the newly opened file descriptor [5]. As per the WASI Preview 1 specification, RIGHTS_FD_READ grants the right to perform operations such as fd_read and sock_recv, while RIGHTS_FD_WRITE grants the right to perform operations such as fd_write and sock_send [6][7].
Citations:
- 1: https://livesplit.org/asr/wasi/constant.RIGHTS_FD_READ.html
- 2: https://wasi.bchlr.de/wasihost_core/wasi_snapshot_preview1/native/constant.rights_fd_read.html
- 3: https://livesplit.org/asr/wasi/constant.RIGHTS_FD_WRITE.html
- 4: https://wasi.bchlr.de/wasihost_core/wasi_snapshot_preview1/native/constant.rights_fd_write.html
- 5: https://docs.rs/wasip1/latest/wasip1/fn.path_open.html
- 6: https://github.com/WebAssembly/WASI/blob/wasi-0.1/preview1/docs.md
- 7: https://github.com/WebAssembly/WASI/blob/main/legacy/preview1/docs.md
🌐 Web query:
WASI Preview 1 RIGHTS_SYNC_FD bit 0 value 1 rights constants
💡 Result:
In WASI Preview 1, the constant RIGHTS_FD_SYNC is defined as a bitwise value of 1 shifted left by 4, which equals 16 (decimal) or 0x10 (hexadecimal) [1][2][3]. It is not associated with bit 0 or the value 1; bit 0 (value 1) is reserved for RIGHTS_FD_DATASYNC [4][3]. Summary of the constants mentioned: - RIGHTS_FD_DATASYNC: 1 << 0 (Value 1) [3] - RIGHTS_FD_SYNC: 1 << 4 (Value 16) [1][2][3] The RIGHTS_FD_SYNC constant represents the right to invoke the fd_sync function. Additionally, if the path_open right is set, this right includes the capability to invoke path_open with fdflags::rsync and fdflags::dsync [1][5][3].
Citations:
- 1: https://github.com/WebAssembly/WASI/blob/wasi-0.1/preview1/docs.md
- 2: https://github.com/WebAssembly/WASI/blob/main/legacy/preview1/docs.md
- 3: https://github.com/WebAssembly/wasi-libc/blob/2b7e73ae7ac0bad6391d89cc3274a28412243389/libc-bottom-half/headers/public/wasi/api.h
- 4: https://docs.rs/wasip1/latest/src/wasip1/lib_generated.rs.html
- 5: https://wasi.bchlr.de/wasihost_core/wasi_snapshot_preview1/native/constant.rights_fd_sync.html
Corrija los valores de derechos WASI en las constantes.
Los valores actuales son incorrectos según WASI Preview 1. El valor 0x01 corresponde a RIGHTS_FD_DATASYNC, no a lectura. El valor 0x02 corresponde a RIGHTS_FD_READ, no a escritura. Por tanto, path_open asigna permisos de sincronización y lectura en lugar de lectura y escritura.
Use 0x02 para RIGHTS_READ (bit 1) y 0x40 para RIGHTS_WRITE (bit 6):
Cambios propuestos
-const RIGHTS_READ: u64 = 0x0000000000000001;
-const RIGHTS_WRITE: u64 = 0x0000000000000002;
+const RIGHTS_READ: u64 = 0x0000000000000002;
+const RIGHTS_WRITE: u64 = 0x0000000000000040;
pub fn path_open(dir_fd: i32, path: &[u8], for_write: bool, create: bool) -> Result<i32, WasiError> {
let mut oflags: u32 = 0;
- let mut rights_base: u64 = RIGHTS_READ;
- if for_write {
- rights_base |= RIGHTS_WRITE;
- }
+ let mut rights_base = if for_write { RIGHTS_WRITE } else { RIGHTS_READ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const RIGHTS_READ: u64 = 0x0000000000000001; | |
| const RIGHTS_WRITE: u64 = 0x0000000000000002; | |
| pub fn path_open(dir_fd: i32, path: &[u8], for_write: bool, create: bool) -> Result<i32, WasiError> { | |
| let mut oflags: u32 = 0; | |
| let mut rights_base: u64 = RIGHTS_READ; | |
| if for_write { | |
| rights_base |= RIGHTS_WRITE; | |
| } | |
| if create { | |
| oflags |= 0x01; // O_CREAT | |
| rights_base |= RIGHTS_WRITE; | |
| } | |
| let mut opened_fd: i32 = -1; | |
| let ret = unsafe { | |
| __wasi_path_open(dir_fd, 0, path.as_ptr(), path.len(), oflags, | |
| rights_base, rights_base, 0, &mut opened_fd) | |
| const RIGHTS_READ: u64 = 0x0000000000000002; | |
| const RIGHTS_WRITE: u64 = 0x0000000000000040; | |
| pub fn path_open(dir_fd: i32, path: &[u8], for_write: bool, create: bool) -> Result<i32, WasiError> { | |
| let mut oflags: u32 = 0; | |
| let mut rights_base = if for_write { RIGHTS_WRITE } else { RIGHTS_READ }; | |
| if create { | |
| oflags |= 0x01; // O_CREAT | |
| rights_base |= RIGHTS_WRITE; | |
| } | |
| let mut opened_fd: i32 = -1; | |
| let ret = unsafe { | |
| __wasi_path_open(dir_fd, 0, path.as_ptr(), path.len(), oflags, | |
| rights_base, rights_base, 0, &mut opened_fd) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/bindings/rust/src/wasi.rs` around lines 169 - 185,
Corrige las constantes RIGHTS_READ y RIGHTS_WRITE usadas por path_open: asigna
0x02 a RIGHTS_READ y 0x40 a RIGHTS_WRITE según WASI Preview 1. Mantén sin
cambios la lógica que combina estos derechos en rights_base.
| typedef struct { | ||
| uint64_t st_dev; uint64_t st_ino; uint32_t st_filetype; | ||
| uint32_t st_nlink; uint64_t st_size; uint64_t st_atim; | ||
| uint64_t st_mtim; uint64_t st_ctim; | ||
| } __wasi_filestat_t; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
wasi_snapshot_preview1 filestat 64 bytes offsets filetype 16 nlink 24 size 32
💡 Result:
In the WASI wasi_snapshot_preview1 specification, the filestat structure is a 64-byte record used to store file attributes [1][2]. The memory layout of this structure is defined with the following field offsets [1][2]: - 0: dev (device ID, 8 bytes) [1] - 8: ino (inode number, 8 bytes) [1] - 16: filetype (file type, 1 byte, padded to 8 bytes) [3][1] - 24: nlink (link count, 8 bytes) [3][1] - 32: size (file size in bytes, 8 bytes) [3][1] - 40: atim (access timestamp, 8 bytes) [1] - 48: mtim (modification timestamp, 8 bytes) [1] - 56: ctim (status change timestamp, 8 bytes) [1] The filetype field is specifically located at offset 16 [3][2], followed by the nlink field at offset 24 [3][2], and the size field at offset 32 [3][2]. The structure is designed for 8-byte alignment, which leads to padding after the 1-byte filetype field [3]. Explicit static assertions in the WASI libc implementation confirm these offsets and the total size of 64 bytes [2].
Citations:
- 1: https://docs.rs/krasm/latest/src/krasm/runtime/wasi/types.rs.html
- 2: https://github.com/WebAssembly/wasi-libc/blob/2b7e73ae7ac0bad6391d89cc3274a28412243389/libc-bottom-half/headers/public/wasi/api.h
- 3: https://github.com/WebAssembly/WASI/blob/wasi-0.1/preview1/docs.md
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files \
packages/compiler/src/bindings/wasi.h \
packages/linker/templates/wasi.h \
packages/compiler/src/bindings/wasi.rs \
packages/linker/templates/wasi.rs \
packages/compiler/src/bindings/wasi.ts \
packages/linker/templates/wasi.as.ts
printf '%s\n' '--- C layouts and call sites ---'
for f in packages/compiler/src/bindings/wasi.h packages/linker/templates/wasi.h; do
echo "### $f"
sed -n '25,42p;100,128p' "$f"
rg -n -C 4 '__wasi_filestat_t|path_filestat_get|filestat' "$f"
done
printf '%s\n' '--- Rust layouts and stat_file ---'
for f in packages/compiler/src/bindings/wasi.rs packages/linker/templates/wasi.rs; do
echo "### $f"
sed -n '48,72p;195,220p' "$f"
rg -n -C 5 '__wasi_filestat_t|stat_file|path_filestat_get|filestat' "$f"
done
printf '%s\n' '--- AssemblyScript layouts and call sites ---'
for f in packages/compiler/src/bindings/wasi.ts packages/linker/templates/wasi.as.ts; do
echo "### $f"
sed -n '145,188p' "$f"
rg -n -C 4 'pathFilestatGet|wasiPathFilestatGet|Filestat' "$f"
done
printf '%s\n' '--- copy relationship ---'
cmp -s packages/compiler/src/bindings/wasi.h packages/linker/templates/wasi.h && echo 'wasi.h copies identical' || echo 'wasi.h copies differ'
cmp -s packages/compiler/src/bindings/wasi.rs packages/linker/templates/wasi.rs && echo 'wasi.rs copies identical' || echo 'wasi.rs copies differ'
cmp -s packages/compiler/src/bindings/wasi.ts packages/linker/templates/wasi.as.ts && echo 'AssemblyScript copies identical' || echo 'AssemblyScript copies differ'Repository: AndyTechnologies/wasm-apps
Length of output: 25280
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import ctypes
class Current(ctypes.Structure):
_fields_ = [
("st_dev", ctypes.c_uint64),
("st_ino", ctypes.c_uint64),
("st_filetype", ctypes.c_uint32),
("st_nlink", ctypes.c_uint32),
("st_size", ctypes.c_uint64),
("st_atim", ctypes.c_uint64),
("st_mtim", ctypes.c_uint64),
("st_ctim", ctypes.c_uint64),
]
class Preview1(ctypes.Structure):
_fields_ = [
("st_dev", ctypes.c_uint64),
("st_ino", ctypes.c_uint64),
("st_filetype", ctypes.c_uint8),
("_padding", ctypes.c_uint8 * 7),
("st_nlink", ctypes.c_uint64),
("st_size", ctypes.c_uint64),
("st_atim", ctypes.c_uint64),
("st_mtim", ctypes.c_uint64),
("st_ctim", ctypes.c_uint64),
]
for cls in (Current, Preview1):
print(cls.__name__, ctypes.sizeof(cls))
for name, _ in cls._fields_:
print(name, getattr(cls, name).offset)
PY
printf '%s\n' '--- Rust stat_file definitions and uses ---'
rg -n -C 8 '__wasi_filestat_t|stat_file' packages/compiler/src/bindings/wasi.rs packages/linker/templates/wasi.rs
printf '%s\n' '--- all filestat buffer allocations and loads ---'
rg -n -C 5 'alloc\(56\)|alloc\(64\)|st(Filetype|Nlink|Size|Atim|Mtim|Ctim)|load<i(32|64)>' \
packages/compiler/src/bindings/wasi.ts packages/linker/templates/wasi.as.tsRepository: AndyTechnologies/wasm-apps
Length of output: 28640
Align all six filestat bindings with the 64-byte WASI layout.
wasi_snapshot_preview1 uses filetype at offset 16 as u8, seven padding bytes, nlink at offset 24 as u64, and size at offset 32. The current C and Rust structs and AssemblyScript buffers are 56 bytes, causing an 8-byte overwrite and incorrect field decoding.
Update:
packages/compiler/src/bindings/wasi.hpackages/linker/templates/wasi.hpackages/compiler/src/bindings/wasi.rspackages/linker/templates/wasi.rspackages/compiler/src/bindings/wasi.tspackages/linker/templates/wasi.as.ts
Use explicit padding in C and Rust. Allocate 64 bytes in AssemblyScript, load filetype as u8, load nlink as i64, and shift subsequent offsets by 8 bytes. Update Rust stat_file to convert the new u8 field to its u32 return type.
📍 Affects 6 files
packages/compiler/src/bindings/wasi.h#L32-L36(this comment)packages/linker/templates/wasi.h#L32-L36packages/compiler/src/bindings/wasi.rs#L55-L65packages/linker/templates/wasi.rs#L55-L65packages/compiler/src/bindings/wasi.ts#L168-L182packages/linker/templates/wasi.as.ts#L168-L182
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/bindings/wasi.h` around lines 32 - 36, Align the
__wasi_filestat_t definitions in packages/compiler/src/bindings/wasi.h (32-36)
and packages/linker/templates/wasi.h (32-36) to the 64-byte WASI layout using
explicit padding. Update the corresponding Filestat structs in
packages/compiler/src/bindings/wasi.rs (55-65) and
packages/linker/templates/wasi.rs (55-65) with u8 filetype, padding, and u64
nlink, and convert filetype to u32 in stat_file. In
packages/compiler/src/bindings/wasi.ts (168-182) and
packages/linker/templates/wasi.as.ts (168-182), allocate 64 bytes, load filetype
as u8 and nlink as i64, and shift later field offsets by 8 bytes.
| inline int path_open(int dir_fd, const char *path, size_t path_len, int oflags) { | ||
| uint32_t flags = 0; | ||
| uint64_t rights_base = 0; | ||
| if (oflags & 0x01) { rights_base |= 0x0000000000000001ULL; } // O_READ -> rights only (WASI flag 0x01 is O_CREAT) | ||
| if (oflags & 0x02) { rights_base |= 0x0000000000000002ULL; } // O_WRITE -> rights only (WASI flag 0x02 is O_DIRECTORY) | ||
| if (oflags & 0x40) { flags |= 0x01; rights_base |= 0x0000000000000004ULL; } // O_CREAT -> WASI O_CREAT | ||
| if (oflags & 0x200) { flags |= 0x08; } // O_TRUNC -> WASI O_TRUNC | ||
| int32_t opened_fd = -1; | ||
| int err = __wasi_path_open(dir_fd, 0, path, path_len, flags, | ||
| rights_base, rights_base, 0, &opened_fd); | ||
| return err ? -err : (int)opened_fd; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
WASI preview1 rights bitflags fd_read fd_write path_create_file bit positions
💡 Result:
In WASI preview1, the rights are represented as a 64-bit flag field where each bit corresponds to a specific capability [1]. The bit positions for the requested rights are as follows: fd_read: Bit 1 (value 1 << 1) [2][1] fd_write: Bit 6 (value 1 << 6) [2][1] path_create_file: Bit 10 (value 1 << 10) [2][1] For context, fd_read grants the right to invoke functions like fd_read and sock_recv [2], while fd_write grants the right to invoke fd_write and sock_send [2]. The path_create_file right allows a file descriptor to be used to invoke path_open with the oflags::creat flag [2]. These bits are part of a larger set of rights defined in the WASI preview1 specification [3][2].
Citations:
- 1: https://docs.rs/krasm/latest/src/krasm/runtime/wasi/types.rs.html
- 2: https://github.com/WebAssembly/WASI/blob/main/legacy/preview1/docs.md
- 3: https://github.com/WebAssembly/WASI/blob/wasi-0.1/preview1/docs.md
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
for f in packages/compiler/src/bindings/wasi.h packages/linker/templates/wasi.h \
packages/compiler/src/bindings/wasi.rs packages/linker/templates/wasi.rs \
packages/compiler/src/bindings/wasi.ts packages/linker/templates/wasi.as.ts; do
printf '\n--- %s ---\n' "$f"
sed -n '90,205p' "$f"
done
printf '\n--- related WASI constants and path_open call sites ---\n'
rg -n --glob '!node_modules' --glob '!dist' --glob '!build' \
'RIGHTS_|path_open|pathOpen|fd_read|fd_write|path_create_file|O_CREAT|O_TRUNC' packages | head -300Repository: AndyTechnologies/wasm-apps
Length of output: 39983
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- repository metadata and tests ---'
git ls-files | rg '(^|/)(wasi|fs|filesystem|preview1|wasm)' | head -300
printf '\n--- all path_open definitions and callers ---\n'
rg -n --glob '!node_modules' --glob '!dist' --glob '!build' \
'\b(path_open|pathOpen)\b' . | head -400
printf '\n--- generated-source relationship clues ---\n'
rg -n --glob '!node_modules' --glob '!dist' --glob '!build' \
'packages/linker/templates|bindings/wasi|generate.*template|template' package.json packages scripts .github 2>/dev/null | head -250Repository: AndyTechnologies/wasm-apps
Length of output: 13086
🌐 Web query:
WASI preview1 path_open fs_rights_base fs_rights_inheriting path_create_file required rights specification
💡 Result:
In WASI preview1 (wasi_snapshot_preview1), the path_open function requires explicit specification of fs_rights_base and fs_rights_inheriting to define the capabilities of the newly created file descriptor [1][2]. Rights Specification: 1. fs_rights_base: These represent the rights that apply directly to the file descriptor being created [1][3]. The implementation may return a descriptor with fewer rights than requested if those rights are not applicable to the type of file being opened, but it should not generally grant more rights than requested [1][4]. 2. fs_rights_inheriting: These define the rights that will apply to any file descriptors derived from the newly created one (e.g., if the opened file is a directory and subsequent files are opened within it) [1][3][4]. Specific Right Dependencies: - path_create_file: To successfully use oflags::creat (which creates a file if it does not exist) within path_open, the caller must possess the RIGHTS_PATH_CREATE_FILE right [5][6]. - path_open: To invoke the path_open function itself, the caller must possess the RIGHTS_PATH_OPEN right on the parent directory file descriptor [7][5][6]. Key Nuances: - The parent directory file descriptor must have these rights (or allow them to be inherited) for the operation to succeed [3][4]. - While the specification for these rights is central to WASI preview1's capability-based security model, implementation behaviors—particularly regarding what rights are granted by default—have historically varied across different runtimes [3][4]. - These rights mechanisms are specific to preview1 and were significantly redesigned in later versions of the WASI filesystem proposals [4][8].
Citations:
- 1: https://docs.rs/wasip1/latest/wasip1/fn.path_open.html
- 2: https://go.dev/src/syscall/fs_wasip1.go
- 3: How does
fs_rights_inheritingwork inpath_open? WebAssembly/wasi-filesystem#23 - 4: Semantics of path_open WebAssembly/wasi-filesystem#8
- 5: https://github.com/WebAssembly/WASI/blob/wasi-0.1/preview1/docs.md
- 6: https://wasi.bchlr.de/wasihost_core/wasi_snapshot_preview1/struct.Rights.html
- 7: https://docs.rs/wasip1/latest/wasip1/constant.RIGHTS_PATH_OPEN.html
- 8: https://github.com/WebAssembly/WASI/tree/main/proposals/filesystem
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
files = [
Path("packages/compiler/src/bindings/wasi.h"),
Path("packages/linker/templates/wasi.h"),
Path("packages/compiler/src/bindings/wasi.rs"),
Path("packages/linker/templates/wasi.rs"),
Path("packages/compiler/src/bindings/wasi.ts"),
Path("packages/linker/templates/wasi.as.ts"),
]
for p in files:
text = p.read_text()
print(f"{p}:")
for i, line in enumerate(text.splitlines(), 1):
if "path_open" in line or "pathOpen" in line or "RIGHTS_" in line or "wasiPathOpen" in line:
print(f" {i}: {line}")
PY
printf '\n--- filesystem wrapper call context ---\n'
rg -n -C 5 --glob '!node_modules' --glob '!dist' --glob '!build' \
'pathOpen|path_open|forWrite|create' packages/compiler packages/linker/templates | head -500Repository: AndyTechnologies/wasm-apps
Length of output: 43722
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- exact six wrapper bodies ---'
python3 - <<'PY'
from pathlib import Path
for p in [
Path("packages/compiler/src/bindings/wasi.h"),
Path("packages/linker/templates/wasi.h"),
Path("packages/compiler/src/bindings/wasi.rs"),
Path("packages/linker/templates/wasi.rs"),
Path("packages/compiler/src/bindings/wasi.ts"),
Path("packages/linker/templates/wasi.as.ts"),
]:
lines = p.read_text().splitlines()
hits = [i for i, x in enumerate(lines) if "path_open" in x or "pathOpen" in x]
print(f"\n{p}")
if hits:
lo, hi = max(0, hits[0]-12), min(len(lines), hits[0]+30)
for n in range(lo, hi):
print(f"{n+1}: {lines[n]}")
PY
printf '\n--- WASI imports/declarations surrounding the ABI ---\n'
rg -n -C 8 --glob '!node_modules' --glob '!dist' --glob '!build' \
'__wasi_path_open|wasiPathOpen|fd_read|fd_write|path_create_file' \
packages/compiler/src/bindings packages/linker/templates | head -500Repository: AndyTechnologies/wasm-apps
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- spec references available in repository ---'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'fd_datasync|fd_read|fd_seek|fd_write|path_create_file|path_open' . | head -500
printf '\n--- direct equality/relationship between source and template copies ---\n'
python3 - <<'PY'
from pathlib import Path
pairs = [
("packages/compiler/src/bindings/wasi.h", "packages/linker/templates/wasi.h"),
("packages/compiler/src/bindings/wasi.rs", "packages/linker/templates/wasi.rs"),
("packages/compiler/src/bindings/wasi.ts", "packages/linker/templates/wasi.as.ts"),
]
for a, b in pairs:
x, y = Path(a).read_text(), Path(b).read_text()
print(f"{a} == {b}: {x == y}")
PYRepository: AndyTechnologies/wasm-apps
Length of output: 16704
🌐 Web query:
site:github.com/WebAssembly/WASI legacy preview1 docs.md path_open fs_rights_base path_create_file fd_read
💡 Result:
The terms referenced (legacy_preview1, path_open, fs_rights_base, path_create_file, and fd_read) relate to the WASI (WebAssembly System Interface) snapshot_preview1 specification [1]. WASI snapshot_preview1 (often referred to as wasi_snapshot_preview1 or simply preview1) is a legacy interface definition for WASI, now maintained under the legacy/ directory in the WebAssembly/WASI repository [2][1]. It was defined using the witx (WebAssembly Interface Type eXperimental) language [3][1]. Key components referenced in your query: 1. path_open: This is a function in the WASI preview1 API, similar to the POSIX openat function [3][4]. It is used to open files or directories relative to a provided base file descriptor [3][5]. 2. fs_rights_base: This field is part of the file descriptor status (fdstat) structure [2][6]. It defines the base rights that apply to operations performed using the file descriptor itself, as opposed to fs_rights_inheriting which applies to file descriptors derived from it [3][4]. 3. path_create_file: This is a specific right within the rights record [2][6]. Specifically, having this right set is required to invoke path_open with the oflags::creat flag to create new files [2][6]. 4. fd_read: This is also a right within the rights record [2][6]. It grants permission to invoke functions like fd_read (for reading from a file descriptor) and sock_recv [2][6]. If rights::fd_seek is also set, it additionally enables the fd_pread function [2]. These rights are part of WASI's capability-based security model, where access to files and resources is governed by the specific rights granted to a file descriptor rather than by broad filesystem permissions [5].
Citations:
- 1: https://github.com/WebAssembly/WASI/blob/main/legacy/README.md
- 2: https://github.com/WebAssembly/WASI/blob/main/legacy/preview1/docs.md
- 3: https://github.com/WebAssembly/WASI/blob/01bb90d8b66cbc1d50349aaaab9ac5b143c9c98c/legacy/preview1/witx/wasi_snapshot_preview1.witx
- 4: https://github.com/WebAssembly/WASI/blob/a206794fea66118945a520f6e0af3754cc51860b/phases/snapshot/witx/wasi_snapshot_preview1.witx
- 5: What's the intended semantics of
path_openwith absolute path? WebAssembly/WASI#374 - 6: https://github.com/WebAssembly/WASI/blob/wasi-0.1/preview1/docs.md
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
# Read-only behavioral model of the six shown flag conversions.
cases = [
("read", False, False),
("write", True, False),
("read-create", False, True),
("write-create", True, True),
]
for name, for_write, create in cases:
c_oflags = (0x01 if not for_write else 0) | (0x40 if create else 0)
c_rights = (0x01 if not for_write else 0) | (0x02 if for_write else 0)
rust_ts_oflags = 0x01 if create else 0
rust_ts_rights = 0x01 | (0x02 if for_write or create else 0)
print(name, {
"c_oflags": hex(c_oflags),
"c_rights": hex(c_rights),
"rust_ts_oflags": hex(rust_ts_oflags),
"rust_ts_rights": hex(rust_ts_rights),
})
PY
printf '\n--- file operation wrappers and expected access mode ---\n'
rg -n -C 4 --glob '!node_modules' --glob '!dist' --glob '!build' \
'pathOpen\(|path_open\(|fdRead|fd_read|fdWrite|fd_write|open' \
packages/compiler/src packages/linker/templates | head -700Repository: AndyTechnologies/wasm-apps
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- git status and tracked file sizes ---'
git status --short
wc -l packages/compiler/src/bindings/wasi.h packages/linker/templates/wasi.h \
packages/compiler/src/bindings/wasi.rs packages/linker/templates/wasi.rs \
packages/compiler/src/bindings/wasi.ts packages/linker/templates/wasi.as.ts
printf '\n--- package-level references to generated bindings ---\n'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'wasi\.(h|rs|ts|as\.ts)|bindings' packages/compiler packages/linker package.json | head -400Repository: AndyTechnologies/wasm-apps
Length of output: 11095
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
expected = {
"fd_read": 1 << 1,
"fd_write": 1 << 6,
"path_create_file": 1 << 10,
}
files = [
Path("packages/compiler/src/bindings/wasi.h"),
Path("packages/linker/templates/wasi.h"),
Path("packages/compiler/src/bindings/wasi.rs"),
Path("packages/linker/templates/wasi.rs"),
Path("packages/compiler/src/bindings/wasi.ts"),
Path("packages/linker/templates/wasi.as.ts"),
Path("packages/compiler/src/bindings/rust/src/wasi.rs"),
]
for p in files:
text = p.read_text()
values = [int(x, 0) for x in re.findall(r"(?:RIGHTS_READ|rights_base\s*\|=|rightsBase\s*\|=|RIGHTS_WRITE)[^;\n]*?(0x[0-9A-Fa-f]+|\d+\s*<<\s*\d+)", text)]
print(f"{p}: values={values}")
if "RIGHTS_READ" in text:
for name in ("RIGHTS_READ", "RIGHTS_WRITE"):
m = re.search(rf"{name}\s*:\s*u?64\s*=\s*(0x[0-9A-Fa-f]+|\d+\s*<<\s*\d+)", text)
print(f" {name}={m.group(1) if m else 'missing'}")
if "path_create_file" in text.lower():
print(" contains path_create_file")
PYRepository: AndyTechnologies/wasm-apps
Length of output: 834
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from urllib.request import urlopen
url = "https://raw.githubusercontent.com/WebAssembly/WASI/main/legacy/preview1/docs.md"
text = urlopen(url, timeout=10).read().decode()
needles = ["path_create_file", "fd_read", "fd_write", "path_open", "fs_rights_base"]
for needle in needles:
print(f"\n--- {needle} ---")
lines = text.splitlines()
hits = [i for i, line in enumerate(lines) if needle in line]
for i in hits[:5]:
for n in range(max(0, i-3), min(len(lines), i+8)):
print(f"{n+1}: {lines[n]}")
PYRepository: AndyTechnologies/wasm-apps
Length of output: 2450
Correct the WASI rights bits in all seven path_open wrappers.
Use fd_read (1 << 1) and fd_write (1 << 6). The current values grant fd_datasync and fd_read, so fd_read and fd_write fail with ENOTCAPABLE on rights-enforcing runtimes. Remove the C++ 0x04 mapping. Keep O_CREAT as oflags = 0x01; path_create_file (1 << 10) must be available on the parent dir_fd, not in the returned descriptor's fs_rights_base.
Apply this to the six listed files and packages/compiler/src/bindings/rust/src/wasi.rs.
📍 Affects 6 files
packages/compiler/src/bindings/wasi.h#L110-L121(this comment)packages/linker/templates/wasi.h#L110-L121packages/compiler/src/bindings/wasi.rs#L165-L185packages/linker/templates/wasi.rs#L165-L185packages/compiler/src/bindings/wasi.ts#L150-L166packages/linker/templates/wasi.as.ts#L150-L166
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/bindings/wasi.h` around lines 110 - 121, Update the
path_open wrappers in packages/compiler/src/bindings/wasi.h (110-121),
packages/linker/templates/wasi.h (110-121),
packages/compiler/src/bindings/wasi.rs (165-185),
packages/linker/templates/wasi.rs (165-185),
packages/compiler/src/bindings/wasi.ts (150-166),
packages/linker/templates/wasi.as.ts (150-166), and
packages/compiler/src/bindings/rust/src/wasi.rs to map fd_read to 1 << 1 and
fd_write to 1 << 6. Remove the C++ 0x04 rights mapping, preserve O_CREAT as
oflags 0x01, and ensure path_create_file (1 << 10) is provided on the parent
dir_fd rather than the returned descriptor’s fs_rights_base.
| function makeIovec(data: usize, len: i32): usize { | ||
| let buf = heap.alloc(8); | ||
| store<usize>(buf, data, 0); | ||
| store<i32>(buf, len, 4); | ||
| return buf; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Both AssemblyScript WASI wrappers leak every heap.alloc block. The shared root cause is that makeIovec and each result cell allocate memory and no code path calls heap.free. Every stdoutWrite, fdWrite, fdRead, fdSeek, fdPrestatGet, pathOpen, pathFilestatGet, environSizesGet, and clockTime call leaks 4 to 8 bytes, and fs path resolution multiplies the leak by the descriptor scan.
packages/compiler/src/bindings/wasi.ts#L69-L74: free the iovec and each result cell before returning, or replace them with module-level scratch buffers.packages/linker/templates/wasi.as.ts#L69-L74: apply the same change in this generated copy.
📍 Affects 2 files
packages/compiler/src/bindings/wasi.ts#L69-L74(this comment)packages/linker/templates/wasi.as.ts#L69-L74
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/bindings/wasi.ts` around lines 69 - 74, The WASI
wrappers leak memory allocated by makeIovec and their result cells. Update
packages/compiler/src/bindings/wasi.ts lines 69-74 and
packages/linker/templates/wasi.as.ts lines 69-74 consistently, freeing each
allocated iovec and result cell before returning or replacing them with reusable
module-level scratch buffers; ensure stdoutWrite, fdWrite, fdRead, fdSeek,
fdPrestatGet, pathOpen, pathFilestatGet, environSizesGet, and clockTime no
longer retain allocations.
| // Existing manifest: backup bytes (Cargo.toml + Cargo.lock si existen) → inyectar (D3) | ||
| let backupToml: Buffer | null = null; | ||
| let backupLock: Buffer | null = null; | ||
| if (hasCargoToml) { | ||
| backupToml = fs.readFileSync(cargoTomlPath); | ||
| if (fs.existsSync(cargoLockPath)) { | ||
| backupLock = fs.readFileSync(cargoLockPath); | ||
| } | ||
| } | ||
|
|
||
| try { | ||
| if (hasCargoToml) { | ||
| // Inyectar la dep del crate vendido (REQ-3). Dentro del try: si el write | ||
| // falla (ENOSPC/EACCES), el finally restaura el manifest original (D3). | ||
| const injected = injectBindingsDependency(backupToml!.toString('utf-8'), BINDINGS_RUST_DIR, isWin32); | ||
| fs.writeFileSync(cargoTomlPath, injected, 'utf-8'); | ||
| } else { | ||
| // No Cargo.toml: crear uno temporal con cdylib + dep del crate vendido (REQ-2) | ||
| const tempCargoContent = this.generateTempCargoToml(options.fileName, BINDINGS_RUST_DIR, isWin32); | ||
| fs.writeFileSync(cargoTomlPath, tempCargoContent, 'utf-8'); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Look for concurrent or batched invocations of the toolchain strategies.
rg -nP --type=ts -C4 '\b(compile|compileFile)\s*\(' packages/compiler/src | rg -n -C4 'Promise\.all|Promise\.allSettled|map\(|for await'
rg -nP --type=ts -C4 'strategy\.compile\(' packagesRepository: AndyTechnologies/wasm-apps
Length of output: 24055
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
fd -t f 'rust-strategy|compiler|toolchain-router|index' packages | head -80
printf '%s\n' '--- rust strategy outline ---'
ast-grep outline packages/compiler/src/strategies/rust-strategy.ts
printf '%s\n' '--- rust strategy relevant source ---'
cat -n packages/compiler/src/strategies/rust-strategy.ts | sed -n '1,240p'
printf '%s\n' '--- compile call sites and concurrency constructs ---'
rg -n -P --type=ts -C3 '\b(strategy\.)?compile\s*\(|Promise\.(all|allSettled)|\.map\s*\(' packages --glob '!**/*.test.ts'
printf '%s\n' '--- exported compiler API ---'
rg -n -P --type=ts -C4 'ToolchainRouter|compileFile|compile\(' packages/compiler/src packages/cli/src --glob '!**/*.test.ts'Repository: AndyTechnologies/wasm-apps
Length of output: 40293
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- compiler CLI batching and per-file compilation ---'
cat -n packages/compiler/src/cli.ts | sed -n '95,145p'
cat -n packages/compiler/src/cli.ts | sed -n '1,95p'
printf '%s\n' '--- package CLI project compilation ---'
cat -n packages/cli/src/index.ts | sed -n '125,225p'
printf '%s\n' '--- router construction and strategy sharing ---'
cat -n packages/compiler/src/toolchain-router.ts | sed -n '1,125p'
printf '%s\n' '--- Rust parallelism tests or documented constraints ---'
rg -n -P -C3 'parallel|same directory|same source|concurr|Cargo\.toml|Cargo\.lock' packages/compiler packages/cli README.md docs 2>/dev/null || trueRepository: AndyTechnologies/wasm-apps
Length of output: 41292
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
from itertools import permutations
# Model the synchronous file operations and the async cargo interval for two
# overlapping compile() calls. Each call backs up Cargo.toml/Cargo.lock,
# writes the injected manifest, waits for cargo, then restores in finally.
def run(schedule, initial_toml="ORIGINAL", initial_lock=None):
files = {"Cargo.toml": initial_toml}
if initial_lock is not None:
files["Cargo.lock"] = initial_lock
state = {
"A": {"backup_toml": None, "backup_lock": None},
"B": {"backup_toml": None, "backup_lock": None},
}
events = {
"A": ["backup", "inject", "cargo_done", "restore"],
"B": ["backup", "inject", "cargo_done", "restore"],
}
seen = {"A": 0, "B": 0}
for who in schedule:
event = events[who][seen[who]]
seen[who] += 1
if event == "backup":
state[who]["backup_toml"] = files.get("Cargo.toml")
state[who]["backup_lock"] = files.get("Cargo.lock")
elif event == "inject":
files["Cargo.toml"] = "INJECTED"
elif event == "cargo_done":
# Cargo creates a lockfile when none exists.
files.setdefault("Cargo.lock", "GENERATED")
elif event == "restore":
files["Cargo.toml"] = state[who]["backup_toml"]
if state[who]["backup_lock"] is None:
files.pop("Cargo.lock", None)
else:
files["Cargo.lock"] = state[who]["backup_lock"]
return files
# Only schedules that preserve each call's operation order are valid.
def valid(schedule):
positions = {"A": [], "B": []}
for i, who in enumerate(schedule):
positions[who].append(i)
return all(a < b for a, b in zip(positions["A"], positions["A"][1:])) and \
all(a < b for a, b in zip(positions["B"], positions["B"][1:]))
for initial_lock in (None, "ORIGINAL_LOCK"):
bad = []
for schedule in permutations("AABBCCDDEE".replace("C", "").replace("D", "").replace("E", "")):
# The compact permutation above is not used; enumerate all six events
# with each call's four ordered operations.
pass
schedules = []
def interleave(a, b, prefix=()):
if not a and not b:
schedules.append(prefix)
if a:
interleave(a[1:], b, prefix + (a[0],))
if b:
interleave(a, b[1:], prefix + (b[0],))
# Labels: backup, inject, cargo_done, restore.
interleave(tuple("AAAA"), tuple("BBBB"))
# Distinguish the event index while retaining the actor for the model.
outcomes = [(s, run(s, initial_lock=initial_lock)) for s in schedules]
bad = [(s, f) for s, f in outcomes
if f.get("Cargo.toml") != "ORIGINAL" or
(initial_lock is None and "Cargo.lock" in f)]
print("initial Cargo.lock:", "present" if initial_lock else "absent")
print("valid interleavings:", len(outcomes))
print("bad final states:", len(bad))
print("example:", bad[0] if bad else None)
PYRepository: AndyTechnologies/wasm-apps
Length of output: 527
Serialize Rust compilations per source directory.
RustCompilerStrategy.compile() edits Cargo.toml and awaits cargo, so overlapping calls can restore an injected manifest or remove another call's Cargo.lock. Add a per-directory mutex or compile in an isolated temporary workspace.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 128-128: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(cargoTomlPath)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 130-130: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(cargoLockPath)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 139-139: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(cargoTomlPath, injected, 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 143-143: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(cargoTomlPath, tempCargoContent, 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/strategies/rust-strategy.ts` around lines 125 - 145,
Serialize Rust compilations per source directory in
RustCompilerStrategy.compile(), covering the Cargo.toml injection, cargo
invocation, and restoration/cleanup flow as one critical section. Add or reuse a
directory-keyed mutex so concurrent compilations for the same source directory
cannot overwrite manifests or remove another compilation’s Cargo.lock, while
allowing different directories to compile concurrently.
| // Read in chunks up to 64KB | ||
| let mut buf = vec![0u8; 65536]; | ||
| let n = wasi::fd_read(fd, &mut buf)?; | ||
| let _ = wasi::fd_close(fd); | ||
| buf.truncate(n); | ||
| Ok(buf) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fixed 64 KiB single-read buffer truncates larger files in both readFile implementations. Both language bindings hardcode a 65536-byte buffer and perform exactly one read syscall, so files larger than 64 KiB are silently truncated and short reads return incomplete data.
packages/linker/templates/fs.rs#L56-L61: loopwasi::fd_readuntil it returns 0 and growbuf, then truncate to the total byte count.packages/linker/templates/fs.as.ts#L79-L89: loopfdReaduntil it returns 0, accumulate into a growable buffer, and copy the total byte count into the resultArrayBuffer.
📍 Affects 2 files
packages/linker/templates/fs.rs#L56-L61(this comment)packages/linker/templates/fs.as.ts#L79-L89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/linker/templates/fs.rs` around lines 56 - 61, Update both readFile
implementations to read complete files instead of performing one fixed 64 KiB
read: in packages/linker/templates/fs.rs lines 56-61, loop wasi::fd_read until
it returns 0, growing buf and truncating to the total byte count; in
packages/linker/templates/fs.as.ts lines 79-89, loop fdRead until it returns 0,
accumulate the chunks in a growable buffer, and copy the total bytes into the
returned ArrayBuffer.
| // Read in chunks up to 64KB | ||
| let mut buf = vec![0u8; 65536]; | ||
| let n = wasi::fd_read(fd, &mut buf)?; | ||
| let _ = wasi::fd_close(fd); | ||
| buf.truncate(n); | ||
| Ok(buf) | ||
| } | ||
|
|
||
| pub fn read_file_to_buf(path: &[u8], buf: &mut [u8]) -> Result<usize, i32> { | ||
| let r = resolve_path(path).ok_or(-1)?; | ||
| let fd = wasi::path_open(r.dir_fd, &r.relative, false, false)?; | ||
| let n = wasi::fd_read(fd, buf)?; | ||
| let _ = wasi::fd_close(fd); | ||
| Ok(n) | ||
| } | ||
|
|
||
| pub fn write_file(path: &[u8], data: &[u8]) -> Result<usize, i32> { | ||
| let r = resolve_path(path).ok_or(-1)?; | ||
| let fd = wasi::path_open(r.dir_fd, &r.relative, true, true)?; | ||
| let n = wasi::fd_write(fd, data)?; | ||
| let _ = wasi::fd_close(fd); | ||
| Ok(n) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Close the file descriptor on the error path.
In read_file, read_file_to_buf and write_file, ? on fd_read or fd_write returns before fd_close. Each failed read or write then leaks a descriptor. Capture the result, close the descriptor, and return the error afterwards.
🛡️ Proposed fix for `read_file`
- let mut buf = vec![0u8; 65536];
- let n = wasi::fd_read(fd, &mut buf)?;
- let _ = wasi::fd_close(fd);
- buf.truncate(n);
- Ok(buf)
+ let mut buf = vec![0u8; 65536];
+ let read = wasi::fd_read(fd, &mut buf);
+ let _ = wasi::fd_close(fd);
+ let n = read?;
+ buf.truncate(n);
+ Ok(buf)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/linker/templates/fs.rs` around lines 56 - 78, Update read_file,
read_file_to_buf, and write_file to capture the fd_read or fd_write result
before propagating it, always call fd_close after the operation, then return the
captured result so both success and error paths close the descriptor.
…65cece4 📝 Add docstrings to `feat/web-console-fs-api-examples`
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/compiler/src/index.ts (1)
144-168: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftCopying only the entry file to
tmpDirbreaks sibling relative imports.Any source that imports
console,fsorwasinow compiles fromtmpDirinstead of its real path. Only that one file is copied. A source that also containsfrom "./math"then fails, because./mathdoes not exist in the temp tree. The comment at lines 136-138 states that compiling from the original path is what keeps relative imports resolvable.A second defect follows from line 154: if
resolvedFileNamelies outsidePROJECT_ROOT,path.relativereturns a..-prefixed path, sosrcFileescapestmpDirand thefinallycleanup at line 233 never removes it.Prefer writing the rewritten source next to the original file under a temporary name, or copy the whole source directory into
tmpDir.Run the following script to check whether any example or test source mixes binding imports with relative imports:
#!/bin/bash # Description: Find AS sources that import bindings and also use relative imports. rg -l --glob '*.wasm.ts' -e "from ['\"](console|fs|wasi)['\"]" . | while IFS= read -r f; do rg -n -e "from ['\"]\./" -e "from ['\"](console|fs|wasi)['\"]" "$f" | sed "s|^|$f:|" done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/index.ts` around lines 144 - 168, The temporary compilation path must preserve sibling relative imports and remain inside tmpDir. Update the source handling around rewriteBindingImports so rewritten files are placed next to the original under a safe temporary name or the complete source directory is copied, and ensure resolvedFileName values outside PROJECT_ROOT cannot produce ..-prefixed paths escaping tmpDir; retain cleanupTmpDir behavior for temporary artifacts.
🟡 Minor comments (12)
.prettierignore-12-15 (1)
12-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winResolve the remaining formatter failure before merge.
These patterns ignore compiler source and distribution files only. The PR objectives state that
pnpm checkstill fails on AssemblyScript linker templates. Format those templates instead of leaving the check red. Do not broaden this rule to hide unrelated files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.prettierignore around lines 12 - 15, Update the AssemblyScript linker template files referenced by the compiler formatting check so they are validly formatted and pnpm check passes. Remove or narrow the .prettierignore entries for packages/compiler/src/bindings/*.ts and packages/compiler/dist/bindings/*.ts, and do not add broader ignore patterns.openspec/changes/bindings-auto-injection/tasks.md-65-65 (1)
65-65: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDo not mark integration verification complete before it runs.
Task 5.3 claims that all 12 examples build and run. The PR objectives state that integration tests and manual validation are still pending. Keep this task unchecked until
node scripts/test-examples.mjscompletes and the result is recorded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/bindings-auto-injection/tasks.md` at line 65, Update task 5.3 in the checklist to remain unchecked until node scripts/test-examples.mjs has completed successfully; do not claim all 12 examples build and run or mark integration verification complete before recording the actual result.openspec/changes/bindings-auto-injection/design.md-37-37 (1)
37-37: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSpecify the fenced-code language.
Use
textfor the ASCII data-flow block somarkdownlintcan validate it.Suggested change
-``` +```text🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/bindings-auto-injection/design.md` at line 37, Specify the fenced-code language for the ASCII data-flow block in the design document by changing its unlabeled fence to text, preserving the block contents.Source: Linters/SAST tools
openspec/changes/bindings-auto-injection/design.md-39-44 (1)
39-44: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRemove temporary
Cargo.lockwhenhasCargoToml === false.When a temporary
Cargo.tomlis created (line 143),cargo buildat line 150 can generateCargo.lockin the source directory. ThehasCargoToml === falsecleanup path (lines 196–199) removes the temporaryCargo.tomlbut does not remove the generatedCargo.lock. In contrast, thehasCargoToml === truepath (lines 187–195) properly cleans upCargo.lockwhen it was generated during the build. Add equivalent cleanup for the temporaryCargo.lockin thehasCargoToml === falsebranch, and test cleanup after both build success and build failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/bindings-auto-injection/design.md` around lines 39 - 44, Update the hasCargoToml === false cleanup branch to remove the Cargo.lock generated alongside the temporary Cargo.toml, matching the existing restoration cleanup used when hasCargoToml === true. Ensure this cleanup runs after both successful and failed builds, and add coverage for both outcomes.openspec/changes/bindings-auto-injection/proposal.md-55-55 (1)
55-55: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAddress compatibility risk from
talc = "5"semver range.
talc = "5"permits any version from 5.0.0 to 5.999.999. The bindings crate is vendored, buttalcis fetched from crates.io during builds. Cargo.lock is not committed (excluded in .gitignore). The proposal documents this as a "Low" risk and requires verification "at apply time," but does not show automated enforcement.Implement one of the following:
- Narrow to an exact version (e.g.,
talc = "=5.0.4") that was validated during design.- Generate and commit a Cargo.lock for bindings builds to pin resolved versions.
- Automate compatibility verification to detect talc API drift before build.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/bindings-auto-injection/proposal.md` at line 55, Update the talc dependency compatibility plan in the bindings auto-injection proposal to provide automated version/API-drift enforcement instead of relying only on “verify at apply time.” Choose one supported approach: pin talc to the validated exact version, commit a Cargo.lock for bindings builds, or add a pre-build compatibility check that detects talc API drift; document the selected mechanism and change the risk assessment accordingly.packages/compiler/src/strategies/rust-strategy.ts-200-207 (1)
200-207: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winThe
throwinfinallydiscards the original error and reports a wrong message.Biome flags this as
lint/correctness/noUnsafeFinally. Two concrete defects follow:
- If the build failed and the restore also failed, the build error is lost. The caller never sees the compiler diagnostics.
- If the build succeeded and only the restore failed, the message still claims "Rust build failed", which is false.
Capture the pending error and attach it, then build the message from the actual state.
🐛 Proposed fix
- } catch (restoreErr: any) { - // El restore nunca debe enmascarar el error original del build ni dejar - // el workspace con el manifest inyectado silenciosamente (D3). - throw new CompilerError(`Rust build failed and restoring Cargo.toml/Cargo.lock also failed: ${restoreErr.message ?? restoreErr}`, { - code: 'COMPILER_ERROR', - fileName: options.fileName, - stderr: restoreErr.message ?? restoreErr, - }); + } catch (restoreErr: any) { + // El restore no debe perder el error original del build (D3). + throw new CompilerError(`Restoring Cargo.toml/Cargo.lock failed after the Rust build: ${restoreErr.message ?? restoreErr}`, { + code: 'COMPILER_ERROR', + fileName: options.fileName, + stderr: restoreErr.message ?? restoreErr, + restoreError: restoreErr.message ?? String(restoreErr), + }); }To preserve the build error, store it in the
catchblock above and include it indetailshere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/strategies/rust-strategy.ts` around lines 200 - 207, Update the Rust build cleanup flow around the restore-error catch and its enclosing finally so it never throws directly from finally. Preserve the original build error captured by the preceding catch, attach it to the restore failure details, and construct the CompilerError message based on whether the build failed, restoration failed, or both, so successful builds with restore failures are not reported as build failures.Source: Linters/SAST tools
packages/compiler/src/strategies/rust-strategy.ts-196-199 (1)
196-199: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the lockfile that cargo created in the temporary-manifest path.
When no
Cargo.tomlexisted, cargo still writesCargo.lockinto the source directory during the build. This branch deletes only the temporaryCargo.toml, so the lockfile stays in the user's tree. ThehasCargoTomlbranch already deletes an unexpected lockfile at line 194. Make the cleanup consistent.🐛 Proposed fix
} else if (fs.existsSync(cargoTomlPath)) { // Limpiar el Cargo.toml temporal que creamos fs.rmSync(cargoTomlPath, { force: true }); + // Cargo pudo crear un Cargo.lock junto al manifest temporal + if (!backupLock) fs.rmSync(cargoLockPath, { force: true }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/strategies/rust-strategy.ts` around lines 196 - 199, Update the cleanup branch around cargoTomlPath and the existing hasCargoToml lockfile removal to also delete the Cargo.lock created in the temporary-manifest path, using the same forceful cleanup behavior and preserving the existing Cargo.toml cleanup.packages/compiler/src/strategies/cpp-strategy.ts-70-70 (1)
70-70: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
CMAKE_CXX_FLAGSbreaks when the bindings path contains a space.CMake stores
CMAKE_CXX_FLAGSas one string and splits it on whitespace when it composes compiler command lines. IfBINDINGS_DIRcontains a space, the include path breaks. Package installation paths commonly contain spaces on macOS and Windows. Quote the path inside the flag value.🐛 Proposed fix
- cmakeArgs.push(`-DCMAKE_CXX_FLAGS=-I${BINDINGS_DIR}`); + cmakeArgs.push(`-DCMAKE_CXX_FLAGS:STRING=-I"${BINDINGS_DIR}"`);Setting this cache variable also replaces any
CMAKE_CXX_FLAGSthe project expects. Consider-DCMAKE_CXX_STANDARD_INCLUDE_DIRECTORIESor a CMake include-path variable instead, so project flags stay intact.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/strategies/cpp-strategy.ts` at line 70, Update the CMake argument construction near cmakeArgs to avoid embedding BINDINGS_DIR in CMAKE_CXX_FLAGS: use CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES or another dedicated CMake include-path variable, preserving existing project compiler flags and correctly handling paths containing spaces.packages/linker/templates/fs.rs-1-26 (1)
1-26: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd
extern crate allocand re-exportVecandvecto fix compilation in thisno_stdmodule.The module declares
no_stdcompatible (line 1) but usesVec<u8>(line 10) andvec without imports. Inno_stdcrates, neitherVecnorvecis in the prelude. The working version atpackages/compiler/src/bindings/rust/src/fs.rsincludes the correct setup:extern crate alloc; use alloc::vec; use alloc::vec::Vec;Add these three lines after line 4 (
use crate::wasi;). The template version andpackages/compiler/src/bindings/fs.rsboth lack these imports; the bindings/rust version already has them and compiles correctly.Secondary issue: If
fd_read()orfd_write()returns an error (lines 58, 67, 75), the?operator causes early return beforefd_close()executes. Consider wrapping these operations in scope guards or explicit error handling to ensure file descriptors close even on read/write failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/linker/templates/fs.rs` around lines 1 - 26, Add extern crate alloc plus alloc::vec and alloc::vec::Vec imports near the existing crate::wasi import so Vec and vec! compile in this no_std module. Also update the fd_read and fd_write flows in the relevant file-operation functions to ensure fd_close executes when either operation returns an error, using explicit cleanup or an appropriate scope guard while preserving error propagation.packages/compiler/src/bindings/fs.ts-79-90 (1)
79-90: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
readFiletruncates files larger than 64 KiB without an error.The buffer is fixed at 65536 bytes and only one
fdReadcall runs. A larger file returns partial content, and the caller cannot detect the truncation. A short read on a smaller file also returns less than the full content.Read in a loop until
fdReadreturns 0, and grow the result buffer. Alternatively, usepathFilestatGetto size the buffer first.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/bindings/fs.ts` around lines 79 - 90, Update readFile to read the entire opened file instead of performing a single fixed 64 KiB fdRead; loop until fdRead returns 0, growing or appending to the result buffer while preserving the existing error handling and closing the descriptor.packages/linker/templates/fs.h-8-8 (1)
8-8: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winGuard the
namespaceblock for C translation units.This template copy has the same gap as
packages/compiler/src/bindings/fs.h. Wrapnamespace fsin#ifdef __cplusplus.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/linker/templates/fs.h` at line 8, Guard the namespace fs declaration in the fs.h template with an __cplusplus preprocessor condition, matching the existing protection in the compiler binding header, so C translation units do not parse the C++ namespace block.Source: Linters/SAST tools
packages/compiler/src/bindings/fs.h-8-8 (1)
8-8: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winGuard the
namespaceblock for C translation units.
wasi.hwraps itsnamespace wasiin#ifdef __cplusplus, but this header does not. If a C translation unit includesfs.h, the compiler fails at line 8. Clang reports "unknown type name 'namespace'" for this line.🛡️ Proposed fix for C compatibility
+#ifdef __cplusplus namespace fs {Close the guard at the end of the file:
} // namespace fs +#endif🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/bindings/fs.h` at line 8, Guard the fs namespace block with an __cplusplus preprocessor check, adding the opening guard before namespace fs and closing it at the end of the header, so C translation units can include fs.h without parsing C++ namespace syntax.Source: Linters/SAST tools
🧹 Nitpick comments (6)
scripts/test-examples.mjs (1)
20-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd mounted-file integration coverage.
Extend the WASI runtime configuration to preopen
dataat/mnt/data;inherit_argv()does not create a preopen. Then add success-path assertions foras-fs,mounts-demo, andrust-fsthat expectHello from mounted directory!.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/test-examples.mjs` around lines 20 - 22, Update the WASI runtime configuration in scripts/test-examples.mjs to explicitly preopen the data directory at /mnt/data rather than relying on inherit_argv(). Add success-path assertions for as-fs, mounts-demo, and rust-fs that verify the output contains “Hello from mounted directory!”.packages/compiler/src/strategies/rust-strategy.test.ts (1)
366-441: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd lifecycle coverage for the temporary-manifest path.
These tests all start from an existing
Cargo.toml. No test covers the branch where noCargo.tomlexists. That branch is where cleanup is incomplete, as noted onpackages/compiler/src/strategies/rust-strategy.tslines 196-199. Add a test that compiles a bare.wasm.rs, lets the mocked cargo createCargo.lock, and asserts that both the temporaryCargo.tomland the createdCargo.lockare gone afterwards.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/strategies/rust-strategy.test.ts` around lines 366 - 441, Add a lifecycle test alongside the existing manifest tests for compiling a bare .wasm.rs without an initial Cargo.toml. Mock execFile to create Cargo.lock during the build, then assert after strategy.compile completes that both the temporary Cargo.toml and generated Cargo.lock are absent.packages/compiler/src/strategies/rust-strategy.ts (1)
30-34: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winEscape the TOML path value on every platform.
TOML basic strings treat
\as an escape introducer regardless of the host platform. A posix path that contains\or"therefore produces an invalid manifest whenisWin32is false. Escaping is a no-op for ordinary posix paths, so the flag adds risk without benefit. ApplyescapeTomlPathValueunconditionally and keepisWin32only if callers still need it.♻️ Proposed change
- const depLine = `wasm_apps_bindings = { path = "${isWin32 ? escapeTomlPathValue(bindingsDir) : bindingsDir}" }`; + const depLine = `wasm_apps_bindings = { path = "${escapeTomlPathValue(bindingsDir)}" }`;This change also requires updating the expectation at
packages/compiler/src/strategies/rust-strategy.test.ts:345-349.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/strategies/rust-strategy.ts` around lines 30 - 34, Update injectBindingsDependency to apply escapeTomlPathValue to bindingsDir unconditionally when constructing depLine, preventing backslashes and quotes from producing invalid TOML on any platform. Remove isWin32 from the function and its callers if it is no longer needed, and update the corresponding rust-strategy test expectation to match the always-escaped path.packages/compiler/src/bindings/fs.h (1)
8-8: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBoth
fs.hcopies share two defects: an unguardednamespaceblock andallocainside the descriptor loop.wasi.hguards its namespace with#ifdef __cplusplus, butfs.hdoes not, so a C translation unit fails at line 8.resolve_pathalso calls__builtin_allocaper iteration, and that memory is released only when the function returns.
packages/compiler/src/bindings/fs.h#L8-L8: wrapnamespace fsin#ifdef __cplusplusand close the guard after line 145.packages/linker/templates/fs.h#L8-L8: apply the same guard in this generated copy.packages/compiler/src/bindings/fs.h#L17-L55: declare one fixed-sizedirnamebuffer before the loop and skip descriptors whosepr_name_lendoes not fit.packages/linker/templates/fs.h#L17-L55: apply the same buffer change in this generated copy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/bindings/fs.h` at line 8, Update both packages/compiler/src/bindings/fs.h (lines 8-8 and 17-55) and packages/linker/templates/fs.h (lines 8-8 and 17-55): guard the fs namespace block with __cplusplus through its closing point, and in resolve_path declare one fixed-size dirname buffer before the descriptor loop, skipping descriptors whose pr_name_len exceeds the buffer capacity instead of allocating with __builtin_alloca per iteration.Source: Linters/SAST tools
packages/compiler/src/bindings/fs.ts (1)
20-38: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
resolvePathperforms a redundantfd_prestat_getper descriptor.
fdPrestatDirNameinpackages/compiler/src/bindings/wasi.tsline 136 already callsfdPrestatGet. Theprestatvalue at line 27 is used only as a null check, so each of the 61 iterations issues twofd_prestat_getcalls. Drop lines 27 and 28 and rely on thedirName == nullcheck at line 32. This also halves the heap allocations leaked bywasi.ts.♻️ Proposed refactor
for (let fd: i32 = 3; fd <= 63; fd++) { - // Get prestat - let prestat = fdPrestatGet(fd); - if (prestat == null) continue; - // Get dir name let dirName = fdPrestatDirName(fd); if (dirName == null) continue;Remove
fdPrestatGetfrom the import list at lines 3 to 6 after this change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/bindings/fs.ts` around lines 20 - 38, Remove the redundant fdPrestatGet call and prestat null check from resolvePath, relying on fdPrestatDirName’s null result to skip unavailable descriptors. Also remove fdPrestatGet from the bindings import list, leaving the existing dirName handling unchanged.packages/compiler/src/bindings/wasi.ts (1)
5-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBiome cannot parse the AssemblyScript
@externaldecorators in both files. The shared root cause is tool configuration: AssemblyScript sources use.tsextensions, so Biome and Prettier treat them as TypeScript and report parse errors on lines 5 to 45.
packages/compiler/src/bindings/wasi.ts#L5-L45: excludepackages/compiler/src/bindingsfrom the Biome configuration and from.prettierignore.packages/linker/templates/wasi.as.ts#L5-L45: excludepackages/linker/templatesin the same way, which also unblocks thepnpm checkfailure reported in the PR objectives.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/bindings/wasi.ts` around lines 5 - 45, Exclude packages/compiler/src/bindings from both Biome configuration and .prettierignore, covering the wasi.ts AssemblyScript bindings at packages/compiler/src/bindings/wasi.ts lines 5-45. Apply the same Biome and Prettier exclusions to packages/linker/templates, covering wasi.as.ts lines 5-45, so these AssemblyScript sources are not parsed as TypeScript and pnpm check succeeds.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/auto-pr.yml:
- Around line 55-63: Update the CI polling command in the workflow to query both
status and conclusion, then fail immediately whenever status is completed and
conclusion is not success. Continue polling only for non-completed runs, while
preserving the successful exit path and existing pending behavior.
In @.github/workflows/release.yml:
- Line 33: Update the release workflow’s pnpm install command to use
--frozen-lockfile instead of --no-frozen-lockfile, and commit any required
pnpm-lock.yaml changes so dependency resolution remains aligned with the
committed lockfile.
In
`@openspec/changes/bindings-auto-injection/specs/bindings-auto-injection/spec.md`:
- Line 11: Align the affected requirements in the spec with the actual binding
implementation: update REQ-1, REQ-4, and REQ-5 so the vendored crate details
match the current symbols in wasm_apps_bindings, remove the pub use
alloc::wasm_setup re-export from the crate shape, switch the internal allocation
reference to $crate::alloc::_talc_reexport, and require rust-hello to call
wasm_setup!() directly. Keep the related scenarios and tests in the same spec
consistent with these symbol and usage changes.
In `@packages/compiler/src/bindings/rust/src/fs.rs`:
- Around line 56-64: Actualiza read_file en
packages/compiler/src/bindings/rust/src/fs.rs (líneas 56-64) y
packages/compiler/src/bindings/fs.rs (líneas 52-60) para leer en un bucle,
acumulando bloques de hasta 64 KiB hasta que fd_read devuelva cero bytes;
conserva el cierre del descriptor y devuelve todos los datos leídos, no solo el
primer bloque.
In `@packages/compiler/src/bindings/rust/src/wasi.rs`:
- Around line 60-69: Corrija el layout de __wasi_filestat_t para cumplir los 64
bytes de WASI Preview1: cambie st_filetype a u8, agregue siete bytes de padding
y use st_nlink: u64 en el offset 24. Actualice stat_file para convertir
st_filetype a u32. Ajuste también RIGHTS_READ a 1 << 1 y RIGHTS_WRITE a 1 << 6
para solicitar los permisos correctos.
- Around line 169-185: Corrige las constantes RIGHTS_READ y RIGHTS_WRITE usadas
por path_open: asigna 0x02 a RIGHTS_READ y 0x40 a RIGHTS_WRITE según WASI
Preview 1. Mantén sin cambios la lógica que combina estos derechos en
rights_base.
In `@packages/compiler/src/bindings/wasi.h`:
- Around line 32-36: Align the __wasi_filestat_t definitions in
packages/compiler/src/bindings/wasi.h (32-36) and
packages/linker/templates/wasi.h (32-36) to the 64-byte WASI layout using
explicit padding. Update the corresponding Filestat structs in
packages/compiler/src/bindings/wasi.rs (55-65) and
packages/linker/templates/wasi.rs (55-65) with u8 filetype, padding, and u64
nlink, and convert filetype to u32 in stat_file. In
packages/compiler/src/bindings/wasi.ts (168-182) and
packages/linker/templates/wasi.as.ts (168-182), allocate 64 bytes, load filetype
as u8 and nlink as i64, and shift later field offsets by 8 bytes.
- Around line 110-121: Update the path_open wrappers in
packages/compiler/src/bindings/wasi.h (110-121),
packages/linker/templates/wasi.h (110-121),
packages/compiler/src/bindings/wasi.rs (165-185),
packages/linker/templates/wasi.rs (165-185),
packages/compiler/src/bindings/wasi.ts (150-166),
packages/linker/templates/wasi.as.ts (150-166), and
packages/compiler/src/bindings/rust/src/wasi.rs to map fd_read to 1 << 1 and
fd_write to 1 << 6. Remove the C++ 0x04 rights mapping, preserve O_CREAT as
oflags 0x01, and ensure path_create_file (1 << 10) is provided on the parent
dir_fd rather than the returned descriptor’s fs_rights_base.
In `@packages/compiler/src/bindings/wasi.ts`:
- Around line 69-74: The WASI wrappers leak memory allocated by makeIovec and
their result cells. Update packages/compiler/src/bindings/wasi.ts lines 69-74
and packages/linker/templates/wasi.as.ts lines 69-74 consistently, freeing each
allocated iovec and result cell before returning or replacing them with reusable
module-level scratch buffers; ensure stdoutWrite, fdWrite, fdRead, fdSeek,
fdPrestatGet, pathOpen, pathFilestatGet, environSizesGet, and clockTime no
longer retain allocations.
In `@packages/compiler/src/strategies/rust-strategy.ts`:
- Around line 125-145: Serialize Rust compilations per source directory in
RustCompilerStrategy.compile(), covering the Cargo.toml injection, cargo
invocation, and restoration/cleanup flow as one critical section. Add or reuse a
directory-keyed mutex so concurrent compilations for the same source directory
cannot overwrite manifests or remove another compilation’s Cargo.lock, while
allowing different directories to compile concurrently.
In `@packages/linker/templates/fs.rs`:
- Around line 56-61: Update both readFile implementations to read complete files
instead of performing one fixed 64 KiB read: in packages/linker/templates/fs.rs
lines 56-61, loop wasi::fd_read until it returns 0, growing buf and truncating
to the total byte count; in packages/linker/templates/fs.as.ts lines 79-89, loop
fdRead until it returns 0, accumulate the chunks in a growable buffer, and copy
the total bytes into the returned ArrayBuffer.
- Around line 56-78: Update read_file, read_file_to_buf, and write_file to
capture the fd_read or fd_write result before propagating it, always call
fd_close after the operation, then return the captured result so both success
and error paths close the descriptor.
---
Outside diff comments:
In `@packages/compiler/src/index.ts`:
- Around line 144-168: The temporary compilation path must preserve sibling
relative imports and remain inside tmpDir. Update the source handling around
rewriteBindingImports so rewritten files are placed next to the original under a
safe temporary name or the complete source directory is copied, and ensure
resolvedFileName values outside PROJECT_ROOT cannot produce ..-prefixed paths
escaping tmpDir; retain cleanupTmpDir behavior for temporary artifacts.
---
Minor comments:
In @.prettierignore:
- Around line 12-15: Update the AssemblyScript linker template files referenced
by the compiler formatting check so they are validly formatted and pnpm check
passes. Remove or narrow the .prettierignore entries for
packages/compiler/src/bindings/*.ts and packages/compiler/dist/bindings/*.ts,
and do not add broader ignore patterns.
In `@openspec/changes/bindings-auto-injection/design.md`:
- Line 37: Specify the fenced-code language for the ASCII data-flow block in the
design document by changing its unlabeled fence to text, preserving the block
contents.
- Around line 39-44: Update the hasCargoToml === false cleanup branch to remove
the Cargo.lock generated alongside the temporary Cargo.toml, matching the
existing restoration cleanup used when hasCargoToml === true. Ensure this
cleanup runs after both successful and failed builds, and add coverage for both
outcomes.
In `@openspec/changes/bindings-auto-injection/proposal.md`:
- Line 55: Update the talc dependency compatibility plan in the bindings
auto-injection proposal to provide automated version/API-drift enforcement
instead of relying only on “verify at apply time.” Choose one supported
approach: pin talc to the validated exact version, commit a Cargo.lock for
bindings builds, or add a pre-build compatibility check that detects talc API
drift; document the selected mechanism and change the risk assessment
accordingly.
In `@openspec/changes/bindings-auto-injection/tasks.md`:
- Line 65: Update task 5.3 in the checklist to remain unchecked until node
scripts/test-examples.mjs has completed successfully; do not claim all 12
examples build and run or mark integration verification complete before
recording the actual result.
In `@packages/compiler/src/bindings/fs.h`:
- Line 8: Guard the fs namespace block with an __cplusplus preprocessor check,
adding the opening guard before namespace fs and closing it at the end of the
header, so C translation units can include fs.h without parsing C++ namespace
syntax.
In `@packages/compiler/src/bindings/fs.ts`:
- Around line 79-90: Update readFile to read the entire opened file instead of
performing a single fixed 64 KiB fdRead; loop until fdRead returns 0, growing or
appending to the result buffer while preserving the existing error handling and
closing the descriptor.
In `@packages/compiler/src/strategies/cpp-strategy.ts`:
- Line 70: Update the CMake argument construction near cmakeArgs to avoid
embedding BINDINGS_DIR in CMAKE_CXX_FLAGS: use
CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES or another dedicated CMake include-path
variable, preserving existing project compiler flags and correctly handling
paths containing spaces.
In `@packages/compiler/src/strategies/rust-strategy.ts`:
- Around line 200-207: Update the Rust build cleanup flow around the
restore-error catch and its enclosing finally so it never throws directly from
finally. Preserve the original build error captured by the preceding catch,
attach it to the restore failure details, and construct the CompilerError
message based on whether the build failed, restoration failed, or both, so
successful builds with restore failures are not reported as build failures.
- Around line 196-199: Update the cleanup branch around cargoTomlPath and the
existing hasCargoToml lockfile removal to also delete the Cargo.lock created in
the temporary-manifest path, using the same forceful cleanup behavior and
preserving the existing Cargo.toml cleanup.
In `@packages/linker/templates/fs.h`:
- Line 8: Guard the namespace fs declaration in the fs.h template with an
__cplusplus preprocessor condition, matching the existing protection in the
compiler binding header, so C translation units do not parse the C++ namespace
block.
In `@packages/linker/templates/fs.rs`:
- Around line 1-26: Add extern crate alloc plus alloc::vec and alloc::vec::Vec
imports near the existing crate::wasi import so Vec and vec! compile in this
no_std module. Also update the fd_read and fd_write flows in the relevant
file-operation functions to ensure fd_close executes when either operation
returns an error, using explicit cleanup or an appropriate scope guard while
preserving error propagation.
---
Nitpick comments:
In `@packages/compiler/src/bindings/fs.h`:
- Line 8: Update both packages/compiler/src/bindings/fs.h (lines 8-8 and 17-55)
and packages/linker/templates/fs.h (lines 8-8 and 17-55): guard the fs namespace
block with __cplusplus through its closing point, and in resolve_path declare
one fixed-size dirname buffer before the descriptor loop, skipping descriptors
whose pr_name_len exceeds the buffer capacity instead of allocating with
__builtin_alloca per iteration.
In `@packages/compiler/src/bindings/fs.ts`:
- Around line 20-38: Remove the redundant fdPrestatGet call and prestat null
check from resolvePath, relying on fdPrestatDirName’s null result to skip
unavailable descriptors. Also remove fdPrestatGet from the bindings import list,
leaving the existing dirName handling unchanged.
In `@packages/compiler/src/bindings/wasi.ts`:
- Around line 5-45: Exclude packages/compiler/src/bindings from both Biome
configuration and .prettierignore, covering the wasi.ts AssemblyScript bindings
at packages/compiler/src/bindings/wasi.ts lines 5-45. Apply the same Biome and
Prettier exclusions to packages/linker/templates, covering wasi.as.ts lines
5-45, so these AssemblyScript sources are not parsed as TypeScript and pnpm
check succeeds.
In `@packages/compiler/src/strategies/rust-strategy.test.ts`:
- Around line 366-441: Add a lifecycle test alongside the existing manifest
tests for compiling a bare .wasm.rs without an initial Cargo.toml. Mock execFile
to create Cargo.lock during the build, then assert after strategy.compile
completes that both the temporary Cargo.toml and generated Cargo.lock are
absent.
In `@packages/compiler/src/strategies/rust-strategy.ts`:
- Around line 30-34: Update injectBindingsDependency to apply
escapeTomlPathValue to bindingsDir unconditionally when constructing depLine,
preventing backslashes and quotes from producing invalid TOML on any platform.
Remove isWin32 from the function and its callers if it is no longer needed, and
update the corresponding rust-strategy test expectation to match the
always-escaped path.
In `@scripts/test-examples.mjs`:
- Around line 20-22: Update the WASI runtime configuration in
scripts/test-examples.mjs to explicitly preopen the data directory at /mnt/data
rather than relying on inherit_argv(). Add success-path assertions for as-fs,
mounts-demo, and rust-fs that verify the output contains “Hello from mounted
directory!”.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6bbfcff6-a88f-49e4-947c-a0aed503471f
⛔ Files ignored due to path filters (2)
examples/rust-fs/src/Cargo.lockis excluded by!**/*.lockexamples/rust-hello/src/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (51)
.github/workflows/auto-pr.yml.github/workflows/release.yml.gitignore.prettierignoredata/greeting.txtexamples/as-fs/expected-stdout.txtexamples/as-fs/src/main.wasm.tsexamples/cpp-saludo/src/main.wasm.cppexamples/mounts-demo/expected-stdout.txtexamples/mounts-demo/src/main.wasm.cppexamples/rust-fs/expected-stdout.txtexamples/rust-fs/src/Cargo.tomlexamples/rust-fs/src/main.wasm.rsexamples/rust-hello/src/main.wasm.rsopenspec/changes/bindings-auto-injection/design.mdopenspec/changes/bindings-auto-injection/proposal.mdopenspec/changes/bindings-auto-injection/specs/bindings-auto-injection/spec.mdopenspec/changes/bindings-auto-injection/tasks.mdpackages/compiler/package.jsonpackages/compiler/scripts/copy-bindings.mjspackages/compiler/src/bindings/console.hpackages/compiler/src/bindings/console.rspackages/compiler/src/bindings/console.tspackages/compiler/src/bindings/fs.hpackages/compiler/src/bindings/fs.rspackages/compiler/src/bindings/fs.tspackages/compiler/src/bindings/rust/Cargo.tomlpackages/compiler/src/bindings/rust/src/alloc.rspackages/compiler/src/bindings/rust/src/console.rspackages/compiler/src/bindings/rust/src/fs.rspackages/compiler/src/bindings/rust/src/lib.rspackages/compiler/src/bindings/rust/src/wasi.rspackages/compiler/src/bindings/wasi.hpackages/compiler/src/bindings/wasi.rspackages/compiler/src/bindings/wasi.tspackages/compiler/src/index.test.tspackages/compiler/src/index.tspackages/compiler/src/strategies/cpp-strategy.tspackages/compiler/src/strategies/rust-strategy.test.tspackages/compiler/src/strategies/rust-strategy.tspackages/compiler/tsconfig.jsonpackages/linker/templates/console.as.tspackages/linker/templates/console.hpackages/linker/templates/console.rspackages/linker/templates/fs.as.tspackages/linker/templates/fs.hpackages/linker/templates/fs.rspackages/linker/templates/wasi.as.tspackages/linker/templates/wasi.hpackages/linker/templates/wasi.rsscripts/test-examples.mjs
🛑 Comments failed to post (2)
.github/workflows/auto-pr.yml (1)
55-63: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail SHA="${1:?commit SHA required}" gh run list --commit "$SHA" --workflow CI \ --json status,conclusion,workflowName,headSha --limit 10Repository: AndyTechnologies/wasm-apps
Length of output: 201
🏁 Script executed:
#!/bin/bash set -euo pipefail file=".github/workflows/auto-pr.yml" test -f "$file" sed -n '35,75p' "$file" printf '\nWorkflow references and run-list queries:\n' rg -n -C 3 'gh run list|workflow_dispatch|workflow_call|CI|conclusion|status' .github/workflows "$file"Repository: AndyTechnologies/wasm-apps
Length of output: 7306
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' 'CI workflow:' sed -n '1,180p' .github/workflows/ci.yml printf '\n%s\n' 'Auto-pr triggers and permissions:' sed -n '1,40p' .github/workflows/auto-pr.yml printf '\n%s\n' 'Workflow files and conclusion handling:' rg -n -C 2 '^(name:|on:| [a-z_]+:)|conclusion|status|gh run list' .github/workflowsRepository: AndyTechnologies/wasm-apps
Length of output: 6421
🌐 Web query:
GitHub Actions workflow run conclusion values status completed conclusion success failure cancelled timed_out startup_failure action_required neutral skipped stale official documentation💡 Result:
In GitHub Actions, workflow run statuses and conclusions are distinct concepts [1][2]. The status represents the lifecycle stage of a run, while the conclusion represents the final result after the run is completed [1][3]. Workflow Run Statuses The status indicates the current lifecycle phase of a workflow run [1]. Valid status values include: - queued: The run is waiting to be processed [4][3]. - in_progress: The run is currently executing [4][3]. - completed: The run has finished all its steps [4][3]. - waiting: The run is waiting for an external event (e.g., deployment protection rules or required reviews) [4][1]. - pending: The run is waiting (often due to concurrency limits) [4][1]. - requested: The run has been requested (primarily internal to GitHub) [4]. Workflow Run Conclusions The conclusion is only available when a run has a status of completed [3]. Valid conclusion values include: - success: The run or step completed successfully [5][3]. - failure: The run or step failed [5][3]. - cancelled: The run or step was cancelled [5][3]. - skipped: The run or step was skipped [5][3]. - action_required: The run or step requires an action to be taken [3]. - neutral: The run or step returned a neutral result (treated as success for dependent checks) [3]. - stale: The run or step was marked stale because it took too long [3]. - timed_out: The run or step exceeded its time limit [3]. Note that in the GitHub REST API, when filtering or querying, it is important to distinguish between these fields. While API documentation may sometimes group these values in parameter lists, they serve different functions: statuses track the progression toward completion, and conclusions provide the final outcome [4][2].
Citations:
- 1: github/docs#44984
- 2: https://github.com/github/docs/issues/20643
- 3: https://docs.github.com/en/pull-requests/reference/status-checks
- 4: https://docs.github.com/rest/actions/workflow-runs
- 5: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts
Fail fast on every completed non-success run.
Query
statusandconclusion. Whenstatus=completed, fail unlessconclusion=success; otherwise continue polling. This prevents conclusions such asstartup_failure,action_required,neutral,skipped, andstalefrom consuming the full timeout.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/auto-pr.yml around lines 55 - 63, Update the CI polling command in the workflow to query both status and conclusion, then fail immediately whenever status is completed and conclusion is not success. Continue polling only for non-completed runs, while preserving the successful exit path and existing pending behavior..github/workflows/release.yml (1)
33-33: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n -C 2 'pnpm install|frozen-lockfile' .github/workflowsRepository: AndyTechnologies/wasm-apps
Length of output: 1305
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' '--- workflow ---' cat -n .github/workflows/release.yml printf '%s\n' '--- package and lockfile metadata ---' find . -maxdepth 3 -type f \( -name 'package.json' -o -name 'pnpm-lock.yaml' -o -name 'pnpm-workspace.yaml' -o -name '.npmrc' -o -name 'packageManager' \) -print for f in $(find . -maxdepth 3 -type f \( -name 'package.json' -o -name 'pnpm-lock.yaml' -o -name 'pnpm-workspace.yaml' -o -name '.npmrc' -o -name 'packageManager' \)); do printf '\n--- %s ---\n' "$f" sed -n '1,120p' "$f" done printf '%s\n' '--- workflow triggers and release commands ---' rg -n -C 3 'on:|workflow_dispatch|tags:|release|publish|pnpm|actions/checkout|setup-node' .github/workflows/release.yml .github/workflows/ci.ymlRepository: AndyTechnologies/wasm-apps
Length of output: 20458
🌐 Web query:
pnpm install --frozen-lockfile --no-frozen-lockfile documentation CI behavior💡 Result:
In pnpm, --frozen-lockfile and --no-frozen-lockfile control whether the installation process allows modifications to the pnpm-lock.yaml file or requires it to be perfectly in sync with package.json [1][2]. --frozen-lockfile: When this flag is enabled, pnpm will not generate or modify the lockfile [1][2]. If the existing lockfile is out of sync with package.json (or if it is missing), the installation will fail [1][2]. This behavior is designed to ensure reproducible builds in CI/CD pipelines [3]. --no-frozen-lockfile: This flag explicitly disables the frozen-lockfile constraint, allowing pnpm to update the lockfile if it is out of sync with package.json [4][5]. CI Behavior: By default, pnpm automatically detects if it is running in a CI environment (using environment variables like CI, CONTINUOUS_INTEGRATION, BUILD_NUMBER, etc.) [2]. When a CI environment is detected, pnpm defaults to --frozen-lockfile mode [1][6]. Consequently, if your lockfile is outdated, the installation will fail in CI by default to prevent non-deterministic builds [1][7]. If you encounter an error in CI because your lockfile is out of sync, you should typically update your lockfile locally and commit the changes to your repository rather than disabling the check in CI [5]. However, if you have a specific requirement to bypass this behavior in CI, you can use the --no-frozen-lockfile flag to override the default [4][5]. Additionally, for CI environments, the command pnpm ci (which is an alias for pnpm install --frozen-lockfile) is the recommended standard, as it is specifically designed for reproducible builds [7].
Citations:
- 1: https://pnpm.io/cli/install
- 2: https://pnpm.io/10.x/cli/install
- 3: pnpm/pnpm#7740
- 4: https://github.com/pnpm/pnpm/blob/e1e29c15/installing/commands/src/install.ts
- 5: pnpm/pnpm#4151
- 6: https://pnpm.io/continuous-integration
- 7: https://stackoverflow.com/questions/70154568/pnpm-equivalent-command-for-npm-ci
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' import json import re from pathlib import Path workflow = Path(".github/workflows/release.yml").read_text() package = json.loads(Path("package.json").read_text()) lockfile = Path("pnpm-lock.yaml").read_text() checks = { "release_install_is_non_frozen": "pnpm install --no-frozen-lockfile" in workflow, "release_builds_after_install": bool(re.search(r"pnpm install --no-frozen-lockfile\s*\n\s*pnpm -r build", workflow)), "release_publishes_packages": "pnpm publish" in workflow, "package_manager_matches_setup": package.get("packageManager") == "pnpm@11.10.0", "lockfile_has_v9": "lockfileVersion: '9.0'" in lockfile, "lockfile_has_root_importer": re.search(r"^ \.:$", lockfile, re.M) is not None, } for name, value in checks.items(): print(f"{name}={value}") # Compare every declared dependency specifier with the corresponding lockfile # importer block, without resolving dependencies or modifying repository files. for manifest in [Path("package.json"), *sorted(Path("packages").glob("*/package.json"))]: data = json.loads(manifest.read_text()) rel = "." if manifest.name == "package.json" else str(manifest.parent) marker = " .:" if rel == "." else f" {rel}:" start = lockfile.find(marker) end = lockfile.find("\n ", start + len(marker)) if start >= 0 else -1 block = lockfile[start:] if start >= 0 and end < 0 else lockfile[start:end] for section in ("dependencies", "devDependencies", "optionalDependencies"): for name, spec in data.get(section, {}).items(): expected = f" {name}:\n specifier: {spec}" print(f"{rel}:{section}:{name}:specifier_in_lockfile={expected in block}") PYRepository: AndyTechnologies/wasm-apps
Length of output: 2221
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' '--- repository state ---' git status --short printf '%s\n' '--- lockfile and manifest references ---' rg -n -C 2 'pnpm-lock|pnpm install|frozen-lockfile|packageManager|lockfile' \ README.md CONTRIBUTING.md .github package.json pnpm-workspace.yaml .npmrc 2>/dev/null || trueRepository: AndyTechnologies/wasm-apps
Length of output: 2518
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' import json import re from pathlib import Path lock_lines = Path("pnpm-lock.yaml").read_text().splitlines() importers = {} in_importers = False current = None for line in lock_lines: if line == "importers:": in_importers = True continue if not in_importers: continue m = re.fullmatch(r" ([^ ].*):", line) if m: current = m.group(1) importers[current] = {} continue if current is None: continue section = re.fullmatch(r" (dependencies|devDependencies|optionalDependencies):", line) if section: current_section = section.group(1) importers[current].setdefault(current_section, {}) continue dep = re.fullmatch(r" (.+):", line) if dep and "current_section" in locals(): current_dep = dep.group(1) continue spec = re.fullmatch(r" specifier: (.+)", line) if spec and "current_section" in locals() and "current_dep" in locals(): importers[current][current_section][current_dep] = spec.group(1) failures = [] for manifest in [Path("package.json"), *sorted(Path("packages").glob("*/package.json"))]: data = json.loads(manifest.read_text()) importer = "." if manifest == Path("package.json") else str(manifest.parent) for section in ("dependencies", "devDependencies", "optionalDependencies"): for name, declared in data.get(section, {}).items(): actual = importers.get(importer, {}).get(section, {}).get(name) if actual != declared: failures.append((importer, section, name, declared, actual)) print(f"importers_parsed={sorted(importers)}") print(f"specifier_mismatches={len(failures)}") for failure in failures: print(failure) PYRepository: AndyTechnologies/wasm-apps
Length of output: 11108
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' import json import re from pathlib import Path lines = Path("pnpm-lock.yaml").read_text().splitlines() importers = {} in_importers = False current = None section = None dependency = None for line in lines: if line == "importers:": in_importers = True continue if in_importers and line == "packages:": break if not in_importers: continue m = re.fullmatch(r" (.+):", line) if m and not line.startswith(" "): current = m.group(1).strip("'\"") importers[current] = {} section = dependency = None continue m = re.fullmatch(r" (dependencies|devDependencies|optionalDependencies):", line) if m and current is not None: section = m.group(1) importers[current].setdefault(section, {}) dependency = None continue m = re.fullmatch(r" (.+):", line) if m and current is not None and section is not None: dependency = m.group(1).strip("'\"") continue m = re.fullmatch(r" specifier: (.+)", line) if m and current is not None and section is not None and dependency is not None: importers[current][section][dependency] = m.group(1) failures = [] for manifest in [Path("package.json"), *sorted(Path("packages").glob("*/package.json"))]: data = json.loads(manifest.read_text()) importer = "." if manifest == Path("package.json") else str(manifest.parent) for section in ("dependencies", "devDependencies", "optionalDependencies"): for name, declared in data.get(section, {}).items(): actual = importers.get(importer, {}).get(section, {}).get(name) # pnpm records the overridden specifier for dependencies affected by overrides. if actual is None: failures.append((importer, section, name, declared, actual)) print("importers=", sorted(importers)) print("missing_lockfile_specifiers=", len(failures)) for failure in failures: print(failure) PYRepository: AndyTechnologies/wasm-apps
Length of output: 286
Keep release installs frozen.
--no-frozen-lockfileallows pnpm to resolve dependencies that differ from the committedpnpm-lock.yaml. Because this workflow builds and publishes packages, restore--frozen-lockfileand commit any required lockfile update.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml at line 33, Update the release workflow’s pnpm install command to use --frozen-lockfile instead of --no-frozen-lockfile, and commit any required pnpm-lock.yaml changes so dependency resolution remains aligned with the committed lockfile.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (3)
packages/linker/templates/wasi.h (1)
44-196: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAttach the doc comments to their functions.
Lines 44-195 contain 20 doc blocks stacked before
namespace wasi {. None is adjacent to the function it describes, so doc tools attach nothing and readers cannot map a block to a function. Move each block directly above its function.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/linker/templates/wasi.h` around lines 44 - 196, The documentation blocks in namespace wasi are detached from the functions they describe. Move each of the 20 comments directly above its corresponding function declaration, preserving the existing descriptions and ordering so documentation tools associate every block with the correct function.packages/compiler/src/strategies/rust-strategy.ts (1)
36-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEscape the TOML path value on every platform.
injectBindingsDependencyescapes the path only whenisWin32istrue. A POSIX path may contain"or\, which then produces an invalid manifest. Escaping is safe for plain POSIX paths, so apply it unconditionally and drop the flag.♻️ Proposed refactor
- const depLine = `wasm_apps_bindings = { path = "${isWin32 ? escapeTomlPathValue(bindingsDir) : bindingsDir}" }`; + const depLine = `wasm_apps_bindings = { path = "${escapeTomlPathValue(bindingsDir)}" }`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/strategies/rust-strategy.ts` around lines 36 - 41, Update injectBindingsDependency to always pass bindingsDir through escapeTomlPathValue when constructing depLine, regardless of platform. Remove the isWin32 parameter and its conditional usage, while preserving the existing idempotence guard and line-ending handling.packages/compiler/src/bindings/fs.h (1)
8-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
resolve_pathdoc block sits abovenamespace fsin both C++ headers. The two headers are copies, so the same misplaced comment appears twice.
packages/compiler/src/bindings/fs.h#L8-L20: move the doc block from lines 8-15 to directly aboveresolve_pathat line 20.packages/linker/templates/fs.h#L8-L19: move the doc block from lines 8-14 to directly aboveresolve_pathat line 19.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler/src/bindings/fs.h` around lines 8 - 20, Move the resolve_path documentation block so it directly precedes the resolve_path function in packages/compiler/src/bindings/fs.h (lines 8-20) and packages/linker/templates/fs.h (lines 8-19); remove it from above namespace fs in both copied headers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/compiler/src/bindings/rust/src/fs.rs`:
- Line 42: Convert prestat.pr_name_len to usize when supplying the length to the
vec! allocation in the surrounding filesystem code, preserving the existing
buffer contents and allocation behavior.
In `@packages/compiler/src/bindings/rust/src/wasi.rs`:
- Around line 194-195: Remove the duplicated or body-less Rust function
signature lines so each binding contains only one valid declaration: in
packages/compiler/src/bindings/rust/src/wasi.rs at 194-195, 267-268, 292-293,
438-439, and 611-612 for fd_read, fd_prestat_get, fd_prestat_dir_name,
path_unlink_file, and environ_sizes_get; in
packages/compiler/src/bindings/wasi.rs at 165-166 and 621 for fd_write and exit;
in packages/compiler/src/bindings/fs.rs at 113-114 for read_file_to_buf; in
packages/compiler/src/bindings/rust/src/fs.rs at 142-143 for write_file; and in
packages/linker/templates/fs.rs at 76-77, 132-133, and 211-212 for read_file,
write_file, and mkdir.
In `@packages/compiler/src/bindings/wasi.rs`:
- Around line 1-6: The three no_std WASI binding copies must import alloc before
using Vec and vec!: update packages/compiler/src/bindings/wasi.rs lines 1-6,
packages/compiler/src/bindings/fs.rs lines 1-11, and
packages/linker/templates/fs.rs lines 1-11 by adding extern crate alloc, use
alloc::vec, and use alloc::vec::Vec; additionally cast prestat.pr_name_len to
usize in fd_prestat_dir_name callers at packages/compiler/src/bindings/fs.rs
line 36 and packages/linker/templates/fs.rs line 34.
In `@packages/compiler/src/bindings/wasi.ts`:
- Around line 215-217: Update the RIGHTS_READ and RIGHTS_WRITE constants in the
WASI bindings and corresponding AssemblyScript template and Rust copies to use
the correct WASI Preview1 rights bits: FD_READ for read and FD_WRITE for write.
Ensure pathOpen’s rightsBase receives the corrected values consistently across
all bindings and templates.
In `@packages/compiler/src/index.ts`:
- Around line 156-166: Update the rewritten-source handling around
rewrittenFromDisk so sibling relative imports continue resolving from the
original disk source location: either retain unchanged disk sources and
configure bindings through --path, or copy the complete local relative-import
closure into tmpDir. Add an integration test covering a disk source that imports
both a bare binding and ./local-module.
- Around line 161-166: Validate the normalized path derived from
resolvedFileName in compileAssemblyScriptCore before creating directories or
writing files, rejecting any candidate outside tmpDir with a directory-boundary
check. Ensure this protects all callers, including the CLI and compileWasm,
while preserving valid paths within tmpDir.
In `@packages/compiler/src/strategies/rust-strategy.ts`:
- Around line 206-214: Update the restore-error handling around the Rust
strategy’s compile/finally flow so the finally block never throws. Capture any
restore failure from the cleanup logic, preserve the original build
CompilerError and stderr when the build fails, and only throw the wrapped
restore error after compile completes successfully; otherwise retain the
ToolchainResult.
In `@packages/linker/templates/wasi.as.ts`:
- Around line 75-80: Update makeIovec and the affected fdWrite, fdRead, fdSeek,
and fdStat callers to free every heap.alloc result after the corresponding WASI
operation completes, including iovec buffers and temporary allocations. Ensure
cleanup occurs before each return and on all relevant paths so repeated calls do
not exhaust linear memory.
- Around line 215-216: Update the WASI rights constants used by fdRead and
fdWrite, including the mirrored bindings, so RIGHTS_READ uses 0x2
(RIGHTS_FD_READ) and RIGHTS_WRITE uses 0x40 (RIGHTS_FD_WRITE), replacing the
incorrect bit values while leaving unrelated rights unchanged.
- Around line 251-265: Update pathFilestatGet to use the 64-byte WASI Preview1
filestat layout: allocate 64 bytes, load stNlink as i64 at offset 24, and shift
stSize, stAtim, stMtim, and stCtim to offsets 32, 40, 48, and 56 while keeping
stFiletype at 16. Change Filestat.stNlink to i64, and apply the matching layout
and type updates in the compiler WASI bindings.
In `@packages/linker/templates/wasi.h`:
- Around line 32-36: Update __wasi_filestat_t and its matching C and Rust
definitions to use an 8-bit st_filetype, seven bytes of padding, and a 64-bit
st_nlink, producing the required 64-byte Preview1 layout. Update Rust callers
that return st_filetype as u32 to cast the field explicitly, while preserving
the existing path_filestat_get behavior.
- Around line 262-273: Update path_open so O_READ contributes WASI FD_READ (0x2)
and O_WRITE contributes FD_WRITE (0x40) to rights_base, while preserving O_CREAT
and O_TRUNC in flags. When O_CREAT is set, include PATH_CREATE_FILE in the
directory rights; when O_TRUNC is set, include PATH_FILESTAT_SET_SIZE, and pass
the resulting rights to __wasi_path_open.
---
Nitpick comments:
In `@packages/compiler/src/bindings/fs.h`:
- Around line 8-20: Move the resolve_path documentation block so it directly
precedes the resolve_path function in packages/compiler/src/bindings/fs.h (lines
8-20) and packages/linker/templates/fs.h (lines 8-19); remove it from above
namespace fs in both copied headers.
In `@packages/compiler/src/strategies/rust-strategy.ts`:
- Around line 36-41: Update injectBindingsDependency to always pass bindingsDir
through escapeTomlPathValue when constructing depLine, regardless of platform.
Remove the isWin32 parameter and its conditional usage, while preserving the
existing idempotence guard and line-ending handling.
In `@packages/linker/templates/wasi.h`:
- Around line 44-196: The documentation blocks in namespace wasi are detached
from the functions they describe. Move each of the 20 comments directly above
its corresponding function declaration, preserving the existing descriptions and
ordering so documentation tools associate every block with the correct function.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 52beed26-cc05-462a-b795-f3c8f396691d
⛔ Files ignored due to path filters (2)
examples/rust-fs/src/Cargo.lockis excluded by!**/*.lockexamples/rust-hello/src/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (49)
.gitignore.prettierignoredata/greeting.txtexamples/as-fs/expected-stdout.txtexamples/as-fs/src/main.wasm.tsexamples/cpp-saludo/src/main.wasm.cppexamples/mounts-demo/expected-stdout.txtexamples/mounts-demo/src/main.wasm.cppexamples/rust-fs/expected-stdout.txtexamples/rust-fs/src/Cargo.tomlexamples/rust-fs/src/main.wasm.rsexamples/rust-hello/src/main.wasm.rsopenspec/changes/bindings-auto-injection/design.mdopenspec/changes/bindings-auto-injection/proposal.mdopenspec/changes/bindings-auto-injection/specs/bindings-auto-injection/spec.mdopenspec/changes/bindings-auto-injection/tasks.mdpackages/compiler/package.jsonpackages/compiler/scripts/copy-bindings.mjspackages/compiler/src/bindings/console.hpackages/compiler/src/bindings/console.rspackages/compiler/src/bindings/console.tspackages/compiler/src/bindings/fs.hpackages/compiler/src/bindings/fs.rspackages/compiler/src/bindings/fs.tspackages/compiler/src/bindings/rust/Cargo.tomlpackages/compiler/src/bindings/rust/src/alloc.rspackages/compiler/src/bindings/rust/src/console.rspackages/compiler/src/bindings/rust/src/fs.rspackages/compiler/src/bindings/rust/src/lib.rspackages/compiler/src/bindings/rust/src/wasi.rspackages/compiler/src/bindings/wasi.hpackages/compiler/src/bindings/wasi.rspackages/compiler/src/bindings/wasi.tspackages/compiler/src/index.test.tspackages/compiler/src/index.tspackages/compiler/src/strategies/cpp-strategy.tspackages/compiler/src/strategies/rust-strategy.test.tspackages/compiler/src/strategies/rust-strategy.tspackages/compiler/tsconfig.jsonpackages/linker/templates/console.as.tspackages/linker/templates/console.hpackages/linker/templates/console.rspackages/linker/templates/fs.as.tspackages/linker/templates/fs.hpackages/linker/templates/fs.rspackages/linker/templates/wasi.as.tspackages/linker/templates/wasi.hpackages/linker/templates/wasi.rsscripts/test-examples.mjs
🚧 Files skipped from review as they are similar to previous changes (32)
- examples/rust-fs/expected-stdout.txt
- .gitignore
- examples/mounts-demo/expected-stdout.txt
- examples/as-fs/expected-stdout.txt
- packages/compiler/src/bindings/rust/Cargo.toml
- data/greeting.txt
- packages/compiler/package.json
- packages/compiler/scripts/copy-bindings.mjs
- examples/mounts-demo/src/main.wasm.cpp
- examples/rust-hello/src/main.wasm.rs
- examples/rust-fs/src/main.wasm.rs
- packages/compiler/src/bindings/console.ts
- packages/linker/templates/console.rs
- .prettierignore
- scripts/test-examples.mjs
- packages/compiler/src/strategies/cpp-strategy.ts
- examples/cpp-saludo/src/main.wasm.cpp
- openspec/changes/bindings-auto-injection/proposal.md
- examples/rust-fs/src/Cargo.toml
- packages/compiler/src/bindings/rust/src/alloc.rs
- examples/as-fs/src/main.wasm.ts
- packages/compiler/src/bindings/rust/src/lib.rs
- packages/compiler/src/bindings/rust/src/console.rs
- openspec/changes/bindings-auto-injection/tasks.md
- packages/linker/templates/fs.as.ts
- packages/linker/templates/console.as.ts
- openspec/changes/bindings-auto-injection/specs/bindings-auto-injection/spec.md
- packages/compiler/src/bindings/fs.ts
- packages/compiler/src/bindings/console.rs
- packages/compiler/tsconfig.json
- packages/linker/templates/wasi.rs
- packages/compiler/src/bindings/wasi.h
| pub fn fd_read(fd: i32, buf: &mut [u8]) -> Result<usize, WasiError> { | ||
| pub fn fd_read(fd: i32, buf: &mut [u8]) -> Result<usize, WasiError> { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Duplicated function signature lines break every Rust binding copy. The copies were produced with the signature line repeated above the function body, which is a syntax error in each file.
packages/compiler/src/bindings/rust/src/wasi.rs#L194-L195: delete the repeated signature line forfd_read, and also forfd_prestat_get(267-268),fd_prestat_dir_name(292-293),path_unlink_file(438-439) andenviron_sizes_get(611-612).packages/compiler/src/bindings/wasi.rs#L165-L166: delete the repeatedfd_writesignature, and the body-lessexitsignature at line 621.packages/compiler/src/bindings/fs.rs#L113-L114: delete the repeatedread_file_to_bufsignature.packages/compiler/src/bindings/rust/src/fs.rs#L142-L143: delete the repeatedwrite_filesignature.packages/linker/templates/fs.rs#L76-L77: delete the body-lessread_filesignature, and the same duplicates forwrite_file(132-133) andmkdir(211-212).
📍 Affects 5 files
packages/compiler/src/bindings/rust/src/wasi.rs#L194-L195(this comment)packages/compiler/src/bindings/wasi.rs#L165-L166packages/compiler/src/bindings/fs.rs#L113-L114packages/compiler/src/bindings/rust/src/fs.rs#L142-L143packages/linker/templates/fs.rs#L76-L77
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/bindings/rust/src/wasi.rs` around lines 194 - 195,
Remove the duplicated or body-less Rust function signature lines so each binding
contains only one valid declaration: in
packages/compiler/src/bindings/rust/src/wasi.rs at 194-195, 267-268, 292-293,
438-439, and 611-612 for fd_read, fd_prestat_get, fd_prestat_dir_name,
path_unlink_file, and environ_sizes_get; in
packages/compiler/src/bindings/wasi.rs at 165-166 and 621 for fd_write and exit;
in packages/compiler/src/bindings/fs.rs at 113-114 for read_file_to_buf; in
packages/compiler/src/bindings/rust/src/fs.rs at 142-143 for write_file; and in
packages/linker/templates/fs.rs at 76-77, 132-133, and 211-212 for read_file,
write_file, and mkdir.
| // WASI mid-level wrapper for Rust (no_std compatible) | ||
| #![allow(non_snake_case, non_camel_case_types, dead_code)] | ||
|
|
||
| // ── Raw syscall declarations ─────────────────────── | ||
|
|
||
| #[link(wasm_import_module = "wasi_snapshot_preview1")] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Three no_std bindings use Vec and vec! without importing alloc. Only packages/compiler/src/bindings/rust/src/wasi.rs declares extern crate alloc; with the vec imports; the other copies omit them, so the Vec and vec! uses do not resolve.
packages/compiler/src/bindings/wasi.rs#L1-L6: addextern crate alloc;,use alloc::vec;anduse alloc::vec::Vec;forfd_prestat_dir_name.packages/compiler/src/bindings/fs.rs#L1-L11: add the same three declarations afteruse crate::wasi;, and convertprestat.pr_name_lentousizeat line 36.packages/linker/templates/fs.rs#L1-L11: add the same three declarations, and convertprestat.pr_name_lentousizeat line 34.
📍 Affects 3 files
packages/compiler/src/bindings/wasi.rs#L1-L6(this comment)packages/compiler/src/bindings/fs.rs#L1-L11packages/linker/templates/fs.rs#L1-L11
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/bindings/wasi.rs` around lines 1 - 6, The three no_std
WASI binding copies must import alloc before using Vec and vec!: update
packages/compiler/src/bindings/wasi.rs lines 1-6,
packages/compiler/src/bindings/fs.rs lines 1-11, and
packages/linker/templates/fs.rs lines 1-11 by adding extern crate alloc, use
alloc::vec, and use alloc::vec::Vec; additionally cast prestat.pr_name_len to
usize in fd_prestat_dir_name callers at packages/compiler/src/bindings/fs.rs
line 36 and packages/linker/templates/fs.rs line 34.
| const RIGHTS_READ: i64 = 0x0000000000000001; | ||
| const RIGHTS_WRITE: i64 = 0x0000000000000002; | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="packages/compiler/src/bindings/wasi.ts"
printf '%s\n' '--- target lines ---'
sed -n '180,245p' "$file"
printf '%s\n' '--- rights and WASI call sites ---'
rg -n -C 4 'RIGHTS_(READ|WRITE)|pathOpen|fdRead|fdWrite|FD_(DATASYNC|READ|WRITE)' "$file" packages/compiler
printf '%s\n' '--- repository references to the constants ---'
rg -n -C 3 'RIGHTS_READ|RIGHTS_WRITE' .Repository: AndyTechnologies/wasm-apps
Length of output: 24252
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
expected = {
"FD_DATASYNC": 0x1,
"FD_READ": 0x2,
"FD_WRITE": 0x40,
}
source = Path("packages/compiler/src/bindings/wasi.ts").read_text()
values = {
name: int(value, 16)
for name, value in re.findall(
r"const RIGHTS_(READ|WRITE): i64 = (0x[0-9a-fA-F]+);", source
)
}
actual = {
"FD_DATASYNC": values["READ"],
"FD_READ": values["WRITE"],
"FD_WRITE": values["WRITE"],
}
print("declared constants:", values)
print("Preview1 comparison:")
for name, want in expected.items():
got = actual[name] if name != "FD_WRITE" else None
if name == "FD_WRITE":
got = 0x2 # current RIGHTS_WRITE value used for write requests
print(f" {name}: current=0x{got:x}, expected=0x{want:x}, match={got == want}")
read_rights = values["READ"]
write_rights = values["READ"] | values["WRITE"]
print(f"read path rights_base=0x{read_rights:x}: FD_READ present={bool(read_rights & expected['FD_READ'])}")
print(f"write path rights_base=0x{write_rights:x}: FD_WRITE present={bool(write_rights & expected['FD_WRITE'])}")
PY
printf '%s\n' '--- generation and consumption references ---'
rg -n -C 3 'wasi\.as\.ts|wasi\.rs|bindings/wasi|templates/wasi|generate.*template|template.*wasi' \
package.json packages scripts .github 2>/dev/null || true
printf '%s\n' '--- relevant Rust implementations ---'
sed -n '345,435p' packages/compiler/src/bindings/rust/src/wasi.rs
sed -n '345,410p' packages/linker/templates/wasi.rsRepository: AndyTechnologies/wasm-apps
Length of output: 7023
Use the WASI Preview1 rights bits in all bindings and templates.
pathOpen passes rightsBase to both rights parameters. 0x1 is FD_DATASYNC, not FD_READ; 0x2 is FD_READ, not FD_WRITE. Therefore, read opens lack FD_READ, and write/create opens lack FD_WRITE (0x40). Apply the corrected values to the TypeScript, AssemblyScript template, and Rust copies of these bindings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/bindings/wasi.ts` around lines 215 - 217, Update the
RIGHTS_READ and RIGHTS_WRITE constants in the WASI bindings and corresponding
AssemblyScript template and Rust copies to use the correct WASI Preview1 rights
bits: FD_READ for read and FD_WRITE for write. Ensure pathOpen’s rightsBase
receives the corrected values consistently across all bindings and templates.
| function makeIovec(data: usize, len: i32): usize { | ||
| let buf = heap.alloc(8); | ||
| store<usize>(buf, data, 0); | ||
| store<i32>(buf, len, 4); | ||
| return buf; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Free the scratch allocations.
makeIovec and each caller allocate with heap.alloc and never call heap.free. Every write, read, seek and stat call leaks memory. In a loop the module exhausts linear memory. Free each allocation before returning.
♻️ Proposed fix for `fdWrite`
export function fdWrite(fd: i32, data: usize, len: i32): i32 {
let iov = makeIovec(data, len);
let nwritten = heap.alloc(4);
let ret = wasiFdWrite(fd, iov, 1, nwritten);
- return ret == 0 ? load<i32>(nwritten) : -ret;
+ let result = ret == 0 ? load<i32>(nwritten) : -ret;
+ heap.free(nwritten);
+ heap.free(iov);
+ return result;
}Also applies to: 90-95, 132-152
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/linker/templates/wasi.as.ts` around lines 75 - 80, Update makeIovec
and the affected fdWrite, fdRead, fdSeek, and fdStat callers to free every
heap.alloc result after the corresponding WASI operation completes, including
iovec buffers and temporary allocations. Ensure cleanup occurs before each
return and on all relevant paths so repeated calls do not exhaust linear memory.
| const RIGHTS_READ: i64 = 0x0000000000000001; | ||
| const RIGHTS_WRITE: i64 = 0x0000000000000002; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
sed -n '185,245p' packages/linker/templates/wasi.as.ts
printf '%s\n' '--- rights references ---'
rg -n -C 3 'RIGHTS_(READ|WRITE)|pathOpen|fdWrite' packages/linker/templates/wasi.as.ts packagesRepository: AndyTechnologies/wasm-apps
Length of output: 17840
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import re
from pathlib import Path
source = Path("packages/linker/templates/wasi.as.ts").read_text()
constants = dict(re.findall(r"const\s+(RIGHTS_(?:READ|WRITE)):\s*i64\s*=\s*0x([0-9a-fA-F]+)", source))
expected = {"RIGHTS_READ": 0x02, "RIGHTS_WRITE": 0x40}
print("declared:", {k: int(v, 16) for k, v in constants.items()})
print("expected:", expected)
for for_write, create in ((False, False), (True, False), (False, True), (True, True)):
rights = int(constants["RIGHTS_READ"], 16)
if for_write:
rights |= int(constants["RIGHTS_WRITE"], 16)
if create:
rights |= int(constants["RIGHTS_WRITE"], 16)
print(f"forWrite={for_write}, create={create}: rights=0x{rights:016x}",
"read=" + str(bool(rights & expected["RIGHTS_READ"])),
"write=" + str(bool(rights & expected["RIGHTS_WRITE"])),
"datasync=" + str(bool(rights & 0x01)))
PY
printf '%s\n' '--- mirrored declarations ---'
rg -n 'const RIGHTS_(READ|WRITE)' packages/linker/templates packages/compiler/src/bindings
printf '%s\n' '--- relevant tests and generation references ---'
rg -n -i -C 2 'pathOpen|path_open|fdWrite|RIGHTS_READ|RIGHTS_WRITE|templates/wasi|bindings/wasi' packages tests 2>/dev/null || trueRepository: AndyTechnologies/wasm-apps
Length of output: 30665
Correct the WASI rights bit values.
RIGHTS_FD_READ is 0x2, and RIGHTS_FD_WRITE is 0x40; 0x1 is RIGHTS_FD_DATASYNC. With the current values, fdRead and fdWrite can fail with ENOTCAPABLE. Apply these values to all mirrored WASI bindings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/linker/templates/wasi.as.ts` around lines 215 - 216, Update the WASI
rights constants used by fdRead and fdWrite, including the mirrored bindings, so
RIGHTS_READ uses 0x2 (RIGHTS_FD_READ) and RIGHTS_WRITE uses 0x40
(RIGHTS_FD_WRITE), replacing the incorrect bit values while leaving unrelated
rights unchanged.
| export function pathFilestatGet(dirFd: i32, path: usize, pathLen: i32): Filestat | null { | ||
| let buf = heap.alloc(56); // 8 * 7 = 56 bytes | ||
| let ret = wasiPathFilestatGet(dirFd, 0, path, pathLen, buf); | ||
| if (ret != 0) return null; | ||
| let fs = new Filestat(); | ||
| fs.stDev = load<i64>(buf, 0); | ||
| fs.stIno = load<i64>(buf, 8); | ||
| fs.stFiletype = load<i32>(buf, 16); | ||
| fs.stNlink = load<i32>(buf, 20); | ||
| fs.stSize = load<i64>(buf, 24); | ||
| fs.stAtim = load<i64>(buf, 32); | ||
| fs.stMtim = load<i64>(buf, 40); | ||
| fs.stCtim = load<i64>(buf, 48); | ||
| return fs; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target file ---'
sed -n '220,285p' packages/linker/templates/wasi.as.ts
printf '%s\n' '--- related Filestat definitions and helpers ---'
rg -n -C 4 'class Filestat|struct Filestat|stFiletype|wasiPathFilestatGet|function load|export function load' packages/linker packages | head -240
printf '%s\n' '--- repository context ---'
rg -n -C 3 'pathFilestatGet\(' packages tests .github 2>/dev/null | head -200Repository: AndyTechnologies/wasm-apps
Length of output: 21198
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- allocator definitions and call sites ---'
rg -n -C 5 'class Heap|namespace heap|heap\.alloc|function alloc|export function alloc' packages | head -300
printf '%s\n' '--- all Filestat implementations and field uses ---'
rg -n -C 5 'class Filestat|stNlink|stFiletype|stSize|stAtim|stCtim' packages tests 2>/dev/null | head -360
printf '%s\n' '--- AssemblyScript and binding context ---'
rg -n '"(assemblyscript|`@assemblyscript/loader`)"|assemblyscript' package.json packages/*/package.json pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null | head -120Repository: AndyTechnologies/wasm-apps
Length of output: 26023
🌐 Web query:
WASI snapshot preview1 __wasi_filestat_t layout st_dev st_ino st_filetype st_nlink offsets
💡 Result:
In the WASI snapshot preview1 specification, the wasi_filestat_t structure is defined with a total size of 64 bytes and an alignment of 8 bytes [1][2][3]. The layout and field offsets of the structure are as follows [1][2][3]: Field | Type | Offset --- | --- | --- dev | wasi_device_t (u64) | 0 ino | wasi_inode_t (u64) | 8 filetype | wasi_filetype_t (u8, with padding) | 16 nlink | wasi_linkcount_t (u64) | 24 size | wasi_filesize_t (u64) | 32 atim | wasi_timestamp_t (u64) | 40 mtim | wasi_timestamp_t (u64) | 48 ctim | wasi_timestamp_t (u64) | 56 The structure uses 8-byte alignment [1][3]. Notably, while the filetype field is a u8, it is followed by padding to ensure the subsequent field (nlink) aligns correctly at the 24-byte offset [2][4]. This structure is consistently implemented across WASI-compliant environments to maintain binary compatibility [2][5].
Citations:
- 1: https://github.com/WebAssembly/WASI/blob/wasi-0.1/preview1/docs.md
- 2: https://github.com/WebAssembly/wasi-libc/blob/2b7e73ae7ac0bad6391d89cc3274a28412243389/libc-bottom-half/headers/public/wasi/api.h
- 3: https://github.com/WebAssembly/WASI/blob/main/legacy/preview1/docs.md
- 4: https://docs.rs/krasm/latest/src/krasm/runtime/wasi/types.rs.html
- 5: https://docs.rs/ffmpeg-wasi/latest/ffmpeg_wasi/swscale/struct.__wasi_filestat_t.html
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- file header and heap imports ---'
sed -n '1,75p' packages/linker/templates/wasi.as.ts
printf '%s\n' '--- compiler binding header ---'
sed -n '1,75p' packages/compiler/src/bindings/wasi.ts
printf '%s\n' '--- exact duplicated function bodies ---'
diff -u <(sed -n '45,65p;248,265p' packages/linker/templates/wasi.as.ts) <(sed -n '45,65p;248,265p' packages/compiler/src/bindings/wasi.ts) || true
printf '%s\n' '--- stNlink consumers ---'
rg -n 'stNlink' --glob '!packages/linker/templates/wasi.as.ts' --glob '!packages/compiler/src/bindings/wasi.ts' . 2>/dev/null || trueRepository: AndyTechnologies/wasm-apps
Length of output: 5586
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
fields = [
("st_dev", 0, 8),
("st_ino", 8, 8),
("st_filetype", 16, 1),
("st_nlink", 24, 8),
("st_size", 32, 8),
("st_atim", 40, 8),
("st_mtim", 48, 8),
("st_ctim", 56, 8),
]
allocation = 56
print("record_size:", max(offset + size for _, offset, size in fields))
for name, offset, size in fields:
end = offset + size
print(f"{name}: [{offset}, {end})", "OUT_OF_BOUNDS" if end > allocation else "within_allocation")
print("proposed st_nlink conversion loses bits:", "yes" if 8 > 4 else "no")
PYRepository: AndyTechnologies/wasm-apps
Length of output: 509
Use the 64-byte Preview1 filestat layout.
Allocate 64 bytes. Read stFiletype at offset 16, stNlink as i64 at offset 24, and the remaining fields at offsets 32, 40, 48, and 56. Change Filestat.stNlink to i64. Apply the same fix in packages/compiler/src/bindings/wasi.ts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/linker/templates/wasi.as.ts` around lines 251 - 265, Update
pathFilestatGet to use the 64-byte WASI Preview1 filestat layout: allocate 64
bytes, load stNlink as i64 at offset 24, and shift stSize, stAtim, stMtim, and
stCtim to offsets 32, 40, 48, and 56 while keeping stFiletype at 16. Change
Filestat.stNlink to i64, and apply the matching layout and type updates in the
compiler WASI bindings.
| typedef struct { | ||
| uint64_t st_dev; uint64_t st_ino; uint32_t st_filetype; | ||
| uint32_t st_nlink; uint64_t st_size; uint64_t st_atim; | ||
| uint64_t st_mtim; uint64_t st_ctim; | ||
| } __wasi_filestat_t; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- wasi.h structure and call sites ---'
sed -n '1,80p;270,325p' packages/linker/templates/wasi.h
printf '%s\n' '--- related declarations and size assumptions ---'
rg -n -C 3 '__wasi_filestat_t|path_filestat_get|filestat' packages/linker packages | head -240
printf '%s\n' '--- repository metadata ---'
git diff --stat
git status --shortRepository: AndyTechnologies/wasm-apps
Length of output: 23303
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- Rust ABI definitions for comparison ---'
sed -n '50,75p' packages/linker/templates/wasi.rs
sed -n '50,75p' packages/compiler/src/bindings/wasi.rs
printf '%s\n' '--- generation or duplication references ---'
rg -n -C 3 'templates/wasi\.h|bindings/wasi\.h|wasi\.h' packages scripts .github 2>/dev/null | head -240
printf '%s\n' '--- standalone layout probe ---'
python3 - <<'PY'
import ctypes
class Current(ctypes.Structure):
_fields_ = [
('st_dev', ctypes.c_uint64),
('st_ino', ctypes.c_uint64),
('st_filetype', ctypes.c_uint32),
('st_nlink', ctypes.c_uint32),
('st_size', ctypes.c_uint64),
('st_atim', ctypes.c_uint64),
('st_mtim', ctypes.c_uint64),
('st_ctim', ctypes.c_uint64),
]
class Preview1(ctypes.Structure):
_fields_ = [
('st_dev', ctypes.c_uint64),
('st_ino', ctypes.c_uint64),
('st_filetype', ctypes.c_uint8),
('_padding', ctypes.c_uint8 * 7),
('st_nlink', ctypes.c_uint64),
('st_size', ctypes.c_uint64),
('st_atim', ctypes.c_uint64),
('st_mtim', ctypes.c_uint64),
('st_ctim', ctypes.c_uint64),
]
for cls in (Current, Preview1):
print(cls.__name__, 'size=', ctypes.sizeof(cls),
'offsets=', {name: getattr(cls, name).offset for name, _ in cls._fields_})
PYRepository: AndyTechnologies/wasm-apps
Length of output: 3367
🌐 Web query:
WASI snapshot preview1 __wasi_filestat_t layout filetype u8 nlink u64 64 bytes official specification
💡 Result:
In the WASI snapshot preview1 (WASI 0.1) specification, the wasi_filestat_t structure is defined with a total size of 64 bytes and an alignment of 8 bytes [1]. The layout of the structure is as follows: - dev (wasi_device_t): Offset 0, 8 bytes (u64) [1] - ino (wasi_inode_t): Offset 8, 8 bytes (u64) [1] - filetype (wasi_filetype_t): Offset 16, 1 byte (u8) [1] - Padding: Offset 17, 7 bytes [1] - nlink (wasi_linkcount_t): Offset 24, 8 bytes (u64) [1] - size (wasi_filesize_t): Offset 32, 8 bytes (u64) [1] - atim (wasi_timestamp_t): Offset 40, 8 bytes (u64) [1] - mtim (wasi_timestamp_t): Offset 48, 8 bytes (u64) [1] - ctim (wasi_timestamp_t): Offset 56, 8 bytes (u64) [1] While the filetype is a 1-byte u8 field, the following nlink field (a 64-bit unsigned integer) is aligned to an 8-byte boundary, resulting in 7 bytes of implicit padding between filetype and nlink to ensure correct alignment [1]. The total structure size is strictly 64 bytes [1]. This definition is explicitly verified in wasi-libc using static assertions for size, alignment, and individual member offsets [1].
Citations:
Correct __wasi_filestat_t to the 64-byte Preview1 layout.
Use u8 for st_filetype, seven padding bytes, and u64 for st_nlink. Apply the same ABI correction to the matching C and Rust definitions, and cast st_filetype in Rust callers that return u32. The current 56-byte objects cannot safely receive the 64-byte path_filestat_get result.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/linker/templates/wasi.h` around lines 32 - 36, Update
__wasi_filestat_t and its matching C and Rust definitions to use an 8-bit
st_filetype, seven bytes of padding, and a 64-bit st_nlink, producing the
required 64-byte Preview1 layout. Update Rust callers that return st_filetype as
u32 to cast the field explicitly, while preserving the existing
path_filestat_get behavior.
| inline int path_open(int dir_fd, const char *path, size_t path_len, int oflags) { | ||
| uint32_t flags = 0; | ||
| uint64_t rights_base = 0; | ||
| if (oflags & 0x01) { rights_base |= 0x0000000000000001ULL; } // O_READ -> rights only (WASI flag 0x01 is O_CREAT) | ||
| if (oflags & 0x02) { rights_base |= 0x0000000000000002ULL; } // O_WRITE -> rights only (WASI flag 0x02 is O_DIRECTORY) | ||
| if (oflags & 0x40) { flags |= 0x01; rights_base |= 0x0000000000000004ULL; } // O_CREAT -> WASI O_CREAT | ||
| if (oflags & 0x200) { flags |= 0x08; } // O_TRUNC -> WASI O_TRUNC | ||
| int32_t opened_fd = -1; | ||
| int err = __wasi_path_open(dir_fd, 0, path, path_len, flags, | ||
| rights_base, rights_base, 0, &opened_fd); | ||
| return err ? -err : (int)opened_fd; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- packages/linker/templates/wasi.h ---'
sed -n '235,285p' packages/linker/templates/wasi.h
printf '%s\n' '--- packages/linker/templates/fs.h ---'
sed -n '60,110p' packages/linker/templates/fs.h
printf '%s\n' '--- related WASI constants and path_open callers ---'
rg -n -C 2 'path_open|FD_DATASYNC|FD_READ|FD_SEEK|FD_WRITE|RIGHTS_|O_READ|O_WRITE|O_CREAT|O_TRUNC' packages/linker/templatesRepository: AndyTechnologies/wasm-apps
Length of output: 12278
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
source = Path("packages/linker/templates/wasi.h").read_text()
calls = Path("packages/linker/templates/fs.h").read_text()
body = source[source.index("inline int path_open"):source.index("inline int path_filestat_get")]
mapping = {}
for mask, bit in re.findall(r'oflags & (0x[0-9a-fA-F]+).*?rights_base \|= (0x[0-9a-fA-F]+)', body):
mapping[int(mask, 16)] = int(bit, 16)
print("path_open mappings:", {hex(k): hex(v) for k, v in mapping.items()})
for label, oflags in (
("read", 0x01),
("write/create/truncate", 0x02 | 0x40 | 0x200),
):
rights = 0
for mask, bit in mapping.items():
if oflags & mask:
rights |= bit
print(label, "oflags=", hex(oflags), "rights_base=", hex(rights))
print(" FD_READ:", bool(rights & (1 << 1)))
print(" FD_WRITE:", bool(rights & (1 << 6)))
print(" FD_SEEK:", bool(rights & (1 << 2)))
print(" FD_FILESTAT_SET_SIZE:", bool(rights & (1 << 10)))
PY
printf '%s\n' '--- all path_open-related C++ constants/usages ---'
rg -n -C 3 'rights_base|fd_write|fd_read|path_open|0x200|0x40' packages/linker/templates --glob '*.h'Repository: AndyTechnologies/wasm-apps
Length of output: 7879
🌐 Web query:
WASI Preview1 path_open rights_base O_TRUNC FD_FILESTAT_SET_SIZE FD_WRITE FD_READ PATH_CREATE_FILE specification
💡 Result:
In WASI Preview 1, path_open is the primary function for opening files and directories, utilizing a capability-based security model where file descriptor access is governed by explicit rights [1][2]. The function signature is: path_open(fd, dirflags, path, oflags, fs_rights_base, fs_rights_inheriting, fdflags) -> Fd [3][4] Regarding the specific parameters and rights: 1. fs_rights_base: Defines the rights that apply to the newly opened file descriptor itself [3][5]. For operations involving FD_WRITE or O_TRUNC, specific rights must be present [6]. 2. O_TRUNC (in oflags): When using O_TRUNC to truncate a file, the implementation typically requires the file descriptor to be opened with write access [6]. Historically, this necessitated both FD_WRITE and FD_PATH_FILESTAT_SET_SIZE rights [6]. Because many host operating systems require write access to truncate a file, implementations often enforce that the file descriptor is opened for writing if O_TRUNC is specified [6]. 3. FD_WRITE and FD_READ: These rights are essential for standard I/O [1]. When calling path_open, the application must explicitly provide the necessary rights in fs_rights_base [7][8]. Implementations often use these rights to determine the underlying access mode (e.g., O_RDONLY, O_WRONLY, O_RDWR) for the host file descriptor [7][8]. If neither is provided, some platforms have historically defaulted to read-only access for technical reasons, though this behavior can vary [9]. 4. FD_FILESTAT_SET_SIZE: This right allows the caller to modify the size of the file (e.g., via fd_filestat_set_size) [10]. It is often bundled with requirements for truncation (O_TRUNC) depending on the specific WASI runtime implementation's enforcement of host-level constraints [6]. 5. PATH_CREATE_FILE (often associated with oflags::creat): To create a file using path_open, the capability right PATH_OPEN must be set, which includes the right to invoke path_open with oflags::creat [10]. In summary, WASI Preview 1's capability system requires callers to explicitly grant the necessary rights (FD_READ, FD_WRITE, etc.) when calling path_open [1][7]. If a required right (like FD_WRITE for O_TRUNC) is missing, the operation will typically fail, as the sandbox denies the action [6][8]. These rights are distinct from "inheriting rights," which are rights applied to new file descriptors derived from the current one [3][5].
Citations:
- 1: https://github.com/bytecodealliance/wasi.dev/blob/main/docs/releases/wasi-p1.md
- 2: https://github.com/WebAssembly/WASI/tree/main/proposals/filesystem
- 3: https://docs.rs/wasip1/latest/wasip1/fn.path_open.html
- 4: https://docs.wasmtime.dev/api/wasi_common/snapshots/preview_1/wasi_snapshot_preview1/fn.path_open.html
- 5: How does
fs_rights_inheritingwork inpath_open? WebAssembly/wasi-filesystem#23 - 6: Do not allow truncating files which are not opened for writing. bytecodealliance/wasmtime#570
- 7: https://github.com/WebAssembly/wasi-libc/blob/master/libc-bottom-half/cloudlibc/src/libc/fcntl/openat.c
- 8: [9.0.0] fix Wasi rights system to work with wasi-libc bytecodealliance/wasmtime#6462
- 9: Implicit "read" access when no rights specified in path_open on Unix bytecodealliance/wasmtime#617
- 10: https://github.com/WebAssembly/WASI/blob/wasi-0.1/preview1/docs.md
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import urllib.request
url = "https://raw.githubusercontent.com/WebAssembly/WASI/wasi-0.1/preview1/docs.md"
text = urllib.request.urlopen(url, timeout=10).read().decode()
for term in ("RIGHTS_FD_READ", "RIGHTS_FD_WRITE", "RIGHTS_FD_FILESTAT_SET_SIZE",
"RIGHTS_PATH_CREATE_FILE", "O_TRUNC", "path_open"):
print(f"\n--- {term} ---")
lines = text.splitlines()
for i, line in enumerate(lines):
if term in line:
print("\n".join(f"{j+1}: {lines[j]}" for j in range(max(0, i-3), min(len(lines), i+8))))
PY
printf '%s\n' '--- path resolution and directory descriptor flow ---'
sed -n '1,75p' packages/linker/templates/fs.hRepository: AndyTechnologies/wasm-apps
Length of output: 2450
🌐 Web query:
WASI preview1 docs path_open ENOTCAPABLE O_TRUNC FD_FILESTAT_SET_SIZE RIGHTS_PATH_CREATE_FILE exact
💡 Result:
In WASI preview1, the interplay between path_open, O_TRUNC, and file rights was a core part of its capability-based security model, though it has been deprecated in later WASI iterations [1][2][3]. Under WASI preview1 semantics: 1. ENOTCAPABLE: This error was traditionally returned when a file descriptor lacked the specific rights required to perform a requested operation [3]. In the context of path_open, if an application attempted to open a file with flags (like O_TRUNC) or base/inheriting rights that the directory descriptor or the target file did not support, the implementation would typically return ENOTCAPABLE [4][3]. 2. O_TRUNC and Rights: - To use O_TRUNC during path_open, the caller is required to possess the RIGHT_PATH_FILESTAT_SET_SIZE right [5][6][4]. - Historically, some implementations required RIGHT_FD_WRITE in addition to or instead of RIGHT_PATH_FILESTAT_SET_SIZE to perform the truncation, as truncation is a destructive operation [5][6]. - If a caller provided O_TRUNC but failed to provide the necessary RIGHT_PATH_FILESTAT_SET_SIZE in the fs_rights_base parameter of path_open, the call would fail, often with ENOTCAPABLE or a similar access-denied error depending on the specific runtime implementation [5][6]. 3. Evolution: The rights system in WASI preview1—specifically the distinction between base and inheriting rights and the use of ENOTCAPABLE—was found to be overly complex and inconsistent with POSIX and other host platforms [1][2][7]. Consequently, the rights system was removed in later proposals (such as wasi-filesystem), shifting toward a model that relies more on standard POSIX-like access modes [1][2][3]. In summary, path_open with O_TRUNC requires the caller to demonstrate authority via RIGHT_PATH_FILESTAT_SET_SIZE. Lacking this specific right while requesting O_TRUNC would result in an error, conceptually aligned with the preview1 ENOTCAPABLE semantics [5][6][4].
Citations:
- 1: Remove the rights system WebAssembly/wasi-filesystem#31
- 2: https://github.com/WebAssembly/WASI/tree/main/proposals/filesystem
- 3: Stop returning
NOTCAPABLEerrors from WASI calls. bytecodealliance/wasmtime#4666 - 4: https://github.com/WebAssembly/WASI/blob/wasi-0.1/preview1/docs.md
- 5: Do not allow truncating files which are not opened for writing. bytecodealliance/wasmtime#570
- 6: bytecodealliance/wasmtime@049c926
- 7: How does
fs_rights_inheritingwork inpath_open? WebAssembly/wasi-filesystem#23
🏁 Script executed:
#!/bin/bash
set -eu
url='https://raw.githubusercontent.com/WebAssembly/WASI/wasi-0.1/preview1/docs.md'
curl -ksSfL "$url" | rg -n -C 8 \
'RIGHTS_FD_READ|RIGHTS_FD_WRITE|RIGHTS_FD_FILESTAT_SET_SIZE|RIGHTS_PATH_FILESTAT_SET_SIZE|RIGHTS_PATH_CREATE_FILE|O_TRUNC|path_open'Repository: AndyTechnologies/wasm-apps
Length of output: 9958
Map path_open access flags to the correct WASI rights.
rights_base is 0x1 for reads and 0x6 for writes. These values omit FD_READ (1 << 1) and FD_WRITE (1 << 6), so fd_read and fd_write can return ENOTCAPABLE. Map O_READ to 0x2 and O_WRITE to 0x40. Keep O_CREAT and O_TRUNC in flags. Ensure dir_fd has PATH_CREATE_FILE for O_CREAT and PATH_FILESTAT_SET_SIZE for O_TRUNC.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/linker/templates/wasi.h` around lines 262 - 273, Update path_open so
O_READ contributes WASI FD_READ (0x2) and O_WRITE contributes FD_WRITE (0x40) to
rights_base, while preserving O_CREAT and O_TRUNC in flags. When O_CREAT is set,
include PATH_CREATE_FILE in the directory rights; when O_TRUNC is set, include
PATH_FILESTAT_SET_SIZE, and pass the resulting rights to __wasi_path_open.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/compiler/src/bindings/fs.h`:
- Around line 25-62: Update the preopened-directory lookup in
packages/compiler/src/bindings/fs.h lines 25-62 and
packages/linker/templates/fs.h lines 24-61 to scan all descriptors and retain
the matching descriptor with the longest directory name, rather than returning
the first match. Apply the same selection logic to both matching branches,
including exact mount-point matches, then return the most-specific descriptor
and relative offset after the scan.
In `@packages/compiler/src/bindings/wasi.rs`:
- Around line 165-166: Remove the duplicate declarations in the WASI bindings,
keeping exactly one declaration with its implementation for each of fd_write and
exit. Ensure neither function remains as a body-less duplicate so the module
parses and compiles.
- Around line 380-382: Update the WASI rights constants in all four binding and
template files so RIGHTS_READ is 0x02 and RIGHTS_WRITE is 0x40, preserving the
existing constant names and declarations.
- Around line 56-65: Update __wasi_filestat_t to match the 64-byte WASI Preview1
layout: change st_filetype to u8, add 7 bytes of padding, and change st_nlink to
u64. Apply the identical layout changes to the mirrored WASI bindings, and cast
st_filetype to u32 in stat_file wherever it is consumed.
In `@packages/compiler/src/strategies/rust-strategy.ts`:
- Around line 124-137: Update the Cargo.lock backup and cleanup flow around the
existing manifest handling so Cargo.lock is snapshotted independently of
hasCargoToml before the build. Restore the snapshot when one existed; otherwise
remove any generated lockfile when cleaning up a temporary Cargo.toml, including
the corresponding path around the later cleanup logic. Add a lifecycle test
covering a source directory without Cargo.toml that verifies the generated
Cargo.lock is removed.
In `@packages/linker/templates/fs.rs`:
- Around line 76-77: Remove the bodyless duplicate declarations preceding
read_file and mkdir, keeping only each function’s complete definition with its
body so the Rust template parses correctly.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e04a178-d533-4c63-a55a-218c43f8d366
⛔ Files ignored due to path filters (2)
examples/rust-fs/src/Cargo.lockis excluded by!**/*.lockexamples/rust-hello/src/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (49)
.gitignore.prettierignoredata/greeting.txtexamples/as-fs/expected-stdout.txtexamples/as-fs/src/main.wasm.tsexamples/cpp-saludo/src/main.wasm.cppexamples/mounts-demo/expected-stdout.txtexamples/mounts-demo/src/main.wasm.cppexamples/rust-fs/expected-stdout.txtexamples/rust-fs/src/Cargo.tomlexamples/rust-fs/src/main.wasm.rsexamples/rust-hello/src/main.wasm.rsopenspec/changes/bindings-auto-injection/design.mdopenspec/changes/bindings-auto-injection/proposal.mdopenspec/changes/bindings-auto-injection/specs/bindings-auto-injection/spec.mdopenspec/changes/bindings-auto-injection/tasks.mdpackages/compiler/package.jsonpackages/compiler/scripts/copy-bindings.mjspackages/compiler/src/bindings/console.hpackages/compiler/src/bindings/console.rspackages/compiler/src/bindings/console.tspackages/compiler/src/bindings/fs.hpackages/compiler/src/bindings/fs.rspackages/compiler/src/bindings/fs.tspackages/compiler/src/bindings/rust/Cargo.tomlpackages/compiler/src/bindings/rust/src/alloc.rspackages/compiler/src/bindings/rust/src/console.rspackages/compiler/src/bindings/rust/src/fs.rspackages/compiler/src/bindings/rust/src/lib.rspackages/compiler/src/bindings/rust/src/wasi.rspackages/compiler/src/bindings/wasi.hpackages/compiler/src/bindings/wasi.rspackages/compiler/src/bindings/wasi.tspackages/compiler/src/index.test.tspackages/compiler/src/index.tspackages/compiler/src/strategies/cpp-strategy.tspackages/compiler/src/strategies/rust-strategy.test.tspackages/compiler/src/strategies/rust-strategy.tspackages/compiler/tsconfig.jsonpackages/linker/templates/console.as.tspackages/linker/templates/console.hpackages/linker/templates/console.rspackages/linker/templates/fs.as.tspackages/linker/templates/fs.hpackages/linker/templates/fs.rspackages/linker/templates/wasi.as.tspackages/linker/templates/wasi.hpackages/linker/templates/wasi.rsscripts/test-examples.mjs
🚧 Files skipped from review as they are similar to previous changes (36)
- examples/rust-fs/expected-stdout.txt
- examples/as-fs/expected-stdout.txt
- packages/compiler/tsconfig.json
- examples/mounts-demo/expected-stdout.txt
- packages/compiler/scripts/copy-bindings.mjs
- openspec/changes/bindings-auto-injection/proposal.md
- examples/rust-fs/src/main.wasm.rs
- .prettierignore
- examples/cpp-saludo/src/main.wasm.cpp
- examples/rust-fs/src/Cargo.toml
- packages/compiler/src/bindings/console.rs
- data/greeting.txt
- packages/compiler/src/bindings/rust/Cargo.toml
- packages/compiler/src/bindings/rust/src/lib.rs
- packages/compiler/src/strategies/cpp-strategy.ts
- examples/rust-hello/src/main.wasm.rs
- packages/compiler/src/bindings/rust/src/alloc.rs
- packages/compiler/src/bindings/rust/src/console.rs
- packages/linker/templates/console.rs
- packages/compiler/package.json
- examples/as-fs/src/main.wasm.ts
- packages/linker/templates/console.as.ts
- examples/mounts-demo/src/main.wasm.cpp
- packages/compiler/src/bindings/console.ts
- packages/linker/templates/wasi.h
- packages/linker/templates/fs.as.ts
- packages/compiler/src/bindings/wasi.h
- .gitignore
- openspec/changes/bindings-auto-injection/tasks.md
- packages/compiler/src/bindings/rust/src/wasi.rs
- scripts/test-examples.mjs
- packages/linker/templates/wasi.rs
- packages/compiler/src/bindings/rust/src/fs.rs
- openspec/changes/bindings-auto-injection/specs/bindings-auto-injection/spec.md
- packages/compiler/src/bindings/fs.ts
- packages/compiler/src/bindings/fs.rs
| for (int fd = 3; fd <= 63; fd++) { | ||
| // Get prestat to find dir name length | ||
| __wasi_prestat_t prestat; | ||
| int err = __wasi_fd_prestat_get(fd, &prestat); | ||
| if (err) continue; | ||
|
|
||
| // Get dir name | ||
| char* dirname = (char*)__builtin_alloca(prestat.pr_name_len + 1); | ||
| err = __wasi_fd_prestat_dir_name(fd, dirname, prestat.pr_name_len); | ||
| if (err) continue; | ||
|
|
||
| size_t dir_len = prestat.pr_name_len; | ||
| // Trim trailing slash from dirname | ||
| while (dir_len > 0 && dirname[dir_len - 1] == '/') --dir_len; | ||
|
|
||
| // Check if path starts with dirname + '/' | ||
| if (path_len > dir_len && path[dir_len] == '/') { | ||
| bool match = true; | ||
| for (size_t i = 0; i < dir_len; i++) { | ||
| if (path[i] != dirname[i]) { match = false; break; } | ||
| } | ||
| if (match) { | ||
| *out_dir_fd = fd; | ||
| *out_rel_offset = dir_len + 1; // skip '/' separator | ||
| return 0; | ||
| } | ||
| } else if (path_len == dir_len) { | ||
| // Exact match — path IS the mount point (e.g. "/mnt" = root) | ||
| bool match = true; | ||
| for (size_t i = 0; i < dir_len; i++) { | ||
| if (path[i] != dirname[i]) { match = false; break; } | ||
| } | ||
| if (match) { | ||
| *out_dir_fd = fd; | ||
| *out_rel_offset = path_len; // relative part is empty | ||
| return 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Select the most-specific matching preopened directory.
The first matching descriptor can be a parent mount. For /mnt/data/file, a lower-numbered /mnt descriptor can win before /mnt/data. This can access the wrong mount or fail when the parent does not contain the child mount path.
packages/compiler/src/bindings/fs.h#L25-L62: scan all preopens and retain the matching descriptor with the longest directory name.packages/linker/templates/fs.h#L24-L61: apply the same longest-prefix selection logic.
📍 Affects 2 files
packages/compiler/src/bindings/fs.h#L25-L62(this comment)packages/linker/templates/fs.h#L24-L61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/bindings/fs.h` around lines 25 - 62, Update the
preopened-directory lookup in packages/compiler/src/bindings/fs.h lines 25-62
and packages/linker/templates/fs.h lines 24-61 to scan all descriptors and
retain the matching descriptor with the longest directory name, rather than
returning the first match. Apply the same selection logic to both matching
branches, including exact mount-point matches, then return the most-specific
descriptor and relative offset after the scan.
| pub struct __wasi_filestat_t { | ||
| pub st_dev: u64, | ||
| pub st_ino: u64, | ||
| pub st_filetype: u32, | ||
| pub st_nlink: u32, | ||
| pub st_size: u64, | ||
| pub st_atim: u64, | ||
| pub st_mtim: u64, | ||
| pub st_ctim: u64, | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'struct __wasi_filestat_t|st_filetype|st_nlink|st_size|st_atim|st_mtim|st_ctim' \
packages/compiler/src/bindings/wasi.rs \
packages/linker/templates/wasi.rs \
packages/compiler/src/bindings/wasi.ts \
packages/linker/templates/wasi.as.ts
python3 - <<'PY'
fields = [
("dev", 0), ("ino", 8), ("filetype", 16), ("nlink", 24),
("size", 32), ("atim", 40), ("mtim", 48), ("ctim", 56),
]
assert fields[-1][1] + 8 == 64
print("Expected WASI Preview1 filestat size: 64 bytes")
PYRepository: AndyTechnologies/wasm-apps
Length of output: 4078
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- compiler binding ---'
sed -n '1,90p;450,525p' packages/compiler/src/bindings/wasi.rs
printf '%s\n' '--- filestat call sites and implementation references ---'
rg -n -C 5 'path_filestat_get|__wasi_filestat_t|st_filetype|st_nlink' packages
printf '%s\n' '--- template consistency ---'
sed -n '45,75p;475,495p' packages/linker/templates/wasi.rs
printf '%s\n' '--- duplicate declarations mentioned in context ---'
rg -n -C 2 'pub fn (fd_write|exit)\b|fn (fd_write|exit)\b' packages/compiler/src/bindings/wasi.rs packages/linker/templates/wasi.rsRepository: AndyTechnologies/wasm-apps
Length of output: 41982
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- binding files and generation references ---'
fd -i 'wasi\.(rs|h|ts)$' packages
rg -n -C 3 'bindings/(rust/)?src/wasi\.rs|templates/wasi\.rs|wasi\.rs' packages --glob '!**/wasi.rs' --glob '!**/wasi.h' --glob '!**/wasi.ts'
printf '%s\n' '--- deterministic layout calculation ---'
python3 - <<'PY'
# repr(C) field placement for the definitions present in the repository.
rust_fields = [
("st_dev", 8), ("st_ino", 8), ("st_filetype", 4), ("st_nlink", 4),
("st_size", 8), ("st_atim", 8), ("st_mtim", 8), ("st_ctim", 8),
]
wasi_fields = [
("st_dev", 8), ("st_ino", 8), ("st_filetype", 1), ("padding", 7),
("st_nlink", 8), ("st_size", 8), ("st_atim", 8), ("st_mtim", 8),
("st_ctim", 8),
]
def layout(fields):
offset = 0
result = []
for name, size in fields:
alignment = min(size, 8)
offset += (-offset) % alignment
result.append((name, offset, size))
offset += size
return result, offset
for label, fields in [("repository Rust", rust_fields), ("WASI Preview1", wasi_fields)]:
entries, size = layout(fields)
print(label, "size", size)
for entry in entries:
print(" ", entry)
PYRepository: AndyTechnologies/wasm-apps
Length of output: 443
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- binding files and generation references ---'
fd -i 'wasi\.(rs|h|ts)$' packages
rg -n -C 3 'bindings/(rust/)?src/wasi\.rs|templates/wasi\.rs|wasi\.rs' packages --glob '!**/wasi.rs' --glob '!**/wasi.h' --glob '!**/wasi.ts'
printf '%s\n' '--- deterministic layout calculation ---'
python3 - <<'PY'
rust_fields = [
("st_dev", 8), ("st_ino", 8), ("st_filetype", 4), ("st_nlink", 4),
("st_size", 8), ("st_atim", 8), ("st_mtim", 8), ("st_ctim", 8),
]
wasi_fields = [
("st_dev", 8), ("st_ino", 8), ("st_filetype", 1), ("padding", 7),
("st_nlink", 8), ("st_size", 8), ("st_atim", 8), ("st_mtim", 8),
("st_ctim", 8),
]
def layout(fields):
offset = 0
result = []
for name, size in fields:
alignment = min(size, 8)
offset += (-offset) % alignment
result.append((name, offset, size))
offset += size
return result, offset
for label, fields in [("repository Rust", rust_fields), ("WASI Preview1", wasi_fields)]:
entries, size = layout(fields)
print(label, "size", size)
for entry in entries:
print(" ", entry)
PYRepository: AndyTechnologies/wasm-apps
Length of output: 443
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- deterministic layout calculation ---'
python3 - <<'PY'
rust_fields = [
("st_dev", 8), ("st_ino", 8), ("st_filetype", 4), ("st_nlink", 4),
("st_size", 8), ("st_atim", 8), ("st_mtim", 8), ("st_ctim", 8),
]
wasi_fields = [
("st_dev", 8), ("st_ino", 8), ("st_filetype", 1), ("padding", 7),
("st_nlink", 8), ("st_size", 8), ("st_atim", 8), ("st_mtim", 8),
("st_ctim", 8),
]
def layout(fields):
offset = 0
result = []
for name, size in fields:
alignment = min(size, 8)
offset += (-offset) % alignment
result.append((name, offset, size))
offset += size
return result, offset
for label, fields in [("repository Rust", rust_fields), ("WASI Preview1", wasi_fields)]:
entries, size = layout(fields)
print(label, "size", size)
for entry in entries:
print(" ", entry)
PY
printf '%s\n' '--- package metadata references ---'
for file in $(fd -i 'package.json|Cargo.toml|Makefile|README.md' packages); do
rg -n -C 2 'wasi|bindings|templates' "$file" || true
doneRepository: AndyTechnologies/wasm-apps
Length of output: 3460
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
current = [
("st_dev", 8, 8), ("st_ino", 8, 8), ("st_filetype", 4, 4),
("st_nlink", 4, 4), ("st_size", 8, 8), ("st_atim", 8, 8),
("st_mtim", 8, 8), ("st_ctim", 8, 8),
]
proposed = [
("st_dev", 8, 8), ("st_ino", 8, 8), ("st_filetype", 1, 1),
("padding", 7, 1), ("st_nlink", 8, 8), ("st_size", 8, 8),
("st_atim", 8, 8), ("st_mtim", 8, 8), ("st_ctim", 8, 8),
]
expected = [
("st_dev", 0, 8), ("st_ino", 8, 8), ("st_filetype", 16, 1),
("padding", 17, 7), ("st_nlink", 24, 8), ("st_size", 32, 8),
("st_atim", 40, 8), ("st_mtim", 48, 8), ("st_ctim", 56, 8),
]
def layout(fields):
offset = 0
result = []
for name, size, alignment in fields:
offset += (-offset) % alignment
result.append((name, offset, size))
offset += size
return result, offset
for label, fields in [("current", current), ("proposed", proposed)]:
actual, size = layout(fields)
print(label, "size", size)
print(actual)
actual, size = layout(proposed)
assert actual == expected
assert size == 64
print("Proposed layout matches the 64-byte WASI Preview1 layout.")Repository: AndyTechnologies/wasm-apps
Length of output: 686
Use the 64-byte WASI Preview1 filestat layout.
__wasi_filestat_t is 56 bytes, but path_filestat_get writes 64 bytes. This can overwrite adjacent linear memory. Use u8 for st_filetype, add [u8; 7] padding, use u64 for st_nlink, and cast st_filetype to u32 in stat_file. Apply the same layout to the mirrored WASI bindings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/bindings/wasi.rs` around lines 56 - 65, Update
__wasi_filestat_t to match the 64-byte WASI Preview1 layout: change st_filetype
to u8, add 7 bytes of padding, and change st_nlink to u64. Apply the identical
layout changes to the mirrored WASI bindings, and cast st_filetype to u32 in
stat_file wherever it is consumed.
| pub fn fd_write(fd: i32, data: &[u8]) -> Result<usize, WasiError> { | ||
| pub fn fd_write(fd: i32, data: &[u8]) -> Result<usize, WasiError> { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the duplicate function declarations.
Each first declaration has no body. Rust stops parsing before it can compile this binding. Keep one declaration for fd_write and one declaration for exit.
Also applies to: 621-622
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/bindings/wasi.rs` around lines 165 - 166, Remove the
duplicate declarations in the WASI bindings, keeping exactly one declaration
with its implementation for each of fd_write and exit. Ensure neither function
remains as a body-less duplicate so the module parses and compiles.
| const RIGHTS_READ: u64 = 0x0000000000000001; | ||
| const RIGHTS_WRITE: u64 = 0x0000000000000002; | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 2 'RIGHTS_(READ|WRITE)|path_open|pathOpen' \
packages/compiler/src/bindings/wasi.rs \
packages/linker/templates/wasi.rs \
packages/compiler/src/bindings/wasi.ts \
packages/linker/templates/wasi.as.ts \
packages/linker/templates/wasi.hRepository: AndyTechnologies/wasm-apps
Length of output: 11298
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
files = [
Path("packages/compiler/src/bindings/wasi.rs"),
Path("packages/compiler/src/bindings/wasi.ts"),
Path("packages/linker/templates/wasi.rs"),
Path("packages/linker/templates/wasi.as.ts"),
]
rights = {
0x01: "fd_datasync",
0x02: "fd_read",
0x04: "fd_seek",
0x08: "fd_fdstat_set_flags",
0x10: "fd_sync",
0x20: "fd_tell",
0x40: "fd_write",
}
for path in files:
text = path.read_text()
values = {}
for line in text.splitlines():
if "const RIGHTS_READ" in line or "const RIGHTS_WRITE" in line:
name, value = line.split(":", 1)[0].strip(), line.split("=", 1)[1].split(";", 1)[0].strip()
values[name] = int(value, 0)
print(path)
for name in ("const RIGHTS_READ", "const RIGHTS_WRITE"):
value = values.get(name)
print(f" {name} = {value:`#x`} ({rights.get(value, 'unknown')})")
PY
printf '\nRelevant call-site behavior:\n'
sed -n '407,425p' packages/compiler/src/bindings/wasi.rs
sed -n '228,242p' packages/compiler/src/bindings/wasi.tsRepository: AndyTechnologies/wasm-apps
Length of output: 1722
🌐 Web query:
WASI snapshot preview1 rights fd_datasync fd_read fd_write numeric values
💡 Result:
In WASI snapshot preview1, rights are defined as 64-bit bitflags [1][2][3]. The numeric values for fd_datasync, fd_read, and fd_write correspond to specific bits in this bitmask [1][3]. The numeric values are as follows: - RIGHTS_FD_DATASYNC: 1 << 0, which equals 1 (0x0000000000000001) [4][1][3]. - RIGHTS_FD_READ: 1 << 1, which equals 2 (0x0000000000000002) [4][1][3]. - RIGHTS_FD_WRITE: 1 << 6, which equals 64 (0x0000000000000040) [4][1][3]. These rights determine which operations are permitted on a file descriptor [2]. Specifically: - RIGHTS_FD_DATASYNC: Grants the right to invoke fd_datasync [5][2]. - RIGHTS_FD_READ: Grants the right to invoke fd_read and sock_recv [5][2]. If rights::fd_seek is also set, it includes the right to invoke fd_pread [5][2]. - RIGHTS_FD_WRITE: Grants the right to invoke fd_write and sock_send [5][2]. If rights::fd_seek is also set, it includes the right to invoke fd_pwrite [5][2].
Citations:
- 1: https://docs.rs/krasm/latest/src/krasm/runtime/wasi/types.rs.html
- 2: https://github.com/WebAssembly/WASI/blob/main/legacy/preview1/docs.md
- 3: https://docs.rs/wasip1/latest/src/wasip1/lib_generated.rs.html
- 4: https://deno.land/std@0.206.0/wasi/snapshot_preview1.ts
- 5: https://wasi.bchlr.de/wasihost_core/wasi_snapshot_preview1/struct.Rights.html
Correct the WASI descriptor rights.
Set RIGHTS_READ to 0x02 and RIGHTS_WRITE to 0x40 in all four WASI binding and template files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/bindings/wasi.rs` around lines 380 - 382, Update the
WASI rights constants in all four binding and template files so RIGHTS_READ is
0x02 and RIGHTS_WRITE is 0x40, preserving the existing constant names and
declarations.
| const cargoLockPath = path.join(sourceDir, 'Cargo.lock'); | ||
| const hasCargoToml = fs.existsSync(cargoTomlPath); | ||
| const isWin32 = process.platform === 'win32'; | ||
|
|
||
| const release = options.compilerOptions?.release ?? false; | ||
| const buildProfile = release ? 'release' : 'debug'; | ||
|
|
||
| // If no Cargo.toml, create a temporary one with cdylib crate type | ||
| let createdTempCargo = false; | ||
| if (!hasCargoToml) { | ||
| const tempCargoContent = this.generateTempCargoToml(options.fileName); | ||
| fs.writeFileSync(cargoTomlPath, tempCargoContent, 'utf-8'); | ||
| createdTempCargo = true; | ||
| // Existing manifest: backup bytes (Cargo.toml + Cargo.lock si existen) → inyectar (D3) | ||
| let backupToml: Buffer | null = null; | ||
| let backupLock: Buffer | null = null; | ||
| if (hasCargoToml) { | ||
| backupToml = fs.readFileSync(cargoTomlPath); | ||
| if (fs.existsSync(cargoLockPath)) { | ||
| backupLock = fs.readFileSync(cargoLockPath); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Restore or remove Cargo.lock for temporary manifests.
When no Cargo.toml exists, Cargo can create Cargo.lock. This branch removes only the temporary manifest. It leaves the generated lockfile in the source directory.
Snapshot Cargo.lock before the build regardless of hasCargoToml. Restore it if it existed. Otherwise, remove it during temporary-manifest cleanup. Add a lifecycle test for this path.
Proposed fix
- if (hasCargoToml) {
- backupToml = fs.readFileSync(cargoTomlPath);
- if (fs.existsSync(cargoLockPath)) {
- backupLock = fs.readFileSync(cargoLockPath);
- }
- }
+ if (hasCargoToml) {
+ backupToml = fs.readFileSync(cargoTomlPath);
+ }
+ if (fs.existsSync(cargoLockPath)) {
+ backupLock = fs.readFileSync(cargoLockPath);
+ }
...
- } else if (fs.existsSync(cargoTomlPath)) {
- // Limpiar el Cargo.toml temporal que creamos
+ } else {
fs.rmSync(cargoTomlPath, { force: true });
+ if (backupLock) fs.writeFileSync(cargoLockPath, backupLock);
+ else fs.rmSync(cargoLockPath, { force: true });
}Also applies to: 202-205
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 134-134: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(cargoTomlPath)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 136-136: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(cargoLockPath)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/src/strategies/rust-strategy.ts` around lines 124 - 137,
Update the Cargo.lock backup and cleanup flow around the existing manifest
handling so Cargo.lock is snapshotted independently of hasCargoToml before the
build. Restore the snapshot when one existed; otherwise remove any generated
lockfile when cleaning up a temporary Cargo.toml, including the corresponding
path around the later cleanup logic. Add a lifecycle test covering a source
directory without Cargo.toml that verifies the generated Cargo.lock is removed.
…licadas, no_mangle, cast pr_name_len)
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/how-to/configure-project.md`:
- Line 21: Update the mounts examples so WASI is enabled alongside each mount
configuration: set "wasi" to true in docs/how-to/configure-project.md lines
21-21 and docs/reference/config.md lines 13-13, or remove the mounts entries if
WASI should remain disabled.
In `@packages/linker/src/index.ts`:
- Around line 109-111: Update createNativeApp to validate options.mounts after
reading wasi and before code generation: when mounts is non-empty and wasi is
false, throw ConfigError rather than silently omitting the guest filesystem
configuration. Preserve existing behavior for empty mounts and for WASI-enabled
builds.
In `@packages/linker/src/template-context.test.ts`:
- Line 127: Update the full context fixture in template-context.test.ts to
include entryFunctionName and escapedEntryFunctionName, while retaining mounts,
so the fixture satisfies the NunjucksTemplateContext type.
In `@packages/linker/templates/main.c.njk`:
- Around line 143-145: Update the mounts loop in the generated WASI setup to
check the boolean result of preopen_dir for each mount; when it returns false,
report the corresponding m.host and m.guest paths and exit before reaching
set_wasi, while preserving the existing successful preopen flow.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e41d92f9-09b3-4273-978f-6d4cb7bb9256
📒 Files selected for processing (26)
.prettierignoredocs/how-to/configure-project.mddocs/reference/config.mdexamples/as-fs/data/greeting.txtexamples/as-fs/expected-stdout.txtexamples/mounts-demo/data/greeting.txtexamples/mounts-demo/expected-stdout.txtexamples/rust-fs/data/greeting.txtexamples/rust-fs/expected-stdout.txtexamples/rust-fs/src/main.wasm.rspackages/cli/src/index.tspackages/compiler/src/bindings/fs.rspackages/compiler/src/bindings/rust/src/fs.rspackages/compiler/src/bindings/rust/src/wasi.rspackages/compiler/src/bindings/wasi.rspackages/linker/src/build-cache.tspackages/linker/src/codegen.test.tspackages/linker/src/codegen.tspackages/linker/src/index.tspackages/linker/src/template-context.test.tspackages/linker/src/template-context.tspackages/linker/src/template-renderer.test.tspackages/linker/templates/fs.rspackages/linker/templates/main.c.njkpackages/linker/templates/wasi.rspackages/types/src/index.ts
💤 Files with no reviewable changes (5)
- packages/compiler/src/bindings/fs.rs
- packages/compiler/src/bindings/rust/src/wasi.rs
- packages/compiler/src/bindings/wasi.rs
- packages/linker/templates/fs.rs
- packages/linker/templates/wasi.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/rust-fs/src/main.wasm.rs
- .prettierignore
- examples/as-fs/expected-stdout.txt
| "entry": "_start", | ||
| "moduleMatching": "file-name", | ||
| "wasi": false, | ||
| "mounts": [{ "host": "data", "guest": "/mnt/data" }], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Enable WASI in both mounts examples.
Both examples add mounts while "wasi": false remains in the surrounding JSON. A copied configuration will not enable the WASI preopen required by the mounted filesystem.
- docs/how-to/configure-project.md#L21-L21: set
"wasi": trueor removemounts. - docs/reference/config.md#L13-L13: set
"wasi": trueor removemounts.
📍 Affects 2 files
docs/how-to/configure-project.md#L21-L21(this comment)docs/reference/config.md#L13-L13
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/how-to/configure-project.md` at line 21, Update the mounts examples so
WASI is enabled alongside each mount configuration: set "wasi" to true in
docs/how-to/configure-project.md lines 21-21 and docs/reference/config.md lines
13-13, or remove the mounts entries if WASI should remain disabled.
| export async function createNativeApp(options: NativeAppOptions, quiet = false): Promise<string> { | ||
| const { inputPaths, output, entry, wasi, moduleMatching } = options; | ||
| const mounts = resolveMounts(options.mounts); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject mounts when WASI is disabled.
If options.mounts is non-empty and wasi is false, the template omits every preopen_dir call. The build succeeds with no configured guest filesystem. Throw ConfigError, or enable WASI explicitly, before code generation.
Proposed fix
export async function createNativeApp(options: NativeAppOptions, quiet = false): Promise<string> {
const { inputPaths, output, entry, wasi, moduleMatching } = options;
const mounts = resolveMounts(options.mounts);
+ if (mounts.length > 0 && !wasi) {
+ throw new ConfigError('Los mounts requieren wasi: true.', {
+ code: 'MOUNTS_REQUIRE_WASI',
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function createNativeApp(options: NativeAppOptions, quiet = false): Promise<string> { | |
| const { inputPaths, output, entry, wasi, moduleMatching } = options; | |
| const mounts = resolveMounts(options.mounts); | |
| export async function createNativeApp(options: NativeAppOptions, quiet = false): Promise<string> { | |
| const { inputPaths, output, entry, wasi, moduleMatching } = options; | |
| const mounts = resolveMounts(options.mounts); | |
| if (mounts.length > 0 && !wasi) { | |
| throw new ConfigError('Los mounts requieren wasi: true.', { | |
| code: 'MOUNTS_REQUIRE_WASI', | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/linker/src/index.ts` around lines 109 - 111, Update createNativeApp
to validate options.mounts after reading wasi and before code generation: when
mounts is non-empty and wasi is false, throw ConfigError rather than silently
omitting the guest filesystem configuration. Preserve existing behavior for
empty mounts and for WASI-enabled builds.
| modules: [], | ||
| hostFunctions: [], | ||
| globals: [], | ||
| mounts: [], |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '72,132p' packages/linker/src/template-context.test.ts
rg -n -C 2 'interface NunjucksTemplateContext|mounts: TemplateMountEntry|entryFunctionName|escapedEntryFunctionName' \
packages/linker/src/template-context.tsRepository: AndyTechnologies/wasm-apps
Length of output: 2545
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
test = Path("packages/linker/src/template-context.test.ts").read_text()
source = Path("packages/linker/src/template-context.ts").read_text()
fixture = re.search(
r"it\('accepts a full context'.*?const context: NunjucksTemplateContext = \{(.*?)\n \};",
test,
re.S,
)
interface = re.search(
r"export interface NunjucksTemplateContext \{(.*?)\n\}",
source,
re.S,
)
if not fixture or not interface:
raise SystemExit("Could not locate the fixture or interface")
fixture_fields = set(re.findall(r"^\s{6}([A-Za-z][A-Za-z0-9]*):", fixture.group(1), re.M))
required_fields = set(
re.findall(
r"^\s{2}(?!/)([A-Za-z][A-Za-z0-9]*):\s*(?![?])",
interface.group(1),
re.M,
)
)
print("full fixture fields:", sorted(fixture_fields))
print("required interface fields:", sorted(required_fields))
print("missing fields:", sorted(required_fields - fixture_fields))
PY
sed -n '107,138p' packages/linker/src/template-context.tsRepository: AndyTechnologies/wasm-apps
Length of output: 1738
Complete the full context fixture.
Add entryFunctionName, escapedEntryFunctionName, and mounts to the fixture so it satisfies NunjucksTemplateContext.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/linker/src/template-context.test.ts` at line 127, Update the full
context fixture in template-context.test.ts to include entryFunctionName and
escapedEntryFunctionName, while retaining mounts, so the fixture satisfies the
NunjucksTemplateContext type.
| {%- for m in mounts %} | ||
| wasi_config.preopen_dir("{{ m.host }}", "{{ m.guest }}", {{ m.dirPerms }}, {{ m.filePerms }}); | ||
| {%- endfor %} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'preopen_dir|set_wasi' packages/linker/templates/main.c.njk
fd -a 'wasi.hh' "$HOME/.wasm-linker" 2>/dev/null | head -n 1 | xargs -r rg -n -C 3 'preopen_dir'Repository: AndyTechnologies/wasm-apps
Length of output: 588
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- template context ---'
sed -n '1,190p' packages/linker/templates/main.c.njk
printf '%s\n' '--- related generated-code error handling ---'
rg -n -C 3 '_check_result|preopen_dir|set_wasi|std::cerr|return 1' packages/linker packages --glob '!node_modules' --glob '!dist' --glob '!build' | head -n 240
printf '%s\n' '--- repository metadata and dependency references ---'
rg -n -C 3 'wasmtime|wasi.hh|WasiConfig' . --glob '!node_modules' --glob '!dist' --glob '!build' | head -n 240Repository: AndyTechnologies/wasm-apps
Length of output: 48576
🌐 Web query:
Wasmtime C++ v46 WasiConfig preopen_dir bool failure semantics
💡 Result:
In Wasmtime C++ (and the underlying C API), the preopen_dir method in WasiConfig returns a bool to indicate the success or failure of the configuration operation [1][2][3]. Since Wasmtime 20, the semantics of this operation changed to prioritize eager validation [4]. When preopen_dir is called: 1. Failure Semantics: It returns false if the directory cannot be successfully prepared or validated at the time of the configuration call [4]. This is a departure from older versions of Wasmtime, which previously deferred validation and potential errors until the directory was first accessed by the WASI guest [4]. 2. Usage: Because it returns a bool, callers should explicitly check the return value [4]. Ignoring a false return value means your WASI configuration may be silently misconfigured, potentially leading to errors later when the WASI instance is instantiated or used [4]. 3. Underlying Mechanism: The C++ WasiConfig::preopen_dir is a wrapper around the C API function wasi_config_preopen_dir [3]. Wasmtime updates (specifically around the transition to version 20 and subsequent refinements) moved this logic to use a builder-based approach that ensures directory accessibility is verified during the configuration phase [4][5]. For robust applications, always verify the boolean return value of preopen_dir to ensure the host path provided is valid and accessible before proceeding with instance creation [4].
Citations:
- 1: https://bytecodealliance.github.io/wasmtime-cpp/classwasmtime_1_1_wasi_config.html
- 2: https://docs.wasmtime.dev/c-api/wasi_8h.html
- 3: https://docs.wasmtime.dev/c-api/wasi_8hh_source.html
- 4: Since Wasmtime 20, preopens of non-existent directories fail eagerly rather than on first access bytecodealliance/wasmtime#8552
- 5: Use WASI builder directly in C API bytecodealliance/wasmtime#8572
🌐 Web query:
site:docs.wasmtime.dev/c-api/wasi_8hh_source.html "preopen_dir" "bool" WasiConfig
💡 Result:
The preopen_dir function is a method used within the Wasmtime C API to configure a directory to be preopened and made available to WASI APIs [1]. It is not a boolean value, but rather a function that returns a boolean indicating success or failure [1]. When using the C API, this method requires the host path, the guest path, and permissions for directories and files as arguments [1]. bool preopen_dir(const std::string &path, const std::string &guest_path, size_t dir_perms, size_t file_perms) This function internally calls wasi_config_preopen_dir to apply these configurations to the WASI context [1].
Citations:
Handle preopen_dir failure.
If preopen_dir returns false, report the host and guest paths and stop before set_wasi. Otherwise, the template continues without the requested preopen and guest filesystem access can fail later.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/linker/templates/main.c.njk` around lines 143 - 145, Update the
mounts loop in the generated WASI setup to check the boolean result of
preopen_dir for each mount; when it returns false, report the corresponding
m.host and m.guest paths and exit before reaching set_wasi, while preserving the
existing successful preopen flow.
…ustup target) y añade arch al cache key
…regenera test-examples)
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/as-fs/README.md`:
- Around line 7-14: Declara el lenguaje de los cuatro bloques Markdown sin
lenguaje: en examples/as-fs/README.md, líneas 7-14 y 26-29, y en
examples/cpp-saludo/README.md, líneas 7-12 y 24-26, cambia cada fence de
apertura a ```text para los árboles de directorios y las salidas esperadas.
In `@examples/custom-template/README.md`:
- Around line 7-14: Añade identificadores de lenguaje a todos los bloques
Markdown afectados: usa text en los bloques de estructura de
examples/custom-template/README.md (7-14), examples/mounts-demo/README.md
(7-14), examples/multi-toolchain/README.md (7-13),
examples/precompiled/README.md (7-12) y examples/rust-fs/README.md (7-15); usa
text o console en los bloques de salida de esos mismos archivos, respectivamente
en 26-28, 26-29, 25-28, 24-28 y 27-30.
In `@examples/rust-hello/README.md`:
- Around line 15-21: Update the execution instructions in the rust-hello README
to run wapp setup before wapp build, ensuring the documented flow works on a
fresh installation.
- Around line 7-13: Update both fenced code blocks in the README, including the
blocks around the project tree and the additional block near the referenced
later section, to specify the text language identifier. Preserve their contents
and formatting while adding the language marker required by markdownlint.
In `@packages/cli/README.md`:
- Line 3: Actualiza la documentación multi-toolchain en packages/cli/README.md,
líneas 3-3: describe el pipeline y la opción --source-dir incluyendo todas las
extensiones de fuente soportadas, no solo .wasm.ts. En
packages/compiler/README.md, líneas 1-15, corrige las descripciones de
compileWasm y del CLI para reflejar el soporte ampliado, o indica claramente qué
APIs siguen siendo específicas de AssemblyScript.
In `@packages/compiler/README.md`:
- Around line 9-15: Corrige la fila de AssemblyScript en la tabla de bindings
para que el código inline esté correctamente delimitado y los imports `console`,
`fs` y `wasi` se separen mediante comas, sin usar barras verticales sin escapar
dentro de la celda. Verifica que la tabla mantenga tres columnas y cumpla MD038
y MD056.
In `@packages/linker/README.md`:
- Around line 40-56: Actualiza la firma documentada de generateCCode para
incluir el parámetro opcional mounts?: MountSpec[] y añade mounts al ejemplo
correspondiente, manteniendo consistente la documentación con la configuración
de preopens descrita.
In `@wiki/entities/compiler.md`:
- Line 73: Corrige la fila de AssemblyScript en la tabla para que la lista de
imports no contenga caracteres `|` sin escapar; reescribe los módulos `console`,
`fs` y `wasi` usando una separación sin pipes y conserva las dos columnas de la
tabla.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b5c7817-c8e0-4a9f-ad98-89b175972b6a
📒 Files selected for processing (32)
README.mddocs/explanation/architecture.mddocs/explanation/caching.mddocs/explanation/how-it-works.mddocs/explanation/project-structure.mddocs/how-to/run-integration-tests.mddocs/index.mdexamples/as-fs/README.mdexamples/cpp-saludo/README.mdexamples/custom-template/README.mdexamples/mounts-demo/README.mdexamples/multi-toolchain/README.mdexamples/precompiled/README.mdexamples/rust-fs/README.mdexamples/rust-hello/README.mdpackages/cli/CHANGELOG.mdpackages/cli/README.mdpackages/compiler/CHANGELOG.mdpackages/compiler/README.mdpackages/linker/CHANGELOG.mdpackages/linker/README.mdpackages/types/CHANGELOG.mdpackages/types/README.mdscripts/test-examples.mjswiki/concepts/host-functions.mdwiki/entities/cli.mdwiki/entities/compiler.mdwiki/entities/linker.mdwiki/entities/types.mdwiki/index.mdwiki/log.mdwiki/overview.md
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/test-examples.mjs
| ``` | ||
| as-fs/ | ||
| ├── wapp.json # wasi: true + mounts: [{ host: "data", guest: "/mnt/data" }] | ||
| ├── data/ | ||
| │ └── greeting.txt # Archivo montado en /mnt/data/greeting.txt | ||
| └── src/ | ||
| └── main.wasm.ts # Lee /mnt/data/greeting.txt con readFile | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Declara el lenguaje de todos los bloques Markdown.
Los cuatro bloques no declaran un lenguaje y activan markdownlint MD040. Usa text para los árboles de directorios y las salidas esperadas.
examples/as-fs/README.md#L7-L14: cambia el fence del árbol de directorios a```text.examples/as-fs/README.md#L26-L29: cambia el fence de salida esperada a```text.examples/cpp-saludo/README.md#L7-L12: cambia el fence del árbol de directorios a```text.examples/cpp-saludo/README.md#L24-L26: cambia el fence de salida esperada a```text.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
📍 Affects 2 files
examples/as-fs/README.md#L7-L14(this comment)examples/as-fs/README.md#L26-L29examples/cpp-saludo/README.md#L7-L12examples/cpp-saludo/README.md#L24-L26
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/as-fs/README.md` around lines 7 - 14, Declara el lenguaje de los
cuatro bloques Markdown sin lenguaje: en examples/as-fs/README.md, líneas 7-14 y
26-29, y en examples/cpp-saludo/README.md, líneas 7-12 y 24-26, cambia cada
fence de apertura a ```text para los árboles de directorios y las salidas
esperadas.
Source: Linters/SAST tools
| ``` | ||
| custom-template/ | ||
| ├── wapp.json # linker.templatePath: "./templates" | ||
| ├── templates/ | ||
| │ └── main.c.njk # Template Nunjucks personalizado | ||
| └── src/ | ||
| └── main.wasm.ts # Módulo AssemblyScript de ejemplo | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Añade identificadores de lenguaje a los bloques Markdown. Todos los bloques afectados carecen de identificador y activan MD040.
examples/custom-template/README.md#L7-L14: Añadetextal bloque de estructura.examples/custom-template/README.md#L26-L28: Añadetextoconsoleal bloque de salida.examples/mounts-demo/README.md#L7-L14: Añadetextal bloque de estructura.examples/mounts-demo/README.md#L26-L29: Añadetextoconsoleal bloque de salida.examples/multi-toolchain/README.md#L7-L13: Añadetextal bloque de estructura.examples/multi-toolchain/README.md#L25-L28: Añadetextoconsoleal bloque de salida.examples/precompiled/README.md#L7-L12: Añadetextal bloque de estructura.examples/precompiled/README.md#L24-L28: Añadetextoconsoleal bloque de salida.examples/rust-fs/README.md#L7-L15: Añadetextal bloque de estructura.examples/rust-fs/README.md#L27-L30: Añadetextoconsoleal bloque de salida.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
📍 Affects 5 files
examples/custom-template/README.md#L7-L14(this comment)examples/custom-template/README.md#L26-L28examples/mounts-demo/README.md#L7-L14examples/mounts-demo/README.md#L26-L29examples/multi-toolchain/README.md#L7-L13examples/multi-toolchain/README.md#L25-L28examples/precompiled/README.md#L7-L12examples/precompiled/README.md#L24-L28examples/rust-fs/README.md#L7-L15examples/rust-fs/README.md#L27-L30
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/custom-template/README.md` around lines 7 - 14, Añade
identificadores de lenguaje a todos los bloques Markdown afectados: usa text en
los bloques de estructura de examples/custom-template/README.md (7-14),
examples/mounts-demo/README.md (7-14), examples/multi-toolchain/README.md
(7-13), examples/precompiled/README.md (7-12) y examples/rust-fs/README.md
(7-15); usa text o console en los bloques de salida de esos mismos archivos,
respectivamente en 26-28, 26-29, 25-28, 24-28 y 27-30.
Source: Linters/SAST tools
| ``` | ||
| rust-hello/ | ||
| ├── wapp.json # Configuración con wasi: true | ||
| └── src/ | ||
| ├── Cargo.toml # Sin [dependencies] — el compilador inyecta wasm_apps_bindings | ||
| └── main.wasm.rs # Módulo Rust que usa wasm_apps_bindings | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Añade el lenguaje a los bloques de código.
markdownlint reporta MD040 en las líneas 7 y 25. Marca ambos bloques como text.
Also applies to: 25-27
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/rust-hello/README.md` around lines 7 - 13, Update both fenced code
blocks in the README, including the blocks around the project tree and the
additional block near the referenced later section, to specify the text language
identifier. Preserve their contents and formatting while adding the language
marker required by markdownlint.
Source: Linters/SAST tools
| ## Cómo ejecutar | ||
|
|
||
| ```bash | ||
| cd examples/rust-hello | ||
| wapp build | ||
| ./rust-hello | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Incluye wapp setup en el flujo de ejecución.
En una instalación nueva, wapp build falla porque el linker necesita la Wasmtime C-API. Añade wapp setup antes de wapp build, o documenta este paso como requisito previo.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/rust-hello/README.md` around lines 15 - 21, Update the execution
instructions in the rust-hello README to run wapp setup before wapp build,
ensuring the documented flow works on a fresh installation.
| # `@wasm-apps/cli` — Orquestador `wapp` | ||
|
|
||
| CLI unificada que coordina el pipeline completo: descubre archivos fuente `.wasm.ts`, los compila a WebAssembly y linkea un ejecutable nativo autocontenido. Lee la configuración de `wapp.json` y admite sobrescrituras por línea de comandos. | ||
| CLI unificada que coordina el pipeline completo: descubre archivos fuente multi-extension (`.wasm.ts`, `.wasm.cpp`, `.wasm.rs`, `.wasm`, etc.), los compila a WebAssembly y linkea un ejecutable nativo autocontenido. Lee la configuración de `wapp.json` y admite sobrescrituras por línea de comandos. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Mantén coherente la documentación multi-toolchain.
Las nuevas introducciones amplían el soporte, pero cada README conserva instrucciones antiguas que mencionan solo .wasm.ts o AssemblyScript.
packages/cli/README.md#L3-L3: actualiza las descripciones del pipeline y de--source-dirpara incluir todas las extensiones soportadas.packages/compiler/README.md#L1-L15: actualiza las descripciones posteriores decompileWasmy del CLI, o indica explícitamente qué APIs siguen siendo específicas de AssemblyScript.
📍 Affects 2 files
packages/cli/README.md#L3-L3(this comment)packages/compiler/README.md#L1-L15
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/README.md` at line 3, Actualiza la documentación multi-toolchain
en packages/cli/README.md, líneas 3-3: describe el pipeline y la opción
--source-dir incluyendo todas las extensiones de fuente soportadas, no solo
.wasm.ts. En packages/compiler/README.md, líneas 1-15, corrige las descripciones
de compileWasm y del CLI para reflejar el soporte ampliado, o indica claramente
qué APIs siguen siendo específicas de AssemblyScript.
| | Lenguaje | Mecanismo | | ||
| | -------------- | ----------------------------------------------------------------------------------------------------------------- | | ||
| | AssemblyScript | Imports `from 'console' | 'fs' | 'wasi'`reescritos a`bindings/*.ts` (`rewriteBindingImports`) | | ||
| | C++ | Include path del directorio `bindings/` añadido a las flags (`-I`) | | ||
| | Rust | Crate vendido `wasm_apps_bindings` inyectado como dependencia `path` en `Cargo.toml` (`injectBindingsDependency`) | | ||
|
|
||
| Fuentes: `src/bindings/*.ts` (AssemblyScript), `src/bindings/*.h` (C++), `src/bindings/rust/` (crate Rust). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Corrige la tabla de bindings de AssemblyScript.
La línea 11 usa | sin escapar dentro de una celda y contiene inline code mal formado. La tabla puede renderizar columnas adicionales, y markdownlint reporta MD038 y MD056. Usa una lista separada por comas dentro de la celda.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 11-11: Spaces inside code span elements
(MD038, no-space-in-code)
[warning] 11-11: Table column count
Expected: 2; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/compiler/README.md` around lines 9 - 15, Corrige la fila de
AssemblyScript en la tabla de bindings para que el código inline esté
correctamente delimitado y los imports `console`, `fs` y `wasi` se separen
mediante comas, sin usar barras verticales sin escapar dentro de la celda.
Verifica que la tabla mantenga tres columnas y cumpla MD038 y MD056.
Source: Linters/SAST tools
| | `mounts` | `MountSpec[]` | Preopens WASI: `{ host, guest }[]` (opcional) | | ||
|
|
||
| ### Preopens WASI (mounts) | ||
|
|
||
| Con `wasi: true`, `mounts` configura preopens de directorios del host visibles dentro del WASM: | ||
|
|
||
| ```ts | ||
| await createNativeApp({ | ||
| inputPaths: ['wasm-out/modulo.wasm'], | ||
| output: 'dist/mi-app', | ||
| entry: '_start', | ||
| wasi: true, | ||
| mounts: [{ host: 'data', guest: '/mnt/data' }], | ||
| }); | ||
| ``` | ||
|
|
||
| El linker valida cada mount en build-time (`ConfigError`: host/guest obligatorios, host existente y directorio, guest absoluto), resuelve los hosts relativos contra el cwd del build y genera `wasi_config.preopen_dir(...)` con permisos completos (READ|WRITE) en el template `main.c.njk`. Los mounts forman parte del manifiesto de build (invalida la caché si cambian). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Actualiza la firma documentada de generateCCode.
La sección de mounts documenta la generación de preopens, pero la API de la línea 79 todavía termina en importFuncTypes?. Añade mounts?: MountSpec[] a la firma documentada y al ejemplo correspondiente.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/linker/README.md` around lines 40 - 56, Actualiza la firma
documentada de generateCCode para incluir el parámetro opcional mounts?:
MountSpec[] y añade mounts al ejemplo correspondiente, manteniendo consistente
la documentación con la configuración de preopens descrita.
|
|
||
| | Lenguaje | Mecanismo | | ||
| | -------------- | ----------------------------------------------------------------------------------------------------------------- | | ||
| | AssemblyScript | Imports `from 'console' | 'fs' | 'wasi'`reescritos a`bindings/*.ts` (`rewriteBindingImports`) | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Corrige la fila de AssemblyScript.
La Line 73 contiene | sin escapar dentro de una celda. Markdown la interpreta como varias columnas y la tabla deja de tener dos columnas. Reescribe la lista de imports sin pipes.
Propuesta
-| AssemblyScript | Imports `from 'console' | 'fs' | 'wasi'`reescritos a`bindings/*.ts` (`rewriteBindingImports`) |
+| AssemblyScript | Imports desde `console`, `fs` y `wasi` se reescriben a `bindings/*.ts` mediante `rewriteBindingImports` |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | AssemblyScript | Imports `from 'console' | 'fs' | 'wasi'`reescritos a`bindings/*.ts` (`rewriteBindingImports`) | | |
| | AssemblyScript | Imports desde `console`, `fs` y `wasi` se reescriben a `bindings/*.ts` mediante `rewriteBindingImports` | |
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 73-73: Spaces inside code span elements
(MD038, no-space-in-code)
[warning] 73-73: Table column count
Expected: 2; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wiki/entities/compiler.md` at line 73, Corrige la fila de AssemblyScript en
la tabla para que la lista de imports no contenga caracteres `|` sin escapar;
reescribe los módulos `console`, `fs` y `wasi` usando una separación sin pipes y
conserva las dos columnas de la tabla.
Source: Linters/SAST tools
WAPP_KEEP_CPP=1 guarda el main.cpp generado junto al binario y test-examples lo vuelca a stderr cuando un ejemplo falla en runtime. Diagnóstico temporal para investigar 'unknown import fd_write' en CI; se revierte al cerrar el hallazgo.
Los wapp.json de los ejemplos quedaron atrapados por el .gitignore global (patrón wapp.json), así que en checkout fresco no existían y el CLI resolvía defaults (wasi=false). Por eso los ejemplos WASI fallaban en CI con 'unknown import: wasi_snapshot_preview1::fd_write' mientras en local pasaban (los archivos estaban en disco). - .gitignore: re-incluye examples/**/wapp.json (configs de demo) - versiona 10 wapp.json de examples/ - revierte instrumentación de diagnóstico WAPP_KEEP_CPP
Docstrings generation was requested by @AndyTechnologies. * #20 (comment) The following files were modified: * `examples/as-fs/src/main.wasm.ts` * `examples/cpp-saludo/src/main.wasm.cpp` * `examples/mounts-demo/src/main.wasm.cpp` * `examples/rust-fs/src/main.wasm.rs` * `examples/rust-hello/src/main.wasm.rs` * `packages/compiler/src/bindings/console.h` * `packages/compiler/src/bindings/console.rs` * `packages/compiler/src/bindings/console.ts` * `packages/compiler/src/bindings/fs.h` * `packages/compiler/src/bindings/fs.rs` * `packages/compiler/src/bindings/fs.ts` * `packages/compiler/src/bindings/rust/src/console.rs` * `packages/compiler/src/bindings/rust/src/fs.rs` * `packages/compiler/src/bindings/rust/src/wasi.rs` * `packages/compiler/src/bindings/wasi.h` * `packages/compiler/src/bindings/wasi.rs` * `packages/compiler/src/bindings/wasi.ts` * `packages/compiler/src/index.ts` * `packages/compiler/src/strategies/rust-strategy.ts` * `packages/linker/templates/console.as.ts` * `packages/linker/templates/console.h` * `packages/linker/templates/console.rs` * `packages/linker/templates/fs.as.ts` * `packages/linker/templates/fs.h` * `packages/linker/templates/fs.rs` * `packages/linker/templates/wasi.as.ts` * `packages/linker/templates/wasi.h` * `packages/linker/templates/wasi.rs`
Resumen
.wasm.ts/.wasm.cpp/.wasm.rsya no dependen de bindings copiadas a mano en los ejemplos.__wasi_prestat_tde 8 bytes (tag + padding +pr_name_len@4),makeIovecsin write fuera de bounds,path_opencon oflags WASI (O_CREAT 0x01, O_TRUNC 0x08) en vez de Linux.gh run list --workflow CI(check runs) en vez de combined commit-status API.Cambios
packages/compiler/src/bindings/*packages/linker/templates/*packages/compiler/src/strategies/*packages/linker/src/*examples/*openspec/changes/bindings-auto-injection/*.github/workflows/auto-pr.ymlgh run list --workflow CIVerificación
pnpm -r buildpasapnpm checkpasa (falla soloprettier --checksobrepackages/linker/templates/*.as.ts— decorators de AssemblyScript no parseables por prettier; defecto de config preexistente, fix pendiente con el cambio de seguimiento R3-003)vitest run)pnpm test:integration) — pendiente con el cambio de seguimientoNotas
review-356a941320f28656aprobado; follow-up R3-003 (fs-bindings inactivables: falta configurar preopens WASI en CLI/linker) queda registrado como follow-up con su propio diseño.devno cubiertos por el genesis del review (62 paths vs 63 del receipt) — el gate pre-push deniega; la cobertura del review aplica al diff del change, no al rango completo.mainarrastra el avance dedevsin mergear (docs de release por tags, bump 1.4.0, fix path containment). Si se quiere un PR limpio solo del change, se puede rebasar la rama contradev+maino crear PR intermedio adev.Summary by CodeRabbit