Skip to content

Guard the TCP readback contract with a test - #50

Open
Jepson2k wants to merge 2 commits into
mainfrom
test/tcp-readback-contract
Open

Jepson2k wants to merge 2 commits into
mainfrom
test/tcp-readback-contract

Conversation

@Jepson2k

@Jepson2k Jepson2k commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

tcp_offset() and tcp_transform() have raised on an unanswered readback
since 460e4c7 — but nothing asserted it.

That commit ("Apply full TCP transforms through planning and readback",
2026-09-07) changed the behaviour in passing, while doing something else.
The guarantee arrived unannounced and unguarded, which is how
par6#80 came to report parol6 as
still shipping the sentinel twelve days later: nothing in the commit said it
had stopped, and no test said so either. It was one refactor away from
silently returning to a zero vector.

Why a zero vector cannot be the sentinel

Zero is a legitimate offset — a tool deliberately cleared. A caller handed
[0, 0, 0] as a not-answered sentinel cannot tell "the offset is zero" from
"there is no controller", and a host that adopts the readback quietly erases
the offset the user just set. waldoctl's RobotClient says so in as many
words:

Returns exactly three values, and raises when the controller does not
answer. An implementation must not return a zero vector for an unreachable
controller.

tcp_transform() carries the same rule against an identity transform. The
sibling queries answer None for this condition and are right to — no real
rate or pose is None. These two are typed as plain lists of floats and have
no spare value, so they raise.

par6 landed the guarantee behind a test in par6#80. This is the parol6 side of
it.

The test

One integration test, through the real client against the simulator — no
fakes. It proves the two cases are distinguishable, which is the entire
point of the contract:

  • a deliberate set_tcp_offset(0, 0, 0) reads back as the value
    [0.0, 0.0, 0.0]
  • both readbacks raise TimeoutError against a port nothing is listening on

Not yet verified against the violation. The behaviour is already correct on
main, so this test is born green, and a born-green regression test proves
nothing until it is shown to fail against the bug. I will reintroduce the
zero-vector return locally, confirm the test fails, and report the result
here before this merges.

Not changed

  • The dry-run client. DryRunRobotClient.tcp_offset() returns the
    planner's own offset with no controller in the picture; its zero is real,
    not a sentinel.
  • No production code. The behaviour is already correct — this adds only
    the guard that was missing.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TvTCMA9Y9RUaVN33JhHCNQ

Jepson2k and others added 2 commits September 20, 2026 08:53
`tcp_offset()` and `tcp_transform()` have raised on an unanswered
readback since 460e4c7, but nothing asserted it. That commit changed the
behaviour while applying full TCP transforms through planning and
readback, so the guarantee arrived unannounced and unguarded — one
refactor away from silently returning to a zero vector.

Zero is a legitimate offset. A caller handed `[0, 0, 0]` as a
not-answered sentinel cannot tell "the offset is zero" from "there is no
controller", and a host that adopts the readback quietly erases the
offset the user just set. waldoctl's `RobotClient` forbids exactly that,
and par6 carries the same guarantee behind a test (par6#80); this is the
parol6 side of it.

The test drives the real client against the simulator and proves the two
cases are distinguishable: a deliberate `set_tcp_offset(0, 0, 0)` reads
back as the value `[0, 0, 0]`, while both readbacks raise `TimeoutError`
against a port nothing is listening on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvTCMA9Y9RUaVN33JhHCNQ
The test asserted `set_tcp_offset` answered 1, the system-command code.
It answers a command index: `SET_TCP_OFFSET` sits in `QUEUED_CMD_TYPES`
and lands in queue order, not on arrival. Every test job failed on
`assert 264 == 1`, and the readback that followed raced the queue it had
not waited for.

Wait for the index, and set a non-zero offset before clearing it, so the
zero the test reads back is one the controller was told to hold rather
than the state it happened to start in.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvTCMA9Y9RUaVN33JhHCNQ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant