Repository navigation
test: cover device pairing and login codes end to end - #7362
Conversation
97b3e7a to
8b0293c
Compare
8b0293c to
e56ac65
Compare
|
/review |
|
Claude finished @otavio's task in 4m 26s —— View job Code Review CompleteReviewed 10 files and posted 3 inline comments, all Standards judgement calls. There are no rule violations and no Correctness issues. The Spec axis did not run. StandardsThe worst of the three is the "no device has this hostname" loop, written out four times. The first copy is at
SpecThe Spec axis did not run. The PR names only shellhub-io/team#243, which is in another repository this run cannot read, and it closes no issue in this repo. Additional notes
To request another review round, comment |
e56ac65 to
79eb75c
Compare
|
/review |
|
Claude finished @otavio's task in 2m 12s —— View job Code Review CompleteReviewed 11 files on two axes: Standards (repo rules, language conventions, tests, code smells) and Correctness (security and runtime defects). No new issues found. The code looks good as-is. The Spec axis did not run. The PR cites only shellhub-io/team#243, which is in a repository this run cannot read, and no commit carries a |
79eb75c to
f0357a2
Compare
The server gives pairing and login codes a ten minute lifetime, too long to wait out in CI, so the stack learns the redis service and a helper sets the time a cache entry has left. The entry still expires on the cache's own clock, and each test first checks the code still answers, so a 404 afterwards reads as expiry and not as a key that was never there. The accept body is a map rather than requests.DevicePairingAccept. Its Code field carries only a param tag, so marshalling the struct sends "Code":"", the body binding empties the code the path set, and the validator answers 400. The agent writes text logs whatever SHELLHUB_LOG_FORMAT says, so the agent tests match its connection in the text line. The device reads online as soon as the accept authenticates it on the agent's behalf, so only the agent's log shows it connected. The polling case counts the agent's start lines, since a restarted agent learns its tenant from a fresh pairing request instead of from the poll.
A login code is what an agent that already has a tenant prints for its pending device. The cases create codes and poll status with the device's own token, and resolve codes as the namespace's owner, as the accept page does.
A code outside the alphabet never reaches the cache on its own, since the server mints codes only from the alphabet, so a lookup of one is a miss whether or not the server checks it. The test copies a real code's entry under the out-of-alphabet key first, so the 404 shows the check refused it. It asserts accept and resolve only: the pairing status route looks the code up without checking the alphabet, and a miss is the only answer it can give in practice.
The namespace accepts one paired device before the limit is set, because a limit of zero reads as none. Setting the limit to the accepted count moves into a helper the enrollment limits test now shares. A refused accept still leaves a pending device under the pairing's hostname in the namespace. The test does not assert on it either way: that device is a defect to fix in the server, not a behaviour to pin.
The accept page sits outside the app layout and shows no namespace tab, so the switch shows in the page reaching its accept step, which it renders only once the session's tenant is the device's, and in the tab the device page opens under afterwards.
f0357a2 to
300b390
Compare
Summary
Covers Domain 9 (Device Pairing) of shellhub-io/team#243. Stacked on #7360 for its order in the
stack only; it builds on the helpers from #7353.
The agent tests start an agent with no tenant. They read its pairing code from the log, accept it
as the owner, and check the agent connects without a restart and keeps the tenant in its
.tenantfile. Expiry lowers the cache entry's TTL to five seconds and waits for the cache todrop it. Each expiry test first checks the code still answers, so the 404 afterwards comes from
expiry.
The four
[both]items get testcontainers tests only. The API layer asserts each claim, and allthat is left for the UI is rendering, which the issue leaves to Vitest with MSW. The
namespace-switch spec skips community, like the other namespace-switching specs, so it runs in the
enterprise and cloud e2e jobs.
Evidence
Before: none of the 15 Domain 9 items had a test.
After:
go test ./...intests/passes (1234s), and so do the focused runs after review:Each test was seen failing against a broken server, rebuilt into the test stack:
pairing/pairing again with the same public key returns the pending codepairingcode.Normalizereturns its inputcode format/a code is read without its hyphen and in any caselogin code/a new code invalidates the one before itvalidateDeviceAcceptancedevice limit/a namespace at its device limit refuses a pairing until the limit is liftedpairingcode.IsValidaccepts any 8 characterscode format/a code with a character outside the alphabet is not found, even when the cache holds itAcceptDeviceFlownever switches namespaceaccepting a device by its login code › the console switches to the namespace of a device in another oneThe Playwright spec was run on community with its skip lifted, since this checkout holds no
enterprise license: it passes, and fails with the switch disabled.
Merge Danger
Door: two-way
Tests only. The one stack change: every testcontainers stack now registers the
redisservice,which all editions' compose files already run.
Blast Radius: tests
A refused accept at the device limit still leaves a pending device with the pairing's hostname in
the namespace. If someone later accepts that device from the device list, the agent never learns
it was accepted: the pairing status stays pending. The tests leave that unasserted; the fix
belongs in the server.