Repository navigation
feat: implement M2 contracts (issues 5, 6, 7, 8) - #26
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a structured sentinel error taxonomy under pkg/coreerr and integrates it across target parsing, exporters, and the scanning engine. It also enhances the scanning engine to support rich lifecycle events (start, complete, cancel, warning) and updates the documentation and quickstart examples accordingly. The review feedback recommends wrapping errors returned by parseCIDR with coreerr.ErrInvalidInput to maintain error consistency, and updating the event documentation to explicitly warn consumers that callbacks are invoked concurrently and must be thread-safe.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if strings.Contains(input, "/") { | ||
| return parseCIDR(input) |
There was a problem hiding this comment.
The error returned by parseCIDR is returned directly without being wrapped by coreerr.ErrInvalidInput. This breaks the structured error taxonomy for CIDR parsing errors, making it impossible for callers to detect them using errors.Is(err, coreerr.ErrInvalidInput).
Please wrap the error returned by parseCIDR to ensure consistent error handling.
| if strings.Contains(input, "/") { | |
| return parseCIDR(input) | |
| if strings.Contains(input, "/") { | |
| ips, err := parseCIDR(input) | |
| if err != nil { | |
| return nil, fmt.Errorf("%w: %v", coreerr.ErrInvalidInput, err) | |
| } | |
| return ips, nil | |
| } |
| 1. **Non-blocking callbacks:** Event callbacks are executed synchronously by the engine's worker goroutines. Consumers **must not** perform long-blocking operations in the callback. If processing requires heavy I/O, dispatch it to a channel or separate goroutine. | ||
| 2. **Thread safety:** The `Device` pointer in `EventResult` points to a copied value, but consumers should avoid mutating it. |
There was a problem hiding this comment.
The onEvent callback is invoked concurrently from multiple worker goroutines. It is crucial to explicitly document this behavior so that consumers know they must implement thread-safe callbacks when mutating shared state or updating UI components.
| 1. **Non-blocking callbacks:** Event callbacks are executed synchronously by the engine's worker goroutines. Consumers **must not** perform long-blocking operations in the callback. If processing requires heavy I/O, dispatch it to a channel or separate goroutine. | |
| 2. **Thread safety:** The `Device` pointer in `EventResult` points to a copied value, but consumers should avoid mutating it. | |
| 1. **Non-blocking callbacks:** Event callbacks are executed synchronously by the engine's worker goroutines. Consumers **must not** perform long-blocking operations in the callback. If processing requires heavy I/O, dispatch it to a channel or separate goroutine. | |
| 2. **Thread safety:** The `onEvent` callback is invoked concurrently from multiple worker goroutines. Consumers **must** ensure that the callback implementation is fully thread-safe (e.g., by using mutexes or channels) when mutating shared state or updating UI components. Additionally, while the `Device` pointer in `EventResult` points to a copied value, consumers should avoid mutating it. |
This PR closes issues #5, #6, #7, and #8 by introducing a structured error taxonomy, schema versioning, establishing JSON as the canonical export format, and standardizing scan lifecycle events.