Internals Walkthroughs - NormB/sipnab GitHub Wiki

Six ordered checklists for the changes people actually make.

Each step names the test that fails if you skip it.
Where no test enforces a step, it carries (unenforced).
- That is prose, and prose is what gets forgotten.
- So those are the steps to be deliberate about.

Checking the enforcement claims here meant making the change and watching the gate fail. More often than expected, it meant watching it pass.

Where the first draft of this page and reality disagreed, reality won:

Adding a CLI flag does not trip docs_drift_test.
The compiler turns away an MCP tool that holds a lock across an await before clippy gets a word in.
Three of the six checklists turned out to name gates that do not fire at all for the change they sat beside.
- Those steps now carry (unenforced).

The pattern behind all three: a gate that hardcodes its subjects cannot see a new subject.

Examples of hardcoded subjects:
- Three flood scenarios.
- Two output-flag behaviors.
- Until it was rewritten to enumerate the directory, a list of schema filenames.
Such a gate protects what exists from regressing.
It does not notice an addition.
Assume nothing enforces a new thing until you have watched it fail.

Add a CLI flag

Add the #[arg] field to Cli in cli.rs, with a help_heading matching its group.
- → cli_help_test checks the grouping of the ~160-flag help output.
Wire it wherever it takes effect.
- Usually that is plan().
- So the flag becomes part of RunPlan rather than at the point of use.
Give it a row in settings.rs.
- The row names the [section] key it shares with the config file.
  - It also names how the two combine (Merge::Override, Either, Union or Off).
- Or the row names why it has no key (Action, Input, Secret, PerRun).
- A standing setting gets a key, added to config.rs beside its peers.
- → settings::tests::every_flag_has_exactly_one_row fails immediately with not classified: zzz-gate-probe.
- Once the row names a key, the two reference gates in the same module demand two things:
  - docs/cli-reference.md says Config: [section] key.
  - docs/config-reference.md names the flag.
- SIPNAB_SETTINGS_APPLY=1 cargo test --features full --lib settings writes both.
  - Then python3 scripts/build-site-pages.py carries them to the site.
- → config_wiring_test catches a config key that is never read.
Add a test that references the flag by name.
- Any test, anywhere under tests/, including a .trycmd golden.
- → flag_coverage_test fails immediately without one.
  - zzz-gate-probe in its message is the flag you just added.
Document it in at least two examples across the docs.
- → doc_example_coverage_test fails with --your-flag: 0 example(s) until you do.
Add a default-value case to cli_defaults_test.
- (unenforced — that test does not enumerate the CLI, so a missing case passes silently.)

A config key with no flag goes in settings.rs's FILE_ONLY list with its reason.

Verified 2026-10-03:
- A throwaway zzz-gate-probe flag failed every_flag_has_exactly_one_row.
- A throwaway [capture] zzz_probe_key failed two checks:
  - every_config_key_is_named_once (unclassified key).
  - every_config_key_has_a_row_naming_its_flags (no row in docs/config-reference.md).

Verified: adding a throwaway #[arg(long = "zzz-gate-probe")] had these results.

It failed flag_coverage_test and doc_example_coverage_test.
docs_drift_test, cli_defaults_test and config_wiring_test all stayed green.
docs_drift_test guards the other direction.
- That is a flag named in the docs that the CLI does not have.

The two gates that do fire, in the order you meet them:

sequenceDiagram
    autonumber
    participant Dev as you
    participant Cli as src/cli.rs
    participant FC as flag_coverage_test
    participant DE as doc_example_coverage_test
    participant CI

    Dev->>Cli: add #[arg] field
    Dev->>FC: cargo test
    FC-->>Dev: FAIL — flag referenced by no test
    Dev->>Dev: add a test or a .trycmd golden
    Dev->>DE: cargo test
    DE-->>Dev: FAIL — 0 example(s) in the docs
    Dev->>Dev: document it twice
    Dev->>CI: push
    Note over CI: both gates are ratchets — the exemption lists may only shrink

Add a TUI view

Model it on the newest one, StreamLossMap. Its commit touched exactly these places.

Add the variant to View in state.rs.
Add the renderer.
- It is a module beside loss_map.rs.
- Register it in render/mod.rs.
Keep the computation out of the renderer.
- StreamLossMap's binning core lives in rtp/loss_map.rs with its own unit tests.
- So the view logic is testable without a terminal.
Add the controller in controllers/.
- Route it in controllers/mod.rs.
Add the key that opens it, in whatever view it opens from.
Document that key in HELP_TEXT in help.rs and in docs/keybindings.md.
- → keybinding_drift_test fails with these handled KeyCode::Char keys are neither in the F1 help nor allow-listed.
- It names both the view and the character.
Add snapshot coverage in tui_snapshot_test.
- At minimum, the populated case and the empty/degenerate case.
Mirror the keybinding into website/content/docs/keybindings.md.
- (unenforced for a new key — the mirroring obligation is a convention. See CONTRIBUTING.)

Verified: binding KeyCode::Char('Y') in the call-list controller without touching the help failed two assertions in keybinding_drift_test.

Add an MCP tool

Add the handler to server.rs under the #[tool_router] impl.
- Give it a #[tool(name = …, description = …)] attribute.
Write the description as a statement of what the tool returns.
- Never instruct the model to "trust", "verify", "act on" or "ensure" anything about the content.
- That is the prompt-injection rule (D22 in ../design/implementation-plan-phases-8-10.md).
- (Enforced by tests/mcp_tool_descriptions_test.rs, which fails on any of those four words in a tool description.)
- (Corrected 2026-08-05.)
  - This used to call the rule unenforced.
  - It also used to claim server.rs cited a scripts/check-tool-descriptions.sh that does not exist.
  - Both halves are now wrong.
    - server.rs no longer carries the citation.
    - The rule shipped as a Rust test rather than the shell script the plan named.
  - That test also carries every_cited_script_exists.
    - It fails if the dangling citation ever comes back.
  - It now scans every tracked .rs file whole, not the first 40 lines of one.
    - The reason: the narrow version asserted nothing once the citation went away.
Take the store guard, project what you need into owned data, drop the guard, and only then .await.
Bound the response with resolve_limit_with_cap().
- Pass the caller's limit and the server's row_cap.
- Also bound it with the body-bytes cap (--mcp-max-body-bytes moves it).
- A tool that pages with a cursor also takes max_response_bytes.
  - It passes its serialized page, after any fields projection, through SipnabMcp::fit_to_budget.
  - It passes the cursor that resumes after each row.
  - So the whole response stays inside --mcp-max-response-bytes (budget.rs).
- Not resolve_limit().
  - That one takes no ceiling.
  - So a tool wired to it silently ignores --mcp-max-rows.
  - The operator sets a limit and the tool does not honor it.
  - shape.rs says as much at the definition.
    - It is "kept for callers with no configured ceiling".
    - The server is not one of them.
  - This step used to name the wrong function.
    - That is how a recipe written for the reader who does not yet know manufactures the very defect it exists to prevent.
Add an end-to-end case to mcp_stdio_test.
- For HTTP-visible tools, also add one to mcp_http_test.

The step that drops the guard is not a style preference.

The rmcp router requires each handler's future to be Send.
A parking_lot guard held across an await makes it non-Send.
So the build fails on the #[tool] attribute with "future cannot be sent between threads safely".
- That happens before clippy::await_holding_lock (denied at the workspace level) is even consulted.
There are two independent mechanisms. The stricter one is the compiler.

sequenceDiagram
    autonumber
    participant Client as MCP client
    participant Tool as tool handler
    participant DS as DialogStore
    participant Shape as shape.rs

    Client->>Tool: call_tool
    Tool->>DS: read()
    DS-->>Tool: guard
    Tool->>Tool: project into owned summaries
    Tool->>DS: drop(guard)
    Note over Tool: the guard is gone — the future is Send again
    Tool->>Shape: resolve_limit_with_cap, body cap
    Shape-->>Tool: bounded payload
    Tool-->>Client: CallToolResult

Add a security detector

Add the module under src/security/.
- Put it beside scanner_detect.rs or reg_flood.rs.
- Export it from security/mod.rs.
Keep detector state bounded and attacker-keyed maps capped.
- A detector is by definition fed hostile input.
- (unenforced — resource_bounds_test is three hand-written scenarios over DialogStore and RtpStream. It has no way to discover your module.)
Run its check from run_detectors in security/detectors.rs.
- That returns the finding as an Effect for the batch loop to file.
- Emit through the alert engine in alerting.rs rather than printing.
- So every sink (CLI, TUI, fail2ban, MCP security_findings) gets it.
- A new rule name also belongs in two places:
  - SECURITY_FINDING_KINDS (mcp/server.rs).
    - This is the vocabulary security_findings.kinds accepts.
    - A finding filed under a name missing from it is one no caller can filter for.
  - DetectionEngines::armed_kinds (batch.rs).
    - This is what tells an agent the run armed the detector at all.
- (enforced — security_findings_kinds_match_the_names_the_detectors_file_under reads the kind: literals in batch.rs and fails on the difference.)
Add regression coverage to security_test.
- Include the false-positive case.
- A detector that fires on normal traffic is worse than no detector.
- (unenforced — nothing requires a new detector to have a test.)

Verified: a new src/security/ module held an uncapped HashMap<IpAddr, u64>. mod.rs exported it. It left resource_bounds_test at 3/3 and security_test at 38/38. Both steps above are real obligations with no gate behind them. That is exactly why this page writes them down.

If it can transmit anything, it must be opt-in.
- It must respect the HEP-origin restriction (SN-01).
- HEP-sourced packets are ineligible for active response without --hep-allow-kill.

Add an output format

Project through output/model.rs.
- Do not build a bespoke JSON object for a dialog or a stream.
- (unenforced for a new surface — summary_consistency_test pins the three surfaces that already exist, CLI/API/MCP, against DialogSummary/StreamSummary. A fourth writer is invisible to it.)
Add the writer beside json.rs / cli_print.rs.
- Emit through sink.rs.
If it is machine-readable, add or extend a schema in tests/schemas/.
- There is no registration step.
- The all_schemas_compile case in json_schema_test enumerates the directory.
- So a new file becomes a validator the moment it lands.
- A malformed one fails there.
- What is still on you is live-output validation.
  - That means proving the surface actually emits what the schema describes.
Add behavior coverage to output_behavior_test.
- (unenforced — it is two tests about --json-pretty and --call-report exit codes.)

History, and the reason the schema step now reads that way. A deliberately malformed zzz_gate_probe.schema.json was once dropped into tests/schemas/. It left json_schema_test at 6 passed, 0 failed. The test walked a hardcoded list of four filenames and never opened the file. That experiment is what motivated the switch to directory enumeration. The test's own doc comment records it. The model, writer and behavior-coverage steps are still convention with no gate behind them.

Document it in docs/output-formats.md.
- Mirror it into the website page.
- The flag that selects it then owes the CLI-flag checklist above.
A new finding kind or evidence count label in src/analysis.rs changes two published contracts.
- A test holds each one to the table.
- Give the variant its arm in ordinal and meta (or as_str and description for a CountLabel). The compiler insists.
- Give it its place in ALL, which a const assertion checks.
- Then add it to tests/schemas/capture_analysis.schema.json.
  - json_schema_test fails until the kind enum or the counts properties name it.
- Last, the YANG module.
  - Users already hold its current revision, so do not bless over it.
  - Add an entry to REVISIONS in src/analysis/yang.rs with the day's date.
  - Point MODULE_TEXT's include_str! at the new file name.
  - Run SIPNAB_BLESS_YANG=1 cargo test --features full --test yang_module_test.
  - Keep the previous file under yang/ beside the new one.
  - yang_module_test fails until the committed module names the kind.
  - scripts/check-yang.py runs pyang --check-update-from over the pair.

Add a fuzz target

Add fuzz/fuzz_targets/<name>.rs calling one parser entry point on &[u8].
Register the [[bin]] in fuzz/Cargo.toml.
- (unenforced, and this is the trap.)
  - An unregistered file under fuzz_targets/ is not a broken build. It is an invisible one.
  - Cargo never compiles it.
  - So fuzz-check and the pre-push cd fuzz && cargo check both pass.
  - Your target silently never runs.
Seed fuzz/corpus/ with at least one real input.
- (unenforced — despite the name, fuzz_corpus_replay never opens fuzz/corpus/.)
  - It has no filesystem access at all.
  - It drives the parsers with an adversarial seed set and a mutation sweep defined in the file itself.
Add the same entry point to smoke_fuzz_test.
- Coverage-guided fuzzing needs nightly and runs weekly.
- The smoke floor runs in every cargo test.
- It is what actually catches the regression.
When a target finds a crash, commit the reproducer into the corpus in the same change as the fix.
- The seeding step's replay does not read the corpus.
- So add the input to smoke_fuzz_test too, if you want it replayed on every cargo test.

Verified: an unregistered fuzz/fuzz_targets/zzz_gate_probe.rs contained a hard compile error.

It left cd fuzz && cargo check at exit 0.
Registering the same file made the same error fail the build with exit 101.
The gate only ever sees registered targets.