Repository navigation
refactor(users): obtain resource tokens via User#resource_token from models - #452
Merged
Merged
Conversation
…m models Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The specs run next to a real core container, and core owns each system's
`system/<id>` module lookup hash in redis. On every ControlSystem change
event it clears that hash, rebuilds it from the modules' resolved names and
publishes `lookup-change`. It does this asynchronously, and placeos-resource
handles each changefeed event in its own fiber, so a system's create and
update events can finish in either order.
Several specs created a system and then wrote the lookup into redis by hand.
Core's rebuild could land at any point after that, wiping the hand-written
entry. Generator modules also get a random Faker `custom_name`, so core's
version (e.g. "card/1") never matched the seeded key ("PublicEvents/1").
Logging the hash in the register spec shows it: {"PublicEvents/1" => id}
at request time, {"card/1" => id} three seconds later.
- public_events register (404) and chat_gpt_plugin MCP endpoint (missing
prompt): the "PublicEvents/1" / "LLM/1" lookup was gone by the time the
request ran.
- websocket bind (got 2 of 3 updates): core's rebuild removed the module's
lookup and published `lookup-change`. Session's subscriptions remapped
the binding, found no module, unsubscribed, and later publishes were
never delivered. Delaying the first publish by 3s makes it fail every
time.
- systems functions (found in a full run with seed 41906): same race.
Fix: save each system once, with its final module list, so core gets a
single event. Give modules the resolved name the spec expects. Then wait
for core's mapping with the new `wait_for_module_lookups` helper instead of
seeding it. That waits on the actual condition, so no sleeps are needed.
The websocket debug/ignore specs also flaked, for a separate reason. They
sent the module's resolved name, which is a Faker noun such as "hard
drive". Session#debug treats that value as a module id and puts it in
core's debug websocket path, so a name with a space gets a 400 handshake.
They now send the module id, which is what the API expects.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`::Spec.before_each` registers a hook on the root context, so every one
written inside a `describe` ran before every example in the whole suite.
Before each example, ControlSystem/Module/Driver, Storage, signage, oauth
and group tables were each wiped many times over. Each ControlSystem delete
also sent core a changefeed event, adding to the core lag behind the lookup
races fixed in the previous commit.
- Every `::Spec.before_each` inside a describe is now a plain `before_each`
that runs only for that describe. The module-level hook in systems_spec
moves inside `describe Systems`.
- Group clearing really is needed suite-wide. Group grants attach to the
shared, cached spec users, so a group left over from an earlier example
gives a "regular" user permissions it shouldn't have. With the hooks
scoped, asset_categories "fails to create if a regular user" got 201
(seed 41906). helper.cr's global hook now calls `clear_group_tables`
once, and the 17 duplicate `before_each { clear_group_tables }` hooks
are removed. A static check found that all 315 group-creating examples
already sit under a group-clearing hook.
- signage_ai_providers_spec builds the domain's storage through
`setup_signage_ai`. It only avoided leftover storages because uploads_spec
cleared the table globally; without that it collided with a leftover
(seed 43126). Its hook now clears playlist items, uploads and storages,
as signage_ai_spec's does.
- uploads_spec: the two storages in the storage_id/default listing specs
took bucket names from Faker's 24 hacker nouns, so they collided about 1
time in 24 ("authority_id need to be unique"). They now use distinct
random bucket names.
- systems_spec "with core": the state specs wrote the lookup by hand for a
system created at load time, which core's rebuild (or a delete event)
could wipe. They now use the save-once + `wait_for_module_lookups`
pattern through a shared `mapped_module_system` helper, as the functions
spec does. The `get_sys`/`get_driver`/`setup_system` helpers are no
longer used, so they are removed.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The concurrent deletes cascade into one another and can deadlock, which aborted a CI run after every example had passed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The chat manager pings every socket on a 30s timer, so a ping could land during the example and be counted as an update. Wait for the expected signals rather than sleeping a fixed interval. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
POST /users/resource_token,POST /users/:id/resource_tokenand the MS token exchange now callUser#resource_token, which has moved into placeos-models (feat(user): add User#resource_token for delegated SSO access models#333). staff-api uses the same method instead of calling this endpoint (refactor: remove rest-api client, access PlaceOS directly staff-api#391).Users::AccessTokenis now an alias ofModel::User::ResourceToken, so the response JSON is unchanged.Model::Error::NoResourceToken. This PR maps that to a 404, as before.Dependencies
Uses
User#resource_tokenfrom placeos-models 9.119.0 (PlaceOS/models#333).Spec stabilisation
These CI flakes were unrelated to the change but fixed here:
coreoverwrote hand-written Redis lookups (system/<id>). It rebuilds each system's module lookup asynchronously after every ControlSystem change, wiping entries the specs had written themselves. This brokepublic_eventsregister (404),chat_gpt_pluginMCP, websocket bind (missing updates) andsystemsfunctions/state. Each spec now saves the system once with its final modules and waits for core's mapping (wait_for_module_lookups).::Spec.before_eachinsidedescribeblocks registered globally. It cleared tables before every example in the suite. The hooks are now scoped to their describe. Group tables are still cleared globally inspec/helper.cr, because group grants attach to the shared spec users.uploadsstorages could collide on random bucket names.Test plan
./test spec/controllers/users_spec.cr: 45 examples, 0 failures. New examples cover returning a stored token and the 404 when no token is available.🤖 Generated with Claude Code