H-4347: Move logic to create system account to Graph

[TimDiekmann]

🌟 What is the purpose of this PR?

To allow creating bootsrapping permissions we need a way to create a system account from the graph. This moves the current logic to the graph.

🚫 Blocked by

• H-4345: Prepare evaluation of policies when calling from API #6880
• H-1063, H-4344: Rename Team->ActorGroup, Subteam->Team, OwnedById->WebId #6881

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

• does not modify any publishable blocks or libraries, or modifications do not need publishing

📜 Does this require a change to the docs?

The changes in this PR:

• are internal and do not require a docs change

🕸️ Does this require a change to the Turbo Graph?

The changes in this PR:

• do not affect the execution graph

[Copilot]

Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

• libs/@local/graph/api/openapi/openapi.json: Language not supported

Comments suppressed due to low confidence (1)

apps/hash-api/src/graph/knowledge/system-types/user.ts:289

• The removal of the conditional logic that checks for an existing userAccountId in createUser may introduce regressions if a userAccountId is supplied. Confirm the intended behavior or reinstate the check if external callers rely on providing an existing userAccountId.

    // Removed conditional assignment of userAccountId

[hashdotai]

This PR moves the system account creation logic from the API layer to the Graph, which is a good architectural change. The implementation looks generally sound, and the approach of first searching for an existing system account before creating a new one is sensible.

I do have a few concerns about clarity and maintainability:

1. There's commented-out code that should be removed
2. Some of the logic, particularly around using Uuid::nil() and the fallback detection of a system account could benefit from additional comments
3. There are opportunities to improve type safety in the API response

However, these are minor issues that can be addressed with small changes, and the overall approach of moving this functionality to the Graph layer is sound. The implementation correctly abstracts the system account creation logic away from the API, making it reusable across different parts of the application.

[hashdotai]

Instead of using Uuid::nil() as the actor ID for creating the system account, consider using a more explicit approach or providing a comment to explain why Uuid::nil() is used. This helps with future maintainability and understanding the code's intent.

[hashdotai]

Suggested change

	.map(|row| {
	.then(|row| {

Using map here might lead to confusion as you're transforming the row rather than mapping over a collection. Consider using then which better communicates the intent of this operation in an async context.

[TimDiekmann]

Oh, great, this does not even exist...

[hashdotai]

It would be helpful to add a comment explaining why we need to check for a single machine account as an alternative method for identifying the system account. This adds context to the implementation.

[hashdotai]

Add stronger typing for the return value in the REST API. Even though you're using the OpenAPI schema to document the response type, it would be better to have the actual Rust code reflect this with a more specific type than just Json<MachineId>.

[TimDiekmann]

This is strongly typed: MachineId

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 30.73%. Comparing base (401ece7) to head (7aa5fb4).
Report is 28 commits behind head on main.

Files with missing lines	Patch %	Lines
.../hash-api/src/graph/knowledge/system-types/user.ts	0.00%	3 Missing ⚠️
apps/hash-api/src/graph/system-account.ts	0.00%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6884   +/-   ##
=======================================
  Coverage   30.72%   30.73%           
=======================================
  Files         606      606           
  Lines       23280    23266   -14     
  Branches     2966     2965    -1     
=======================================
- Hits         7153     7151    -2     
+ Misses      16070    16058   -12     
  Partials       57       57           

Flag	Coverage Δ
apps.hash-ai-worker-ts	1.32% <ø> (+<0.01%)	⬆️
apps.hash-api	0.00% <0.00%> (ø)
local.hash-backend-utils	3.94% <ø> (ø)
local.hash-graph-sdk	0.00% <ø> (ø)
local.hash-isomorphic-utils	0.00% <ø> (ø)
local.hash-subgraph	21.19% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[hashdotai]

This PR successfully moves the system account creation logic to the Graph component, making it accessible through a dedicated API endpoint. This is a good architectural improvement that will support bootstrapping permissions as mentioned in the PR description.

I appreciate the clean implementation of the get_or_create_system_account functionality and the addition of the REST endpoint. The code is generally well-structured and follows the existing patterns in the codebase.

A few observations:

1. The removal of the RoutedResource trait seems to be part of this refactoring, which simplifies the architecture by moving to direct implementations.
2. The system account discovery logic has two approaches (looking for the first ontology type creator or checking if there's only one machine account), which is a reasonable fallback strategy.
3. The OpenAPI documentation for the new endpoint is well-defined.

My comments are primarily focused on improving documentation for future maintainability, particularly around some of the design decisions made in the implementation. These aren't blocking issues, but addressing them would improve the code quality.

[hashdotai]

Consider adding a more detailed comment here explaining why checking for a single machine account is a valid fallback mechanism for identifying the system account. This would help future developers understand this logic and the assumptions behind it.

[hashdotai]

The use of Uuid::nil() as the actor ID for creating the system account should be explained with a comment. Is this because we need a placeholder actor but don't have a valid one yet? Adding context will help future maintainers understand this design decision.

[hashdotai]

Consider defining a typed response struct for this endpoint rather than using Json<MachineId> directly. This would make the API contract more explicit and provide better documentation through the type system.

[hashdotai]

Since you're removing the RoutedResource trait, ensure you've updated all documentation references to it elsewhere in the codebase. The pattern change from trait-based to impl-based routing should be consistently applied.

[github-actions]

Benchmark results

@rust/hash-graph-benches – Integrations

representative_read_entity

Function	Value	Mean	Flame graphs
entity_by_id	entity type ID: https://blockprotocol.org/@alice/types/entity-type/block/v/1	$$17.3 \mathrm{ms} \pm 199 \mathrm{μs}\left({\color{red}6.72 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	entity type ID: https://blockprotocol.org/@alice/types/entity-type/book/v/1	$$17.2 \mathrm{ms} \pm 180 \mathrm{μs}\left({\color{gray}-1.167 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	entity type ID: https://blockprotocol.org/@alice/types/entity-type/page/v/2	$$16.6 \mathrm{ms} \pm 197 \mathrm{μs}\left({\color{lightgreen}-30.231 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	entity type ID: https://blockprotocol.org/@alice/types/entity-type/organization/v/1	$$17.1 \mathrm{ms} \pm 174 \mathrm{μs}\left({\color{gray}-0.136 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	entity type ID: https://blockprotocol.org/@alice/types/entity-type/person/v/1	$$16.6 \mathrm{ms} \pm 224 \mathrm{μs}\left({\color{lightgreen}-25.989 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	entity type ID: https://blockprotocol.org/@alice/types/entity-type/playlist/v/1	$$15.9 \mathrm{ms} \pm 192 \mathrm{μs}\left({\color{lightgreen}-33.314 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	entity type ID: https://blockprotocol.org/@alice/types/entity-type/song/v/1	$$16.1 \mathrm{ms} \pm 181 \mathrm{μs}\left({\color{gray}-4.269 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	entity type ID: https://blockprotocol.org/@alice/types/entity-type/uk-address/v/1	$$16.9 \mathrm{ms} \pm 268 \mathrm{μs}\left({\color{gray}2.56 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	entity type ID: https://blockprotocol.org/@alice/types/entity-type/building/v/1	$$17.4 \mathrm{ms} \pm 185 \mathrm{μs}\left({\color{red}6.71 \mathrm{\%}}\right) $$	Flame Graph

scaling_read_entity_complete_zero_depth

Function	Value	Mean	Flame graphs
entity_by_id	10 entities	$$2.17 \mathrm{ms} \pm 16.7 \mathrm{μs}\left({\color{gray}-3.637 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	5 entities	$$2.04 \mathrm{ms} \pm 12.5 \mathrm{μs}\left({\color{gray}-0.249 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	50 entities	$$4.35 \mathrm{ms} \pm 21.3 \mathrm{μs}\left({\color{gray}4.66 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	25 entities	$$3.45 \mathrm{ms} \pm 19.3 \mathrm{μs}\left({\color{red}22.4 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	1 entities	$$2.02 \mathrm{ms} \pm 6.51 \mathrm{μs}\left({\color{gray}-0.111 \mathrm{\%}}\right) $$	Flame Graph

representative_read_entity_type

Function	Value	Mean	Flame graphs
get_entity_type_by_id	Account ID: d4e16033-c281-4cde-aa35-9085bf2e7579	$$2.17 \mathrm{ms} \pm 7.76 \mathrm{μs}\left({\color{gray}-0.087 \mathrm{\%}}\right) $$	Flame Graph

scaling_read_entity_linkless

Function	Value	Mean	Flame graphs
entity_by_id	10000 entities	$$13.9 \mathrm{ms} \pm 63.4 \mathrm{μs}\left({\color{red}42.5 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	100 entities	$$2.26 \mathrm{ms} \pm 9.27 \mathrm{μs}\left({\color{gray}-0.313 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	10 entities	$$2.05 \mathrm{ms} \pm 6.72 \mathrm{μs}\left({\color{gray}-0.199 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	1000 entities	$$3.04 \mathrm{ms} \pm 19.8 \mathrm{μs}\left({\color{lightgreen}-5.432 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	1 entities	$$2.03 \mathrm{ms} \pm 6.99 \mathrm{μs}\left({\color{gray}-0.248 \mathrm{\%}}\right) $$	Flame Graph

representative_read_multiple_entities

Function	Value	Mean	Flame graphs
entity_by_property	depths: DT=0, PT=2, ET=2, E=2	$$54.8 \mathrm{ms} \pm 368 \mathrm{μs}\left({\color{gray}0.041 \mathrm{\%}}\right) $$	Flame Graph
entity_by_property	depths: DT=0, PT=0, ET=0, E=0	$$39.7 \mathrm{ms} \pm 209 \mathrm{μs}\left({\color{gray}0.760 \mathrm{\%}}\right) $$	Flame Graph
entity_by_property	depths: DT=0, PT=0, ET=0, E=2	$$44.6 \mathrm{ms} \pm 433 \mathrm{μs}\left({\color{gray}0.169 \mathrm{\%}}\right) $$	Flame Graph
entity_by_property	depths: DT=2, PT=2, ET=2, E=2	$$58.4 \mathrm{ms} \pm 372 \mathrm{μs}\left({\color{gray}1.16 \mathrm{\%}}\right) $$	Flame Graph
entity_by_property	depths: DT=255, PT=255, ET=255, E=255	$$67.0 \mathrm{ms} \pm 353 \mathrm{μs}\left({\color{gray}-1.734 \mathrm{\%}}\right) $$	Flame Graph
entity_by_property	depths: DT=0, PT=0, ET=2, E=2	$$49.9 \mathrm{ms} \pm 374 \mathrm{μs}\left({\color{gray}-1.073 \mathrm{\%}}\right) $$	Flame Graph
link_by_source_by_property	depths: DT=0, PT=2, ET=2, E=2	$$89.5 \mathrm{ms} \pm 704 \mathrm{μs}\left({\color{gray}-1.579 \mathrm{\%}}\right) $$	Flame Graph
link_by_source_by_property	depths: DT=0, PT=0, ET=0, E=0	$$40.2 \mathrm{ms} \pm 455 \mathrm{μs}\left({\color{gray}-1.010 \mathrm{\%}}\right) $$	Flame Graph
link_by_source_by_property	depths: DT=0, PT=0, ET=0, E=2	$$76.3 \mathrm{ms} \pm 844 \mathrm{μs}\left({\color{gray}-2.068 \mathrm{\%}}\right) $$	Flame Graph
link_by_source_by_property	depths: DT=2, PT=2, ET=2, E=2	$$93.1 \mathrm{ms} \pm 686 \mathrm{μs}\left({\color{gray}-2.407 \mathrm{\%}}\right) $$	Flame Graph
link_by_source_by_property	depths: DT=255, PT=255, ET=255, E=255	$$102 \mathrm{ms} \pm 725 \mathrm{μs}\left({\color{gray}-0.595 \mathrm{\%}}\right) $$	Flame Graph
link_by_source_by_property	depths: DT=0, PT=0, ET=2, E=2	$$85.2 \mathrm{ms} \pm 594 \mathrm{μs}\left({\color{gray}0.427 \mathrm{\%}}\right) $$	Flame Graph

scaling_read_entity_complete_one_depth

Function	Value	Mean	Flame graphs
entity_by_id	10 entities	$$58.1 \mathrm{ms} \pm 121 \mathrm{μs}\left({\color{red}76.8 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	5 entities	$$27.3 \mathrm{ms} \pm 145 \mathrm{μs}\left({\color{gray}0.161 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	50 entities	$$5.36 \mathrm{s} \pm 535 \mathrm{ms}\left({\color{gray}-0.359 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	25 entities	$$85.5 \mathrm{ms} \pm 358 \mathrm{μs}\left({\color{lightgreen}-52.657 \mathrm{\%}}\right) $$	Flame Graph
entity_by_id	1 entities	$$21.4 \mathrm{ms} \pm 114 \mathrm{μs}\left({\color{gray}2.06 \mathrm{\%}}\right) $$	Flame Graph