chore: rebind/signal polish — review findings from #567/#568 - #575
Merged
Conversation
…ation Validated the plan's 11 load-bearing assumptions (7 verified, 3 falsified, 1 deferred with in-plan contingency). Fixes for the falsified ones: - Task 6: encode set corrected to pathToFileURL parity, oracle-verified against the installed Bun 1.3.14 / Node v22.21.1 (adds [ \ ] ^ | ~; posix backslash percent-encoded, not rewritten; 5 new table cases). - Tasks 1+2: ordering contract narrowed to GNU-deterministic with a documented, self-correcting second-granularity fallback residual (same-second order degrades to pid order; design unchanged). - Task 9: new Step 0 (npm ci prerequisite) -- cargo test --workspace depends on node_modules via the freshagent tsx MCP resolution; also records the verified green baseline and one known timing flake. Also annotates Task 7 with the verified mode/provider-permanence premise and the global port-safety constraint with the audit result.
The goldens module is mounted as cli_argv_goldens_file inside cli_launch.rs, so the filter 'cli_launch_goldens' matched zero tests and the step's Expected: FAIL could never occur. Use the module-path filter cli_argv_goldens_file and note why.
…module (fresheyes review 2) The plan told the implementer to add unit tests to cli_launch.rs's 'existing #[cfg(test)] mod tests', but that file has no in-file test module (only the cli_launch_goldens.rs mount). Step 1 now explicitly creates the module after the goldens mount and confirms the resulting cli_launch::tests::... paths match Step 2's --lib cli_launch filter.
…nonce date +%s%N is GNU-only; BSD/macOS degraded the nonce to a literal N and made the timestamp useless for ordering. POSIX fallback keeps 19-digit width-stable digits-only nonces, timestamp-first so the consumer's filename sort (next commit) is emission order on GNU date (on the second-granularity fallback, same-second order degrades to pid order -- documented residual, self-correcting). Uniqueness is preserved: GNU keeps ns precision; elsewhere (second, pid) -- same as before.
…te-wins Bare read_dir order made rapid A->B->A switches within one sweep window land nondeterministically. Filename sort over the timestamp-first nonces (previous commit) makes drain order emission order on GNU date (the second-granularity fallback degrades same-second order to pid order -- documented residual), matching the opencode lane's already-shipped contract.
…map (D7 parity) The codex lane probes fresh_codex.has_live_session before honoring a claim; the claude lane only had the durable fresh-agent ledger-row twin, leaving a window where a sidecar with an unlanded row was invisible. Adds the in-memory probe in the same guard position codex uses.
Pure code-move to the repo's #[path]-included child-module convention (pane_ledger_tests.rs et al.); both files now respect the <=1K-line limit and the tests keep private-item access. 34 tests before and after.
…per window The n>=2 refusal arm re-warned on every sweep tick for the whole fork window. Adds a log-only latch on ForkWatch, re-armed by a new Enter (note_fork_submit); refusal semantics and the deliberate no-merge of ambiguous hits are unchanged.
A home dir containing '#', '?', or '%'+hex produced a URL Bun resolves to the wrong path (silent pane-lifetime import failure); space/non-ASCII broke opencode's dedup-by-file-URL contract. Encode the path to the canonical WHATWG form (pathToFileURL parity) via percent-encoding.
…l files Foreign-provider signals were silently retained and re-rejected every sweep for 10 minutes -- the exact silently-never-firing shape the module header warns against. They are permanently unactionable (pane mode and identity provider never change), so: warn once, consume the file (SignalDisposition::Discard). Also reaps orphaned .tmp staging files on the existing TTL. Pane identity is never touched; all legitimate retention (no-pane-yet, not-live-yet) is unchanged.
…tor plan In-TUI /resume may fork into a new rollout file (openai/codex#34972); the flat 'no new file' claim predated the fork lane. Also annotates the pinned interface block as pre-fork-lane. Historical doc, correction markers included.
The drain doc still described the old pid-first <pid>-<ns> nonce shape; the producer now emits <timestamp>-<pid> (branch's own Task 1 change). Also wraps the adjacent long sort-rationale doc line. Doc-comment only.
…ex-lane-self-healing) into rebind-review-polish Semantic merge of the opencode signal lane, preserving both sides: - main's OpencodeDrainOutcome split (rebinds + hellos), plugin-alive hello delete-on-read, HelloTracker with once-per-pane grace-window warning - branch's SignalDisposition ladder (Acted/Retain/Discard) with explicit foreign-provider signal discard, stale .tmp staging reap, and all other polish items (deterministic claude drain ordering, portable timestamp-first nonce, sidecar guard parity, warn-once fork-ambiguity, percent-encoded plugin URL, codex_locator test split) Conflicts (crates/freshell-ws/src/opencode_signal.rs): - drain() doc + signature: kept OpencodeDrainOutcome return with combined doc covering hellos AND stale-.tmp reaping - drain_and_rebind_opencode(): kept main's hello-tracker pass, folded the rebind loop onto the branch's SignalDisposition match - adapted branch test drain_reaps_stale_tmp_staging_files_but_keeps_fresh_ones to the OpencodeDrainOutcome shape Hello files are discriminated at parse time (before the rebind ladder), so the foreign-signal Discard path can never consume a hello as "foreign". Verified: cargo fmt --check, clippy -D warnings, cargo test --workspace (2198 passed / 0 failed), contract freeze (test:port + regen idempotency + freshell-protocol --locked), coordinated npm run check. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Polish pass on the rebind/signal work from #567/#568, closing the minor findings from both delta reviews.
Items fixed
date +%s%N(was broken on macOS); timestamp-first for sortabilityfile://plugin URLs — opencode plugin URL handles unusual characters in home paths; validated against live Node and Bun encoding oraclescodex_locator.rstest module split into a sibling fileReview status
npm run check🤖 Generated with Amplifier