✨ Introduce v18

[ets-dorian]

No description provided.

[Copilot]

Pull request overview

This PR introduces protocol-versioned parameter parsing by wiring a ParameterParser into reader.Reader via construction-time options, and scaffolds a new (currently stubbed) v18 parameters package.

Changes:

• Add reader.ParameterParser + reader.Options and update reader construction call sites to provide a versioned parameter decoder.
• Refactor Protocol16 parameters into internal/parameters/v16 and update tests/imports accordingly.
• Add a new internal/parameters/v18 package and expose NewParserV16() / NewParserV18() constructors.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
reader.go	Removes top-level Reader re-export/wrapper API.
parser.go	Adds versioned constructors and wires v16/v18 parameter parsers into the underlying reader.
internal/reader/reader.go	Introduces ParameterParser + Options and updates NewReader signature accordingly.
internal/types/parameter.go	Formatting-only change.
internal/session/session_test.go	Updates reader construction to pass v16 parameter parser options.
internal/reader/reader_test.go	Updates reader construction to pass v16 parameter parser options.
internal/parameters/v16/parameters.go	Refactors v16 parsing to implement reader.ParameterParser (but currently breaks receiver state/hook emission).
internal/parameters/v16/string.go	Moves Protocol16 string parsing into package v16.
internal/parameters/v16/arrays.go	Moves Protocol16 array parsing into package v16.
internal/parameters/v16/dicts.go	Moves Protocol16 dict/hashtable parsing into package v16.
internal/parameters/v16/*_test.go	Updates imports/reader construction; still uses the old Parse call shape.
internal/parameters/v18/parameters.go	Adds v18 parameter parser stub (currently returns nil without reading).
internal/hooks/hooks_test.go	Updates imports/reader construction but still calls parameter parse with the old signature.
internal/command/sendReliable/reliable.go	Switches reliable parameter parsing to use the configured reader.ParameterParser.
internal/command/sendReliable/reliable_test.go	Updates reader construction to pass v16 parameter parser options.
internal/command/command_test.go	Updates reader construction to pass v16 parameter parser options.

Comments suppressed due to low confidence (4)

internal/parameters/v16/dicts_test.go:97

• p.Parse(reader) is still using the old 1-arg API, but v16.Parameter.Parse now requires an out *types.Parameter argument. Update these tests to pass an output struct (or add a wrapper) so the suite compiles and the expected dictionary/hashtable values are asserted against the parsed output.
  internal/parameters/v16/parameters.go:58
• Parameter.Parse populates the out parameter but never updates the receiver (p.Parameter), yet emit sends p.Parameter to hooks. This means OnParameter hooks will receive zero-values and callers relying on p.Value/p.ID won't see parsed data. Update the receiver to mirror out (or change emit to emit out) before emitting hooks.
  internal/parameters/v16/parameters_test.go:173
• This test still calls param.Parse(reader) with the old 1-arg signature, but v16.Parameter.Parse now requires an out *types.Parameter argument. This won't compile and also doesn't populate param.Value unless the receiver is updated. Adjust the tests to pass an output struct (or restore a 1-arg wrapper) and assert against that output.
  internal/parameters/v16/arrays_test.go:57
• p.Parse(reader) is still using the previous 1-arg API, but v16.Parameter.Parse now requires an out *types.Parameter argument. Update these tests to pass an output parameter (or provide a compatibility wrapper) so they compile and validate the parsed value correctly.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 31 out of 32 changed files in this pull request and generated 19 comments.

Comments suppressed due to low confidence (1)

internal/parameters/v16/parameters.go:50

• Parse populates out, but emit sends p.Parameter (the receiver’s embedded field), which is never updated here. As a result, hooks will receive a zero-value parameter. Update the receiver (p.ParameterHeader/p.Value) before emitting, or change emit to send the out value.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 40 out of 41 changed files in this pull request and generated 16 comments.

Comments suppressed due to low confidence (6)

internal/command/reliable/reliable.go:62

• Parse returns (nil, nil) when header.Type >= ExchangeKeys, which will also match OperationResponse (0x07) and causes the caller to treat parsing as successful while the reader cursor is left mid-command. This will desynchronize subsequent command parsing. This condition should likely be header.Type == ExchangeKeys (or explicitly handle each type) and, if you intentionally skip parsing, you still need to consume the remaining bytes for this command.
  internal/parameters/v16/parameters.go:61
• Parse populates out, but emit sends p.Parameter (the receiver's embedded field), which is never updated here. As a result, sync/async parameter hooks will receive a zero-value/stale parameter instead of the parsed one. Either assign p.ParameterHeader/p.Value before emitting, or change emit to take and emit *out.
  internal/parameters/v16/parameters_test.go:192
• The failure message uses param.Value/param.Value even though assertions compare out.Value. Since param isn't populated anymore, this error output will be misleading; use out.Value/out.Value in the formatted message.
  internal/command/reliable/reliable.go:120
• In the default branch, reader.ReadBytes(int(length) - 14) ignores the returned error and can pass a negative length (if length < 14), which will panic due to an invalid slice range. Compute the remaining bytes based on the current cursor (e.g., remaining := int(length) - reader.Cursor) after validating length against the command header size, then read and handle any error.
  internal/command/reliable/reliable.go:111
• Return-code and debug-message reads ignore errors (ReadInt16LittleEndian(), ReadByte()), which can silently corrupt cursor position on truncated packets. Capture and return these errors so parsing fails safely and stays synchronized.
  internal/command/reliable/reliable_test.go:25
• This test calls reliable.Parse(ctx) but reliable.Parse now requires a length uint32 argument. As written, this file will not compile; pass the appropriate length (e.g., uint32(len(payload)) if the payload is the full reliable message, or the command length if testing the command path).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.