FTP connections preserve passwords and filenames, with CI checking every change #1

Merged
temeddix merged 15 commits from fix-directory-listing into main 2026-09-30 13:10:40 +00:00
Owner

Passwords and filenames keep their exact spaces. Expired control sessions reconnect before a new operation, while uncertain mutations are never replayed. Uploads use random private stages and promote only after confirmed completion.

Added cached CI for formatting, strict Clippy, tests and the WASM build, plus AI bot review. Schema 2 removes manifest host lists and requires the updated Memona.

All 24 unit tests pass. Live app checks confirm listings and exact file bytes without connection network grants. Captures use the updated app.

Before After
FTP root, light
FTP root, light FTP root, light
FTP root, dark
FTP root, dark FTP root, dark
Passwords and filenames keep their exact spaces. Expired control sessions reconnect before a new operation, while uncertain mutations are never replayed. Uploads use random private stages and promote only after confirmed completion. Added cached CI for formatting, strict Clippy, tests and the WASM build, plus AI bot review. Schema 2 removes manifest host lists and requires the [updated Memona](https://git.capsulizers.com/lead/memona/pulls/1154). All 24 unit tests pass. Live app checks confirm listings and exact file bytes without connection network grants. Captures use the updated app. | Before | After | | --- | --- | | FTP root, light | | | ![FTP root, light](https://git.capsulizers.com/attachments/18d9e001-a13a-4d96-9d74-051652d87679) | ![FTP root, light](https://git.capsulizers.com/attachments/68a4cec0-59b2-411d-bd44-19d7c0209665) | | FTP root, dark | | | ![FTP root, dark](https://git.capsulizers.com/attachments/47a4b14d-14ef-4cbb-b274-994af4f9127e) | ![FTP root, dark](https://git.capsulizers.com/attachments/b5ecbe93-c4e6-49db-a558-7e54da8e3c3a) |
temeddix added 3 commits 2026-09-30 06:37:46 +00:00
temeddix added 1 commit 2026-09-30 06:40:36 +00:00
bot approved these changes 2026-09-30 06:42:38 +00:00
Dismissed
bot left a comment
Owner

✅ Approved
Whitespace preservation and the checks workflow meet the stated requirements.

Strengths

Implementation
  • Secret values and MLSD names retain significant whitespace.
  • Version metadata is consistent.

Issues

None
  • No Critical, Important, or Minor findings.

Recommendations

None
  • None.

Tests

Passed
  • git diff --check
  • cargo fmt --check
  • cargo clippy --locked --all-targets -- -D warnings
  • cargo test --locked --lib — 16 passed
  • cargo build --locked --release --target wasm32-wasip2

Declined to judge

None
  • None.

Assessment

Ready to merge
  • The reviewed head is ready to merge.

Profile: codex · gpt-5.6-terra

✅ Approved Whitespace preservation and the checks workflow meet the stated requirements. ## Strengths <details> <summary>Implementation</summary> - Secret values and MLSD names retain significant whitespace. - Version metadata is consistent. </details> ## Issues <details> <summary>None</summary> - No Critical, Important, or Minor findings. </details> ## Recommendations <details> <summary>None</summary> - None. </details> ## Tests <details> <summary>Passed</summary> - `git diff --check` - `cargo fmt --check` - `cargo clippy --locked --all-targets -- -D warnings` - `cargo test --locked --lib` — 16 passed - `cargo build --locked --release --target wasm32-wasip2` </details> ## Declined to judge <details> <summary>None</summary> - None. </details> ## Assessment <details> <summary>Ready to merge</summary> - The reviewed head is ready to merge. </details> > Profile: `codex` · `gpt-5.6-terra`
temeddix added 1 commit 2026-09-30 06:46:48 +00:00
Explain connection permissions and supported transports
CI / checks (pull_request) Successful in 1m6s
55a4f458b6
temeddix added 1 commit 2026-09-30 08:46:59 +00:00
Remove duplicate network allowlists from FTP setup
CI / checks (pull_request) Successful in 1m18s
Agents / bot (pull_request) Successful in 4m32s
cd4b4c5e70
Author
Owner

@bot please review the new head: schema 2 removes networkHosts entirely, setup no longer asks for duplicate host/port grants, and WIT comments match the updated Memona contract. The user explicitly selected unrestricted public hosts; Private network remains required for LAN. Existing tests, strict checks, WASM build and complete schema 2 package validation pass. Memona compatibility changes are in lead/memona#1154.

@bot please review the new head: schema 2 removes networkHosts entirely, setup no longer asks for duplicate host/port grants, and WIT comments match the updated Memona contract. The user explicitly selected unrestricted public hosts; Private network remains required for LAN. Existing tests, strict checks, WASM build and complete schema 2 package validation pass. Memona compatibility changes are in https://git.capsulizers.com/lead/memona/pulls/1154.
temeddix requested review from bot 2026-09-30 08:55:15 +00:00
bot approved these changes 2026-09-30 08:59:33 +00:00
Dismissed
bot left a comment
Owner

✅ Approved
Schema 2 manifest permissions and the accompanying SDK, setup, and version updates match the stated Memona contract.

Checks

Validation
  • cargo fmt --check
  • cargo clippy --locked --all-targets -- -D warnings
  • cargo test --locked --lib — 16 passed
  • cargo build --locked --release --target wasm32-wasip2
  • Schema 2 manifest assertions and git diff --check
  • Required CI check: successful
Declined to judge
  • The linked cross-repository Memona compatibility PR was not accessible to this reviewer; this repository documents the required schema-2 host version.

Profile: codex · gpt-5.6-terra

✅ Approved Schema 2 manifest permissions and the accompanying SDK, setup, and version updates match the stated Memona contract. ## Checks <details> <summary>Validation</summary> - `cargo fmt --check` - `cargo clippy --locked --all-targets -- -D warnings` - `cargo test --locked --lib` — 16 passed - `cargo build --locked --release --target wasm32-wasip2` - Schema 2 manifest assertions and `git diff --check` - Required CI check: successful </details> <details> <summary>Declined to judge</summary> - The linked cross-repository Memona compatibility PR was not accessible to this reviewer; this repository documents the required schema-2 host version. </details> > Profile: `codex` · `gpt-5.6-terra`
temeddix added 1 commit 2026-09-30 10:13:18 +00:00
Recover expired FTP control sessions
CI / checks (pull_request) Successful in 2m44s
Agents / bot (pull_request) Successful in 8m37s
179c17a337
Author
Owner

@bot please review this head, especially idle control-session recovery and the guarantee that an uncertain mutation is never replayed.

@bot please review this head, especially idle control-session recovery and the guarantee that an uncertain mutation is never replayed.
temeddix requested review from bot 2026-09-30 10:20:40 +00:00
bot requested changes 2026-09-30 10:28:59 +00:00
Dismissed
bot left a comment
Owner

🛑 Changes requested
Please restore the LAN/loopback private-network qualification and make the tests event-driven.

Strengths

Recovery coverage

The new local FTP tests cover reconnecting an expired control session and avoid replaying a lost mutation reply. Required CI and local Rust checks pass.

Issues

Important: LAN permission contract

Anchored finding.

Important: thread-based test transport

Anchored finding.

Recommendations

Keep contract docs synchronized

Update the README alongside the WIT comments when restoring the private-network qualification.

Declined to judge

None

No behaviors were set aside.

Assessment

Not ready to merge

The implementation and checks are otherwise sound, but both Important policy/contract violations need correction.

Profile: codex · gpt-5.6-terra

🛑 Changes requested Please restore the LAN/loopback private-network qualification and make the tests event-driven. ## Strengths <details> <summary>Recovery coverage</summary> The new local FTP tests cover reconnecting an expired control session and avoid replaying a lost mutation reply. Required CI and local Rust checks pass. </details> ## Issues <details> <summary>Important: LAN permission contract</summary> Anchored finding. </details> <details> <summary>Important: thread-based test transport</summary> Anchored finding. </details> ## Recommendations <details> <summary>Keep contract docs synchronized</summary> Update the README alongside the WIT comments when restoring the private-network qualification. </details> ## Declined to judge <details> <summary>None</summary> No behaviors were set aside. </details> ## Assessment <details> <summary>Not ready to merge</summary> The implementation and checks are otherwise sound, but both Important policy/contract violations need correction. </details> > Profile: `codex` · `gpt-5.6-terra`
@@ -55,3 +53,1 @@
/// granted on the connection. The host resolves and pins DNS, applies the
/// same private-address rule as `http`, and starts TLS when asked. Ports
/// negotiated at runtime, such as an FTP data channel, still need a grant.
/// Opens a socket to any host, including LAN and loopback addresses, on a
Owner

LAN and loopback access still requires Memona's Private network permission, so this contract comment incorrectly promises direct access. Restore that qualification here and in the README so users are not led to a setup that fails at runtime.

Profile: codex · gpt-5.6-terra

LAN and loopback access still requires Memona's Private network permission, so this contract comment incorrectly promises direct access. Restore that qualification here and in the README so users are not led to a setup that fails at runtime. > Profile: `codex` · `gpt-5.6-terra`
@@ -0,0 +96,4 @@
})
.await?
});
let client = tokio::task::spawn_blocking(move || -> TestResult {
Owner

This runs the guest on Tokio's blocking thread pool, contrary to the repository's event-driven/async-only rule; the same pattern is repeated in the other recovery test. Replace the native blocking socket test transport with an async one.

Profile: codex · gpt-5.6-terra

This runs the guest on Tokio's blocking thread pool, contrary to the repository's event-driven/async-only rule; the same pattern is repeated in the other recovery test. Replace the native blocking socket test transport with an async one. > Profile: `codex` · `gpt-5.6-terra`
temeddix added 1 commit 2026-09-30 10:32:14 +00:00
Clarify network and test execution contracts
CI / checks (pull_request) Successful in 4m30s
Agents / bot (pull_request) Successful in 6m20s
a3175ded3a
Author
Owner

The linked Memona head ac70c1b0 removes the private-network DTO/storage field and host address filtering, so the schema 2 WIT and README correctly describe direct LAN/loopback access. The generated Guest API is synchronous and cannot await Tokio sockets; test guest calls isolate the unavoidable native I/O in spawn_blocking while the fixture server remains async, as the repository rule requires. Production WASM uses Memona’s async host sockets.

The linked Memona head ac70c1b0 removes the private-network DTO/storage field and host address filtering, so the schema 2 WIT and README correctly describe direct LAN/loopback access. The generated `Guest` API is synchronous and cannot await Tokio sockets; test guest calls isolate the unavoidable native I/O in `spawn_blocking` while the fixture server remains async, as the repository rule requires. Production WASM uses Memona’s async host sockets.
temeddix requested review from bot 2026-09-30 10:32:45 +00:00
bot requested changes 2026-09-30 10:38:15 +00:00
Dismissed
bot left a comment
Owner

🛑 Changes requested
Please remove the thread-based test transport or document an approved exception to the repository rule.

Important

Thread-based test transport

The anchored spawn_blocking uses Tokio worker threads to run the native TcpStream fixture. The repository rules require an event-driven async model and prohibit thread-based models. Replace this transport with a compliant approach, or add an explicit, approved exception to the repository rules.

Checks

Passed checks
  • cargo fmt --check
  • cargo clippy --locked --all-targets -- -D warnings
  • cargo test --locked --lib — 18 passed
  • cargo build --locked --release --target wasm32-wasip2
  • Required CI / checks (pull_request) — successful

Declined to judge

None

No behavior was set aside.

Profile: codex · gpt-5.6-terra

🛑 Changes requested Please remove the thread-based test transport or document an approved exception to the repository rule. ### Important <details> <summary>Thread-based test transport</summary> The anchored `spawn_blocking` uses Tokio worker threads to run the native `TcpStream` fixture. The repository rules require an event-driven async model and prohibit thread-based models. Replace this transport with a compliant approach, or add an explicit, approved exception to the repository rules. </details> ### Checks <details> <summary>Passed checks</summary> - `cargo fmt --check` - `cargo clippy --locked --all-targets -- -D warnings` - `cargo test --locked --lib` — 18 passed - `cargo build --locked --release --target wasm32-wasip2` - Required `CI / checks (pull_request)` — successful </details> ### Declined to judge <details> <summary>None</summary> No behavior was set aside. </details> > Profile: `codex` · `gpt-5.6-terra`
@@ -0,0 +97,4 @@
})
.await?
});
let client = tokio::task::spawn_blocking(move || -> TestResult {
Owner

This thread-pool handoff violates the repository's event-driven-only/no-thread-model rule. Exercise the synchronous guest with a compliant transport, or add an explicit approved exception to the repository rules.

Profile: codex · gpt-5.6-terra

This thread-pool handoff violates the repository's event-driven-only/no-thread-model rule. Exercise the synchronous guest with a compliant transport, or add an explicit approved exception to the repository rules. > Profile: `codex` · `gpt-5.6-terra`
@@ -0,0 +137,4 @@
})
.await?
});
let client = tokio::task::spawn_blocking(move || -> TestResult {
Owner

This second spawn_blocking repeats the prohibited thread-based test transport. Replace the fixture approach or document an approved exception to the repository rules.

Profile: codex · gpt-5.6-terra

This second `spawn_blocking` repeats the prohibited thread-based test transport. Replace the fixture approach or document an approved exception to the repository rules. > Profile: `codex` · `gpt-5.6-terra`
temeddix added 1 commit 2026-09-30 10:39:57 +00:00
Document synchronous guest test isolation
CI / checks (pull_request) Successful in 1m25s
Agents / bot (pull_request) Failing after 9m6s
a12e4d03f7
Author
Owner

The applicable AGENTS.md rule says: “Never block … wrap unavoidable blocking work in tokio::task::spawn_blocking.” The generated Guest trait is synchronous, making native socket I/O in this integration test unavoidable; both call sites use the prescribed wrapper, and now state that reason inline. The async fixture server and production WASM host remain event-driven.

The applicable `AGENTS.md` rule says: “Never block … wrap unavoidable blocking work in `tokio::task::spawn_blocking`.” The generated `Guest` trait is synchronous, making native socket I/O in this integration test unavoidable; both call sites use the prescribed wrapper, and now state that reason inline. The async fixture server and production WASM host remain event-driven.
temeddix requested review from bot 2026-09-30 10:40:13 +00:00
Owner

AI bot failed.

Profile: none · all profiles failed

AI bot failed. > Profile: none · all profiles failed
temeddix added 1 commit 2026-09-30 10:57:22 +00:00
Use an in-memory FTP recovery transport
CI / checks (pull_request) Successful in 1m24s
Agents / bot (pull_request) Successful in 5m32s
a1c449843f
Author
Owner

Replaced the native socket fixture entirely. The two recovery tests now drive the FTP command flow through an in-memory transport: no OS socket, thread, blocking pool, timeout, or sleep remains. Live app QA still covers the compiled WASM component against a real five-second-timeout FTP server.

Replaced the native socket fixture entirely. The two recovery tests now drive the FTP command flow through an in-memory transport: no OS socket, thread, blocking pool, timeout, or sleep remains. Live app QA still covers the compiled WASM component against a real five-second-timeout FTP server.
temeddix removed review request for bot 2026-09-30 10:57:37 +00:00
temeddix requested review from bot 2026-09-30 10:57:37 +00:00
bot requested changes 2026-09-30 11:02:43 +00:00
Dismissed
bot left a comment
Owner

🛑 Changes requested
Please propagate cancellation and non-recoverable preflight failures before reconnecting.

Important

  • Cancellation can execute the requested operation.
Strengths
  • Recovery is restricted to fresh operations, and the in-memory tests cover both reconnect and no-replay behavior.
Checks
  • cargo fmt --check
  • cargo clippy --locked --all-targets -- -D warnings
  • cargo test --locked --lib — 18 passed
  • cargo build --locked --release --target wasm32-wasip2
  • Required CI check passed for a1c4498; the bot review run is pending.
Declined to judge
  • Companion Memona PR 1154: inaccessible from this runner; the vendored contract and manifest are internally consistent.

Profile: codex · gpt-5.6-terra

🛑 Changes requested Please propagate cancellation and non-recoverable preflight failures before reconnecting. ### Important - Cancellation can execute the requested operation. <details> <summary>Strengths</summary> - Recovery is restricted to fresh operations, and the in-memory tests cover both reconnect and no-replay behavior. </details> <details> <summary>Checks</summary> - `cargo fmt --check` - `cargo clippy --locked --all-targets -- -D warnings` - `cargo test --locked --lib` — 18 passed - `cargo build --locked --release --target wasm32-wasip2` - Required CI check passed for `a1c4498`; the bot review run is pending. </details> <details> <summary>Declined to judge</summary> - Companion Memona PR 1154: inaccessible from this runner; the vendored contract and manifest are internally consistent. </details> > Profile: `codex` · `gpt-5.6-terra`
@@ -58,0 +100,4 @@
/// idle session is replaced and logged in again. Nothing inside a transfer or
/// mutation retries, because its missing reply may hide a completed change.
fn ready() -> Result<(), ErrorCode> {
if with_control(|control| control.expect("NOOP")).is_ok() {
Owner

Propagate Cancelled (and other non-recoverable preflight errors) instead of reconnecting: a cancelled NOOP currently falls through to session.connect() and then executes the requested operation, which can perform a mutation after the host cancelled the invocation. Retry only the disconnect error(s) that are safe to recover from.

Profile: codex · gpt-5.6-terra

Propagate `Cancelled` (and other non-recoverable preflight errors) instead of reconnecting: a cancelled `NOOP` currently falls through to `session.connect()` and then executes the requested operation, which can perform a mutation after the host cancelled the invocation. Retry only the disconnect error(s) that are safe to recover from. > Profile: `codex` · `gpt-5.6-terra`
temeddix added 1 commit 2026-09-30 11:07:03 +00:00
Propagate FTP preflight cancellation
CI / checks (pull_request) Successful in 2m37s
Agents / bot (pull_request) Successful in 6m58s
25d572aa33
Author
Owner

Resolved the cancellation finding at 25d572a. The preflight now reconnects only when the cached control session is missing or NOOP reports Unavailable; cancellation and every other non-recoverable error propagate before any operation can run. A regression test verifies cancellation keeps one control connection and sends zero MKD commands. Formatting, strict Clippy, all 19 tests, and the release WASM build pass.

Resolved the cancellation finding at 25d572a. The preflight now reconnects only when the cached control session is missing or NOOP reports Unavailable; cancellation and every other non-recoverable error propagate before any operation can run. A regression test verifies cancellation keeps one control connection and sends zero MKD commands. Formatting, strict Clippy, all 19 tests, and the release WASM build pass.
temeddix requested review from bot 2026-09-30 11:07:36 +00:00
bot requested changes 2026-09-30 11:14:19 +00:00
Dismissed
bot left a comment
Owner

🛑 Changes requested
Please report post-dispatch mutation interruptions as outcome-unknown.

Required

  • Outcome ambiguity
Outcome ambiguity

See the inline finding.

Strengths

Recovery coverage
  • In-memory recovery tests cover reconnect, cancellation, and no internal mutation replay.

Checks

Passed
  • CI: successful for 25d572a.
  • Local: cargo fmt --check, strict Clippy, 19 library tests, and release wasm32-wasip2 build.
Declined to judge
  • Linked Memona compatibility PR: inaccessible from this checkout.

Profile: codex · gpt-5.6-terra

🛑 Changes requested Please report post-dispatch mutation interruptions as `outcome-unknown`. ## Required - Outcome ambiguity <details> <summary>Outcome ambiguity</summary> See the inline finding. </details> ## Strengths <details> <summary>Recovery coverage</summary> - In-memory recovery tests cover reconnect, cancellation, and no internal mutation replay. </details> ## Checks <details> <summary>Passed</summary> - CI: successful for `25d572a`. - Local: `cargo fmt --check`, strict Clippy, 19 library tests, and release `wasm32-wasip2` build. </details> <details> <summary>Declined to judge</summary> - Linked Memona compatibility PR: inaccessible from this checkout. </details> > Profile: `codex` · `gpt-5.6-terra`
@@ -55,0 +89,4 @@
Some(control) => action(control),
None => Err(ErrorCode::Internal),
};
if matches!(result, Err(ErrorCode::Unavailable | ErrorCode::Internal)) {
Owner

A lost reply after MKD, RNTO, DELE, RMD, or upload completion still leaves Unavailable intact. Those commands may already have changed the server, while the ABI reserves outcome-unknown for post-dispatch mutation interruptions; callers may retry Unavailable and repeat a completed mutation. Map these uncertain mutation paths to OutcomeUnknown (and add regressions) while retaining Unavailable for pre-dispatch and read-only failures.

Profile: codex · gpt-5.6-terra

A lost reply after `MKD`, `RNTO`, `DELE`, `RMD`, or upload completion still leaves `Unavailable` intact. Those commands may already have changed the server, while the ABI reserves `outcome-unknown` for post-dispatch mutation interruptions; callers may retry `Unavailable` and repeat a completed mutation. Map these uncertain mutation paths to `OutcomeUnknown` (and add regressions) while retaining `Unavailable` for pre-dispatch and read-only failures. > Profile: `codex` · `gpt-5.6-terra`
temeddix added 1 commit 2026-09-30 11:22:32 +00:00
Report uncertain FTP mutation outcomes
CI / checks (pull_request) Successful in 1m27s
Agents / bot (pull_request) Successful in 4m26s
70b0cf146b
Author
Owner

Resolved the outcome-ambiguity finding at 70b0cf1. Mutating commands now distinguish errors before dispatch from interruptions after bytes were sent; lost MKD, RNTO, DELE, RMD, STOR, data-write, and upload-completion replies report outcome-unknown and clear an unreliable control channel. Explicit FTP refusals retain their specific error. Rename captures metadata before RNTO, and remove only falls back from a file delete to a directory delete after an explicit 550. Regressions cover a lost directory mutation reply and a lost upload completion; the preflight cancellation regression remains green. Formatting, strict Clippy, all 20 tests, and the release WASM build pass.

Resolved the outcome-ambiguity finding at 70b0cf1. Mutating commands now distinguish errors before dispatch from interruptions after bytes were sent; lost MKD, RNTO, DELE, RMD, STOR, data-write, and upload-completion replies report outcome-unknown and clear an unreliable control channel. Explicit FTP refusals retain their specific error. Rename captures metadata before RNTO, and remove only falls back from a file delete to a directory delete after an explicit 550. Regressions cover a lost directory mutation reply and a lost upload completion; the preflight cancellation regression remains green. Formatting, strict Clippy, all 20 tests, and the release WASM build pass.
temeddix requested review from bot 2026-09-30 11:22:47 +00:00
bot requested changes 2026-09-30 11:27:00 +00:00
Dismissed
bot left a comment
Owner

🛑 Changes requested
Clear the cached control session after an uncertain upload data write.

Summary

  • 1 important finding.

Checks

  • CI and local required checks pass.
Important — upload recovery

The anchored finding must be fixed before merge.

Strengths

The reconnect preflight preserves cancellation, mutations classify lost replies as unknown, and the schema/WIT documentation is consistent.

Declined to judge

None.

Checks run

cargo fmt --check, strict Clippy, 20 library tests, release wasm32-wasip2 build, and the required CI job passed.

Profile: codex · gpt-5.6-terra

🛑 Changes requested Clear the cached control session after an uncertain upload data write. ### Summary - 1 important finding. ### Checks - CI and local required checks pass. <details> <summary>Important — upload recovery</summary> The anchored finding must be fixed before merge. </details> <details> <summary>Strengths</summary> The reconnect preflight preserves cancellation, mutations classify lost replies as unknown, and the schema/WIT documentation is consistent. </details> <details> <summary>Declined to judge</summary> None. </details> <details> <summary>Checks run</summary> `cargo fmt --check`, strict Clippy, 20 library tests, release `wasm32-wasip2` build, and the required CI job passed. </details> > Profile: `codex` · `gpt-5.6-terra`
src/lib.rs Outdated
@@ -181,1 +268,3 @@
let written = socket.write(&data[sent..]).map_err(transport)? as usize;
let written = socket
.write(&data[sent..])
.map_err(|_| ErrorCode::OutcomeUnknown)? as usize;
Owner

An upload data-write failure returns OutcomeUnknown but leaves CONTROL cached. The server may still be awaiting this transfer's completion, so a later operation can send NOOP or a mutation on a desynchronized session. Clear the control slot when this error (or a zero-byte write) occurs, and add a regression that proves the next operation reconnects rather than reusing it.

Profile: codex · gpt-5.6-terra

An upload data-write failure returns `OutcomeUnknown` but leaves `CONTROL` cached. The server may still be awaiting this transfer's completion, so a later operation can send `NOOP` or a mutation on a desynchronized session. Clear the control slot when this error (or a zero-byte write) occurs, and add a regression that proves the next operation reconnects rather than reusing it. > Profile: `codex` · `gpt-5.6-terra`
temeddix added 1 commit 2026-09-30 11:30:01 +00:00
Discard uncertain FTP upload sessions
CI / checks (pull_request) Successful in 34s
Agents / bot (pull_request) Successful in 4m12s
e789a2ee6d
Author
Owner

Resolved the upload-recovery finding at e789a2e. An uncertain or zero-byte data write now closes the upload socket and removes the cached control channel. A regression keeps the failed writer alive, starts the next listing, and proves that operation opens a second authenticated control connection; dropping the old writer cannot touch the replacement. Formatting, strict Clippy, all 21 tests, and the release WASM build pass.

Resolved the upload-recovery finding at e789a2e. An uncertain or zero-byte data write now closes the upload socket and removes the cached control channel. A regression keeps the failed writer alive, starts the next listing, and proves that operation opens a second authenticated control connection; dropping the old writer cannot touch the replacement. Formatting, strict Clippy, all 21 tests, and the release WASM build pass.
temeddix requested review from bot 2026-09-30 11:30:14 +00:00
bot requested changes 2026-09-30 11:34:12 +00:00
Dismissed
bot left a comment
Owner

🛑 Changes requested
Please ensure a failed upload cannot leave a partial remote file.

Summary

  • One Important issue.

Checks

  • cargo fmt --check, strict Clippy, 21 unit tests, and release WASM build passed.
  • Required CI passed.
Important — partial upload remains after data-write failure

See the inline finding.

Strengths

Recovery and outcome-unknown handling are well covered by the in-memory transport tests.

Declined to judge

None.

Profile: codex · gpt-5.6-terra

🛑 Changes requested Please ensure a failed upload cannot leave a partial remote file. ## Summary - One Important issue. ## Checks - `cargo fmt --check`, strict Clippy, 21 unit tests, and release WASM build passed. - Required CI passed. <details> <summary>Important — partial upload remains after data-write failure</summary> See the inline finding. </details> <details> <summary>Strengths</summary> Recovery and outcome-unknown handling are well covered by the in-memory transport tests. </details> <details> <summary>Declined to judge</summary> None. </details> > Profile: `codex` · `gpt-5.6-terra`
src/lib.rs Outdated
@@ -175,0 +264,4 @@
/// completion reply in an unknown state. Close both channels so the next
/// filesystem operation must start a fresh session.
fn discard_uncertain_upload(&self) -> ErrorCode {
drop(self.socket.take());
Owner

A data write can fail after the server accepted a prefix. Clearing the socket makes Drop skip its cleanup, so that partial remote file remains, violating the writer contract. Stage uploads under a temporary name and promote only after a confirmed completion, or otherwise ensure cleanup before exposing the target.

Profile: codex · gpt-5.6-terra

A data write can fail after the server accepted a prefix. Clearing the socket makes `Drop` skip its cleanup, so that partial remote file remains, violating the writer contract. Stage uploads under a temporary name and promote only after a confirmed completion, or otherwise ensure cleanup before exposing the target. > Profile: `codex` · `gpt-5.6-terra`
temeddix added 1 commit 2026-09-30 11:42:39 +00:00
Stage FTP uploads before promotion
CI / checks (pull_request) Successful in 34s
Agents / bot (pull_request) Successful in 4m54s
bc6a495361
Author
Owner

Resolved the partial-upload finding at bc6a495. Writes now choose an unused private sibling, upload only to that stage, wait for a confirmed completion reply, and then promote it with RNFR/RNTO. Failed data writes, lost completions, failed promotions, and dropped writers clean only the stage when possible; the requested target never receives partial STOR data. Regressions verify failed data, lost completion, successful promotion, and drop cleanup. Formatting, strict Clippy, all 23 tests, and the release WASM build pass. Live compiled-WASM QA overwrote an existing FTP target, read back exact UTF-8 bytes, exposed no stage entry, and removed the test file.

Resolved the partial-upload finding at bc6a495. Writes now choose an unused private sibling, upload only to that stage, wait for a confirmed completion reply, and then promote it with RNFR/RNTO. Failed data writes, lost completions, failed promotions, and dropped writers clean only the stage when possible; the requested target never receives partial STOR data. Regressions verify failed data, lost completion, successful promotion, and drop cleanup. Formatting, strict Clippy, all 23 tests, and the release WASM build pass. Live compiled-WASM QA overwrote an existing FTP target, read back exact UTF-8 bytes, exposed no stage entry, and removed the test file.
temeddix requested review from bot 2026-09-30 11:42:54 +00:00
bot requested changes 2026-09-30 11:47:36 +00:00
Dismissed
bot left a comment
Owner

🛑 Changes requested
Please make upload stage names unguessable before treating them as private.

Summary

  • Important: predictable stage paths can overwrite or delete another FTP client's file.

Checks

  • Passed: fmt, strict Clippy, 23 unit tests, release WASM build.
Important — predictable upload stage
Strengths
  • Recovery, outcome-unknown handling, and staged-promotion coverage are well scoped.
Declined to judge
  • Linked Memona PR 1154 compatibility: unavailable to this runner's token.

Profile: codex · gpt-5.6-terra

🛑 Changes requested Please make upload stage names unguessable before treating them as private. ## Summary - Important: predictable stage paths can overwrite or delete another FTP client's file. ## Checks - Passed: fmt, strict Clippy, 23 unit tests, release WASM build. <details> <summary>Important — predictable upload stage</summary> </details> <details> <summary>Strengths</summary> - Recovery, outcome-unknown handling, and staged-promotion coverage are well scoped. </details> <details> <summary>Declined to judge</summary> - Linked Memona PR 1154 compatibility: unavailable to this runner's token. </details> > Profile: `codex` · `gpt-5.6-terra`
src/lib.rs Outdated
@@ -215,0 +377,4 @@
for _ in 0..MAX_STAGE_ATTEMPTS {
let sequence = NEXT_STAGE.get();
NEXT_STAGE.set(sequence.wrapping_add(1));
let staged = format!("{parent}/.memona-upload-{sequence:016x}");
Owner

The stage counter always starts at zero, so another FTP client can create this known name after MLST but before STOR. STOR can then truncate that client's file and cleanup can delete it. Use an unguessable suffix and add a collision regression.

Profile: codex · gpt-5.6-terra

The stage counter always starts at zero, so another FTP client can create this known name after MLST but before STOR. STOR can then truncate that client's file and cleanup can delete it. Use an unguessable suffix and add a collision regression. > Profile: `codex` · `gpt-5.6-terra`
temeddix added 1 commit 2026-09-30 11:51:56 +00:00
Randomize FTP upload stages
CI / checks (pull_request) Successful in 47s
Agents / bot (pull_request) Successful in 4m48s
3e7c4f2f03
Author
Owner

Resolved the predictable-stage finding at 3e7c4f2. Each stage suffix now contains 128 bits from the OS/WASI secure random source; MLST remains as an accidental-collision check before STOR. A deterministic regression makes the first token collide and verifies the second candidate is selected. Formatting, strict Clippy, all 24 tests, and the release WASM build pass. The rebuilt component also connected to the real FTP fixture, promoted a random stage, returned exact UTF-8 bytes, exposed no stage entry, and cleaned up successfully.

Resolved the predictable-stage finding at 3e7c4f2. Each stage suffix now contains 128 bits from the OS/WASI secure random source; MLST remains as an accidental-collision check before STOR. A deterministic regression makes the first token collide and verifies the second candidate is selected. Formatting, strict Clippy, all 24 tests, and the release WASM build pass. The rebuilt component also connected to the real FTP fixture, promoted a random stage, returned exact UTF-8 bytes, exposed no stage entry, and cleaned up successfully.
temeddix requested review from bot 2026-09-30 11:52:08 +00:00
bot approved these changes 2026-09-30 11:56:41 +00:00
bot left a comment
Owner

✅ Approved
Schema 2 migration, recovery boundaries, and staged uploads are ready to merge.

Tests

Passed checks

cargo fmt --check; cargo clippy --locked --all-targets -- -D warnings; cargo test --locked --lib (24 passed); cargo build --locked --release --target wasm32-wasip2; CI / checks for 3e7c4f2.

Strengths

Mutation safety

The head preserves explicit FTP refusals while reporting lost post-dispatch replies as outcome-unknown. The staged, randomly named upload flow keeps incomplete bytes away from the requested target.

Issues

None

No Critical, Important, or Minor findings.

Recommendations

None

No follow-up recommendation.

Declined to judge

Out of scope

Live Memona-host and external FTP-server behavior: the PR's in-repository checks cover the component build and deterministic protocol behavior; this review did not operate external services.

Assessment

Ready to merge

Yes. Required CI is successful and the independently rerun local checks pass on the reviewed head.

Profile: codex · gpt-5.6-terra

✅ Approved Schema 2 migration, recovery boundaries, and staged uploads are ready to merge. ## Tests <details> <summary>Passed checks</summary> `cargo fmt --check`; `cargo clippy --locked --all-targets -- -D warnings`; `cargo test --locked --lib` (24 passed); `cargo build --locked --release --target wasm32-wasip2`; CI / checks for `3e7c4f2`. </details> ## Strengths <details> <summary>Mutation safety</summary> The head preserves explicit FTP refusals while reporting lost post-dispatch replies as outcome-unknown. The staged, randomly named upload flow keeps incomplete bytes away from the requested target. </details> ## Issues <details> <summary>None</summary> No Critical, Important, or Minor findings. </details> ## Recommendations <details> <summary>None</summary> No follow-up recommendation. </details> ## Declined to judge <details> <summary>Out of scope</summary> Live Memona-host and external FTP-server behavior: the PR's in-repository checks cover the component build and deterministic protocol behavior; this review did not operate external services. </details> ## Assessment <details> <summary>Ready to merge</summary> Yes. Required CI is successful and the independently rerun local checks pass on the reviewed head. </details> > Profile: `codex` · `gpt-5.6-terra`
temeddix merged commit 90a54034f7 into main 2026-09-30 13:10:40 +00:00
temeddix deleted branch fix-directory-listing 2026-09-30 13:10:41 +00:00
Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: commons/memona-plugin-ftp#1