Add 11-4-2 ExecPlan doc and tests; mark 12.4.2 roadmap done

[leynos]

Summary

• Adds the 11-4-2 troubleshooting ExecPlan document and updates associated tests, fixtures, and documentation to align with the expanded guidance. Also updates the roadmap to reflect 12.4.2 completion.

Changes

• Added the new ExecPlan document: docs/execplans/11-4-2-troubleshooting-section.md (472 insertions).
• Updated docs/roadmap.md to mark 12.4.2 as done.
• Expanded the user guide troubleshooting content in docs/users-guide.md to cover codec length mismatch, preamble errors, and TLS issues (en-GB spelling and style).
• Updated docs/wireframe-client-design.md to reflect decisions and TLS handling guidance.
• Expanded test coverage and fixtures to exercise the new troubleshooting guidance:
  • Updated tests/client_runtime.rs and tests/client_preamble.rs for TLS mismatch and invalid preamble scenarios.
  • Updated tests/features/{client_runtime,client_preamble}.feature files with new scenarios for TLS mismatch and invalid acknowledgement bytes.
  • Updated test fixtures under tests/fixtures/{client_runtime, client_preamble}.rs to implement new test servers (invalid acknowledgement, TLS-like response bytes).
  • Updated test scenarios under tests/scenarios for new scenarios.
• Misc: ensured references to docs and tests align with the 11.4.2 ExecPlan and existing public error surface.

Rationale

• Aligns with roadmap item 11.4.2 by providing an expanded, testable ExecPlan and updated user guidance that covers the three misconfiguration classes.
• Moves the troubleshooting guidance from a terse bullet list to structured guidance with clear symptoms, detection steps, and fixes.
• Establishes test coverage (unit/integration via rstest and rstest-bdd) to prove the documented signals.
• Updates the roadmap and design reflection to keep all related docs in sync.

Implementation details

• This is a multi-faceted change: documentation plus test harnesses and non-production test code. No public API changes are introduced, but test harness updates reflect new observable behaviours in the client’s troubleshooting surface.
• The ExecPlan documents how Stage A–E will be executed and validated, and references the user guide, design doc, roadmap, and tests for alignment.
• TLS guidance is framed as misconfiguration diagnostics rather than built-in TLS support.

Review guidance

• Verify the ExecPlan document at docs/execplans/11-4-2-troubleshooting-section.md exists and references the three misconfiguration classes.
• Ensure Stage A–E are defined with concrete actions and checks.
• Check alignment with user guide and design/docs references (docs/users-guide.md, docs/wireframe-client-design.md, docs/roadmap.md).
• Confirm the new test scenarios and fixtures reflect the documented behaviours and use the existing test harness where possible.
• Confirm en-GB spelling and overall documentation style.

Testing plan

• This is a documentation+test change; unit/integration tests and behavioural tests have been added/updated to cover the new diagnostics (rstest and rstest-bdd). Future PRs will gate and validate the Stage-by-Stage milestones (A–E).

Impact

• This change touches documentation, test harnesses, and non-production test code. No public API changes.

Related work

• Roadmap item: 11.4.2
• References in the ExecPlan to:
  • docs/users-guide.md
  • docs/wireframe-client-design.md
  • docs/roadmap.md
  • src/client/error.rs and src/client/preamble_exchange.rs (public error surface)
  • tests/ (runtime and preamble) and tests/features (BDD scenarios)

Acceptance criteria mapping

• A substantive ExecPlan document for 11.4.2 is added.
• The plan enumerates misconfiguration classes (codec length mismatch, preamble errors, TLS issues).
• Stage A–E implementation path is defined with concrete actions and checks.
• References to user guide and design/docs alignment are included.
• Documentation+test changes; no API changes introduced.
• Roadmap entry 12.4.2 completed.

📎 Task: https://www.devboxer.com/task/0a1972f1-c1f8-4f65-a2f5-d0aaf5a1785b

📎 Task: https://www.devboxer.com/task/4d51b4f8-7b1d-4ecd-a70a-ea3c9f79d24b

[coderabbitai]

Walkthrough

Add an ExecPlan and extend troubleshooting documentation; map specific ClientError variants to diagnostic checks; add integration and BDD tests for preamble read failures and TLS/protocol-mismatch transport errors; extend test fixtures and helpers to support these scenarios.

Changes

Cohort / File(s)	Summary
ExecPlan & Roadmap docs/execplans/11-4-2-troubleshooting-section.md, docs/roadmap.md	Add a new ExecPlan for roadmap item 11.4.2 and mark the troubleshooting task as completed in the roadmap.
User-facing docs docs/users-guide.md, docs/wireframe-client-design.md	Expand troubleshooting guidance to map observed failure surfaces to ClientError variants (PreambleRead, PreambleTimeout, PreambleEncode, Wireframe I/O). Add a decision record clarifying TLS/protocol mismatch treatment.
Integration tests — preamble tests/client_preamble.rs, tests/features/client_preamble.feature	Add test asserting invalid server acknowledgement bytes surface as ClientError::PreambleRead. Add BDD scenario for the case. Update Tokio imports for write support.
Integration tests — runtime / TLS mismatch tests/client_runtime.rs, tests/features/client_runtime.feature	Add test and BDD scenario asserting non‑Wireframe TLS-like replies surface as ClientError::Wireframe(WireframeError::Io(_)). Update async I/O imports.
Test fixtures — preamble tests/fixtures/client_preamble.rs	Refactor server spawn helpers (spawn_server, spawn_server_after_preamble), add start_invalid_ack_server, connect_with_invalid_ack, make_failure_signal_callback, and assertion helper was_preamble_read_error. Record connect results and failure-callback invocation consistently.
Test fixtures — runtime tests/fixtures/client_runtime.rs	Add start_tls_mismatch_server to spawn a TCP server that replies with hard-coded TLS-like bytes to simulate protocol mismatch.
BDD scenarios tests/scenarios/client_preamble_scenarios.rs, tests/scenarios/client_runtime_scenarios.rs	Add scenario functions client_preamble_invalid_acknowledgement and client_runtime_tls_mismatch to wire new feature scenarios to test code.
BDD step definitions tests/steps/client_preamble_steps.rs, tests/steps/client_runtime_steps.rs	Add steps to start invalid-ack and TLS-mismatch servers, trigger client connections expecting specific failure kinds, and assert preamble-read or transport-error outcomes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🔧 When frames miscount and preambles go astray,
The client reports what went wrong that day.
Map faults to errors, write tests that prove—
Troubleshoot the path and tighten the groove. ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title directly and clearly summarizes the main changes: adding the 11-4-2 ExecPlan document, related tests, and marking roadmap item 12.4.2 complete.
Description check	✅ Passed	The description comprehensively covers all changes in the PR, including documentation updates, test additions, rationale, implementation details, and acceptance criteria.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch add-troubleshooting-docs-hc72km

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sourcery-ai]

Reviewer's Guide

Adds an ExecPlan for roadmap item 11.4.2, expands client troubleshooting docs for codec-length, preamble, and TLS misconfigurations, and extends the client preamble/runtime test harnesses and BDD scenarios to prove the documented behaviours without changing public APIs.

Sequence diagram for updated client preamble error mapping

sequenceDiagram
    participant Client
    participant PreambleModule
    participant Server

    rect rgb(235,235,255)
    Client->>PreambleModule: write_preamble(preamble_bytes)
    PreambleModule->>Server: send preamble_bytes over TCP
    alt write succeeds
        Server-->>PreambleModule: ack or invalid bytes
        PreambleModule-->>Client: return
    else write I_O_or_encode_failure
        PreambleModule-->>Client: Err(EncodeError)
        Client-->>Client: map to ClientError::PreambleEncode
    end
    end

    rect rgb(235,255,235)
    Client->>PreambleModule: read_preamble()
    alt valid acknowledgement bytes
        Server-->>PreambleModule: valid preamble ack
        PreambleModule-->>Client: Ok(ack, leftover_bytes)
    else invalid_or_malformed_bytes
        Server-->>PreambleModule: invalid ack bytes
        PreambleModule-->>Client: Err(read_or_decode_failure)
        Client-->>Client: map to ClientError::PreambleRead
    end
    end

    rect rgb(255,235,235)
    Client->>PreambleModule: preamble_exchange with timeout
    par server responds before timeout
        Server-->>PreambleModule: response bytes
        PreambleModule-->>Client: Ok
    and timeout expires first
        Client-->>Client: detect timeout expiry
        Client-->>Client: map to ClientError::PreambleTimeout
    end
    end

Loading

File-Level Changes

Change	Details	Files
Refactor client preamble test fixture to share server setup logic and capture new preamble failure signals.	Introduce generic spawn_server and spawn_server_after_preamble helpers to bind, accept and hand off TcpStreams for tests. Add helpers to store connect results with or without a failure-signal channel, centralising client/error bookkeeping and timeout-based failure-callback tracking. Add start_invalid_ack_server and connect_with_invalid_ack paths to simulate malformed acknowledgement bytes and assert PreambleRead errors. Expose was_preamble_read_error and wire new BDD steps and scenarios to exercise invalid acknowledgement behaviour and failure-callback invocation.	tests/fixtures/client_preamble.rs tests/steps/client_preamble_steps.rs tests/scenarios/client_preamble_scenarios.rs tests/features/client_preamble.feature
Extend client runtime fixtures and tests to model TLS/wrong-protocol endpoints and assert error mapping.	Add a start_tls_mismatch_server helper that reads a request and responds with TLS-record-like bytes, logging failures but keeping tests deterministic. Add a unit test that connects a Wireframe client to a TLS-mismatch server and asserts the error surfaces as ClientError::Wireframe(WireframeError::Io(_)). Add matching BDD step, scenario, and feature wiring to describe the TLS port mismatch as a transport error in user-facing language.	tests/client_runtime.rs tests/fixtures/client_runtime.rs tests/steps/client_runtime_steps.rs tests/scenarios/client_runtime_scenarios.rs tests/features/client_runtime.feature
Clarify and expand troubleshooting documentation for client errors, aligning it with the actual error variants emitted by the implementation.	Update the user guide client troubleshooting section to describe codec length mismatch, preamble timeout/read/encode failures, TLS or wrong-protocol port mismatches, correlation mismatches, streaming contention, and transport disconnects with concrete error variants and remedies. Clarify how preamble success and failure callbacks surface errors: mapping callback read failures to ClientError::PreambleRead, timeouts to ClientError::PreambleTimeout, and write failures to ClientError::PreambleEncode given write_preamble’s EncodeError return type. Update the client design document troubleshooting bullets to mirror the user guide, explicitly noting that PreambleWrite is not currently a user-visible signal and framing TLS guidance as wrong-protocol diagnostics rather than native TLS support.	docs/users-guide.md docs/wireframe-client-design.md
Add an ExecPlan for roadmap item 11.4.2 and mark the roadmap task as complete.	Introduce docs/execplans/11-4-2-troubleshooting-section.md capturing purpose, constraints, risks, staged implementation plan, decisions, and validation outcomes for expanding client troubleshooting and diagnostics. Update docs/roadmap.md to mark item 12.4.2 (client troubleshooting section for common misconfigurations) as done, reflecting the new docs and tests.	docs/execplans/11-4-2-troubleshooting-section.md docs/roadmap.md
Add focused tests for invalid preamble acknowledgement handling and tighten mapping to PreambleRead errors.	Add a new test in tests/client_preamble.rs that drives a preamble-aware server returning junk bytes; assert the client surfaces ClientError::PreambleRead and that the failure callback observes the same variant. Refactor on_preamble_failure wiring in the fixtures to reuse a shared make_failure_signal_callback helper, reducing duplication and ensuring consistent signalling semantics.	tests/client_preamble.rs tests/fixtures/client_preamble.rs

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[leynos]

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

tests/fixtures/client_preamble.rs

Comment on file

    pub async fn start_invalid_ack_server(&mut self) -> TestResult {
        let listener = TcpListener::bind("127.0.0.1:0").await?;
        let addr = listener.local_addr()?;
        let handle = tokio::spawn(async move {
            use tokio::io::AsyncWriteExt;

            let (mut stream, _) = listener.accept().await.expect("accept");
            let (_preamble, _) = read_preamble::<_, TestPreamble>(&mut stream)
                .await
                .expect("read preamble");
            stream
                .write_all(&[0xff, 0x00, 0x01])
                .await
                .expect("write invalid acknowledgement");
        });

        self.addr = Some(addr);
        self.server = Some(handle);
        Ok(())
    }

❌ New issue: Code Duplication
The module contains 6 functions with similar structure: ClientPreambleWorld.connect_with_invalid_ack,ClientPreambleWorld.connect_with_timeout,ClientPreambleWorld.start_ack_server,ClientPreambleWorld.start_invalid_ack_server and 2 more functions

[leynos]

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response.

tests/fixtures/client_preamble.rs

Comment on file

        let addr = listener.local_addr()?;
        let handle = tokio::spawn(async move {
            let (mut stream, _) = listener.accept().await.expect("accept");
        self.spawn_server(|mut stream| async move {

❌ New issue: Code Duplication
The module contains 4 functions with similar structure: ClientPreambleWorld.connect_with_invalid_ack,ClientPreambleWorld.connect_with_timeout,ClientPreambleWorld.start_ack_server,ClientPreambleWorld.start_invalid_ack_server

[leynos]

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response.

Code Duplication

tests/fixtures/client_preamble.rs:

What lead to degradation?

The module contains 4 functions with similar structure: ClientPreambleWorld.connect_with_invalid_ack,ClientPreambleWorld.connect_with_timeout,ClientPreambleWorld.start_ack_server,ClientPreambleWorld.start_invalid_ack_server

Why does this problem occur?

Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health.

How to fix it?

A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More
Once you have identified the similarities across functions, look to extract and encapsulate the concept that varies into its own function(s). These shared abstractions can then be re-used, which minimizes the amount of duplication and simplifies change.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4269272de0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/fixtures/client_preamble.rs (1)

1-482: ⚠️ Potential issue | 🟡 Minor

Extract signal helpers to a shared test utilities module.

This file is 482 lines, exceeding the 400-line ceiling by 82 lines. Move create_signal_channel, send_signal, and make_failure_signal_callback (lines 66–89) into a shared test_helpers or test_utils module to reduce coupling and allow reuse in other fixture tests. This extraction alone reduces the file to approximately 440 lines; further refactoring of ClientPreambleWorld setup methods may be warranted if additional reduction is required.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/fixtures/client_preamble.rs` around lines 1 - 482, Extract the three
signal helpers create_signal_channel, send_signal, and
make_failure_signal_callback into a shared test utilities module (e.g.,
test_helpers or test_utils) and update this fixture to import them;
specifically, move the exact implementations (keeping generic signatures and
SenderHolder<T> type alias) out of this file, expose them via pub (or
pub(crate)) functions/alias, then replace the local definitions with use
statements and remove the originals in ClientPreambleWorld; update all call
sites in this file (e.g., in connect_with_preamble, connect_with_ack,
connect_with_timeout, connect_with_invalid_ack) to use the imported helpers so
behavior and types (oneshot::Receiver, SenderHolder, and callback return types)
remain unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/users-guide.md`:
- Around line 2281-2282: The sentence fragment should read "apply a reconnect or
retry policy" instead of "apply reconnect or retry policy"; update the
documentation text that mentions ClientError::Wireframe(WireframeError::Io(_))
to insert the missing preposition "a" before "reconnect" so the clause becomes
"treat ClientError::Wireframe(WireframeError::Io(_)) as a network-level or
peer-closure failure and apply a reconnect or retry policy at the call site."
- Around line 2262-2264: The docs string contains British Oxford spelling
requirement but uses "serialising" in the sentence mentioning
write_preamble(...) and PreambleWrite; update the text to use "serializing"
(i.e., replace "serialising" with "serializing") where it appears in the
sentence that reads "completion while serialising or writing the preamble..." so
the document adheres to the en-GB-oxendict -ize convention.

In `@tests/features/client_runtime.feature`:
- Line 29: Rename or replace the misleading Gherkin step "When the client sends
an oversized payload of 32 bytes" in tests/features/client_runtime.feature to
clearly reflect the TLS-mismatch/transport-error intent (e.g. "When the client
sends a payload expecting a transport error" or "When the client sends a payload
to trigger a TLS mismatch error") and update its corresponding step definition
handler (the step implementation that matches the original string) to use the
new step text so the scenario still invokes the same client send logic; ensure
the string used in the feature and the step-definition function/matcher are kept
in sync.

---

Outside diff comments:
In `@tests/fixtures/client_preamble.rs`:
- Around line 1-482: Extract the three signal helpers create_signal_channel,
send_signal, and make_failure_signal_callback into a shared test utilities
module (e.g., test_helpers or test_utils) and update this fixture to import
them; specifically, move the exact implementations (keeping generic signatures
and SenderHolder<T> type alias) out of this file, expose them via pub (or
pub(crate)) functions/alias, then replace the local definitions with use
statements and remove the originals in ClientPreambleWorld; update all call
sites in this file (e.g., in connect_with_preamble, connect_with_ack,
connect_with_timeout, connect_with_invalid_ack) to use the imported helpers so
behavior and types (oneshot::Receiver, SenderHolder, and callback return types)
remain unchanged.

🪄 Autofix (Beta)

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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 90f11db3-aaac-4593-8bf8-15830d584cfe

📥 Commits

Reviewing files that changed from the base of the PR and between bb1e2e1 and a72eaa9.

📒 Files selected for processing (14)

• docs/execplans/11-4-2-troubleshooting-section.md
• docs/roadmap.md
• docs/users-guide.md
• docs/wireframe-client-design.md
• tests/client_preamble.rs
• tests/client_runtime.rs
• tests/features/client_preamble.feature
• tests/features/client_runtime.feature
• tests/fixtures/client_preamble.rs
• tests/fixtures/client_runtime.rs
• tests/scenarios/client_preamble_scenarios.rs
• tests/scenarios/client_runtime_scenarios.rs
• tests/steps/client_preamble_steps.rs
• tests/steps/client_runtime_steps.rs

[leynos]

@coderabbitai Have the following now been resolved?

Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity.

Please address the comments from this code review:

## Overall Comments
- The TLS-mismatch server logic is duplicated between `tests/client_runtime.rs` and `tests/fixtures/client_runtime.rs::start_tls_mismatch_server`; consider consolidating this into a single helper or fixture to avoid drift if the TLS-like bytes or behaviour ever change.
- Similarly, the invalid preamble acknowledgement server behaviour is now implemented both inline in `tests/client_preamble.rs` and via `ClientPreambleWorld::start_invalid_ack_server`; it would be cleaner to reuse the fixture or extract a shared helper for writing the invalid ack bytes.
- The TLS record byte sequence is hard-coded in multiple places; promoting it to a named constant (e.g. in a test helper module) would make future adjustments clearer and reduce the chance of inconsistent protocol-mismatch simulations.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/users-guide.md`:
- Around line 2268-2269: Edit the sentence that reads "usually surfaces as
`ClientError::Wireframe(WireframeError::Io(_))`, after the first request,
because the bytes returned are not valid Wireframe frames." and remove the comma
before "because" (and the extraneous comma after the parenthesized error if
present) so it reads without the comma interrupting the clause; target the
fragment containing `ClientError::Wireframe(WireframeError::Io(_))` to locate
the sentence.

🪄 Autofix (Beta)

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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5cadeacb-957a-4733-a3af-a5a4fc8decd1

📥 Commits

Reviewing files that changed from the base of the PR and between a72eaa9 and f26ca67.

📒 Files selected for processing (3)

• docs/users-guide.md
• tests/features/client_runtime.feature
• tests/steps/client_runtime_steps.rs