Repository navigation
Make the GUI say what it actually did - #197
Merged
Merged
Conversation
Three result-correctness defects from the independent review, all of the same shape: the interface reported something the backend did not do. 1. The command preview claimed five ports and scanned three. `buildCommand` substituted the GUI's DEFAULT_PORTS placeholder (22,80,443,8080,8443) whenever the Ports field was empty, while execution sent no port list at all and the core fell back to its own default of 22,80,443. That wrong string is what Ctrl+Shift+C copied, what the History menu recorded, and what was stored with the saved result bundle. `-p` is now omitted when the field is empty -- which is both accurate and what `inspect` and `sweep` in the same function already did. 2. Truncated captures were presented as complete. The core sets `packets_truncated` on a capture and `truncated` on a parse. Neither was read anywhere in the GUI. Opening a 20,000-packet file with the default 1,000 limit showed 1,000 rows under a status bar reading "20000 packets", with nothing saying the view was cut off -- and Export CSV then wrote only the loaded rows. The summary now reads "1000 of 20000 packets shown" when either flag is set. 3. Three call sites disagreed on which ports an inspect checked. The row builder used `ports?.length ? ports : open_ports`; the column set and the status-bar summary used `ports ?? open_ports`. A backend returning `ports: []` with a non-empty `open_ports` rendered rows from `open_ports` while the columns came from an empty list, so every cell was blank. All three now call one `inspectPorts` helper, keeping the length check -- `ports` is optional for older backends and an empty list carries no more information than a missing one. Two smaller preview fixes in the same function: the pcap capture branch omitted `--json` while every other command includes it, and interpolated the BPF filter raw, so a filter containing a quote produced a preview that would not parse if pasted. Every new test was confirmed to fail against the previous behaviour: "expected 'netscli scan router.local -p 22,80,44…' to be 'netscli scan router.local --json'" and "expected '20000 packets' to be '1000 of 20000 packets shown'". The pcap summary cases moved to their own file rather than pushing presentation.test.ts past its size cap -- which is the split its transition exception already asked for. 105 GUI tests pass, lint 0 errors, build and file-size guard clean.
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
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.
Three result-correctness defects from the independent review, all the same shape: the interface reported something the backend did not do.
1. The command preview claimed five ports and scanned three
buildCommandsubstituted the GUI'sDEFAULT_PORTSplaceholder (22,80,443,8080,8443) whenever the Ports field was empty, while execution sent no port list at all and the core fell back to its own default of22,80,443.That wrong string is what Ctrl+Shift+C copied, what the History menu recorded, and what was stored with the saved result bundle.
-pis now omitted when the field is empty — which is both accurate and whatinspectandsweepin the same function already did.2. Truncated captures were presented as complete
The core sets
packets_truncatedon a capture andtruncatedon a parse. Neither was read anywhere in the GUI.Opening a 20,000-packet file with the default 1,000 limit showed 1,000 rows under a status bar reading
20000 packets, with nothing saying the view was cut off — and Export CSV then wrote only the loaded rows. The summary now reads1000 of 20000 packets shown.3. Three call sites disagreed on which ports an inspect checked
rows.tsports?.length ? ports : open_portscolumns.ts,summaries.tsports ?? open_portsA backend returning
ports: []with a non-emptyopen_portsrendered rows fromopen_portswhile the columns came from an empty list — every cell blank. All three now call oneinspectPortshelper, keeping the length check:portsis optional for older backends and an empty list carries no more information than a missing one.Also
Two smaller preview fixes in the same function: the pcap capture branch omitted
--jsonwhile every other command includes it, and interpolated the BPF filter raw, so a filter containing a quote produced a preview that would not parse if pasted.Verification
Every new test was confirmed to fail against the previous behaviour:
The pcap summary cases moved to their own file rather than pushing
presentation.test.tspast its size cap — which is the split its transition exception already asked for.105 GUI tests pass, lint 0 errors, build and file-size guard clean.