fix(openapi): fix the problem of keeping the server running after make openapi and make api.json deterministically ordered#1438
Open
Artifizer wants to merge 7 commits intocyberfabric:mainfrom
Open
Conversation
…ke openapi` The problem was that when one run `make openapi` or `make all`, the server ramained running after preventing repetitive make targets execution What was changed: - Extract server startup and cleanup into reusable start_server_and_wait macro with cross-platform support (Linux, macOS, Windows) - Simplify openapi recipe by using the new macro Signed-off-by: Artifizer <artifizer@gmail.com>
Refactor `make openapi` target. Rationale: avoids non-deterministic schema ordering and noisy git diffs; improves build reproducibility and maintainability. 1. Use BTreeMap for OpenAPI components registry to produce stable, alphabetically ordered schema output (replace HashMap -> BTreeMap). 2. Add `scripts/sort_openapi_json.py` and invoke it from Makefile to post-process the fetched OpenAPI JSON, ensuring deterministic key ordering and stable diffs. 3. Regenerated/sorted `docs/api/api.json` Signed-off-by: Artifizer <artifizer@gmail.com>
Contributor
📝 WalkthroughWalkthroughThese changes establish deterministic ordering of OpenAPI specifications across the build pipeline: the Rust registry replaces HashMap with BTreeMap for ordered component schema iteration, a new Python script recursively sorts all JSON keys, and the Makefile refactors server startup/cleanup into a reusable macro while integrating the sorting step. Additionally, test files are updated with postponed type annotation evaluation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Artifizer <artifizer@gmail.com>
…'make all' on macOS Signed-off-by: Artifizer <artifizer@gmail.com>
…ke all' Signed-off-by: Artifizer <artifizer@gmail.com>
Made-with: Cursor
MikeFalcon77
reviewed
Apr 4, 2026
Signed-off-by: Artifizer <artifizer@gmail.com>
Review Summary by QodoFix server cleanup in openapi target and ensure deterministic API spec ordering
WalkthroughsDescription• Fix server process cleanup issue in make openapi target • Ensure deterministic OpenAPI JSON ordering via BTreeMap and sorting script • Add Python 3.9 compatibility to mini-chat tests with __future__ imports • Improve user experience with congratulations message in make all Diagramflowchart LR
A["Makefile openapi target"] -->|"Extract macro"| B["start_server_and_wait macro"]
B -->|"Cross-platform support"| C["Linux/macOS/Windows cleanup"]
D["HashMap components registry"] -->|"Replace with"| E["BTreeMap"]
E -->|"Deterministic ordering"| F["sort_openapi_json.py script"]
F -->|"Post-process"| G["Sorted api.json"]
H["Mini-chat test files"] -->|"Add import"| I["from __future__ import annotations"]
I -->|"Python 3.9 compatible"| J["Tests pass on macOS"]
File Changes1. libs/modkit/src/api/openapi_registry.rs
|
Code Review by Qodo
1. Stray @ breaks openapi
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Makefile
Outdated
Comment on lines
+328
to
+329
| @$(call start_server_and_wait,cargo run --bin hyperspot-server $(E2E_ARGS) -- --config config/quickstart.yaml,$(OPENAPI_URL),300); \ | ||
| @$(call start_server_and_wait,cargo run --bin hyperspot-server $(E2E_ARGS) -- --config config/quickstart.yaml,$(OPENAPI_URL),300); \ |
There was a problem hiding this comment.
1. Stray @ breaks openapi 🐞 Bug ≡ Correctness
In openapi, the second backslash-continued line starts with @$(call ...), so the @ is passed to the shell (not treated as a Makefile recipe prefix) and will likely fail as an invalid command. This can break make openapi (and therefore make all).
Agent Prompt
### Issue description
The `openapi` target has a backslash-continued command sequence where the second continued line begins with `@$(call ...)`. Because it is not the first physical recipe line, `@` is not processed by `make` and is passed to the shell, breaking the command.
### Issue Context
This currently makes `make openapi` unreliable/fail and `make all` depends on `openapi`.
### Fix Focus Areas
- Makefile[324-335]
### Suggested fix
- Remove the second `@` (or remove the entire duplicated `start_server_and_wait` line—see related issue) so continuation lines start with valid shell syntax.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+328
to
+334
| @$(call start_server_and_wait,cargo run --bin hyperspot-server $(E2E_ARGS) -- --config config/quickstart.yaml,$(OPENAPI_URL),300); \ | ||
| @$(call start_server_and_wait,cargo run --bin hyperspot-server $(E2E_ARGS) -- --config config/quickstart.yaml,$(OPENAPI_URL),300); \ | ||
| echo "Fetching OpenAPI spec..."; \ | ||
| mkdir -p $$(dirname "$(OPENAPI_OUT)"); \ | ||
| curl -fsS "$(OPENAPI_URL)" -o "$(OPENAPI_OUT)"; \ | ||
| echo "OpenAPI spec saved to $(OPENAPI_OUT)"; \ | ||
| echo "Stopping hyperspot-server..."; \ | ||
| kill $$SERVER_PID >/dev/null 2>&1 || true; \ | ||
| wait $$SERVER_PID 2>/dev/null || true | ||
| echo "Sorting OpenAPI JSON for deterministic ordering..."; \ | ||
| python3 scripts/sort_openapi_json.py "$(OPENAPI_OUT)"; \ |
There was a problem hiding this comment.
2. Server started twice 🐞 Bug ☼ Reliability
The openapi target calls start_server_and_wait twice for the same command/URL, which can mask a failed second start (e.g., port already in use) because readiness is checked only via curl success. This can leave an unexpected server process running and produce false-positive success.
Agent Prompt
### Issue description
`make openapi` starts the server twice, and the readiness check can succeed even when the second process died (e.g., bind failure) because it only checks `curl` success, not that the newly started PID is still alive.
### Issue Context
This can lead to false success and/or leave a stray server process running, defeating the PR's goal of cleaning up after OpenAPI generation.
### Fix Focus Areas
- Makefile[324-335]
- Makefile[38-102]
### Suggested fix
1) Remove the duplicated second `start_server_and_wait` invocation in the `openapi` recipe.
2) In `start_server_and_wait`, add a post-loop check right after `curl` succeeds:
- If `! is_process_running $SERVER_PID`, print an error referencing the log file and exit non-zero.
This ensures success implies the process you just started is actually alive.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 328-334: The recipe that fetches and sorts the OpenAPI spec should
fail fast so curl/python errors don't get masked; update the Makefile recipe
that runs start_server_and_wait and the subsequent commands (the block invoking
curl -fsS "$(OPENAPI_URL)" -o "$(OPENAPI_OUT)" and python3
scripts/sort_openapi_json.py "$(OPENAPI_OUT)") to enable errexit (e.g., add set
-e or set -euo pipefail at the start of the shell invocation) or chain commands
with && so any failure aborts the target, ensuring failures in curl or the sort
script surface immediately.
In `@scripts/sort_openapi_json.py`:
- Around line 44-57: Wrap file read and write operations around explicit OSError
handling so filesystem errors don't produce raw tracebacks: when opening/reading
the file_path before json.load and when opening/writing the file_path for
json.dump, catch OSError (or IOError) and print a clear error to stderr
(including the exception message), then sys.exit(1); keep existing
JSONDecodeError handling and continue using sort_dict_recursive and json.dump as
before but ensure both read and write I/O are guarded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6097bba4-cc28-4406-95d6-a50a101fa563
📒 Files selected for processing (12)
Makefiledocs/api/api.jsonlibs/modkit/src/api/openapi_registry.rsscripts/sort_openapi_json.pytesting/e2e/modules/mini_chat/test_cleanup.pytesting/e2e/modules/mini_chat/test_code_interpreter_usage.pytesting/e2e/modules/mini_chat/test_full_scenario.pytesting/e2e/modules/mini_chat/test_principles.pytesting/e2e/modules/mini_chat/test_quota_enforcement.pytesting/e2e/modules/mini_chat/test_quota_status.pytesting/e2e/modules/mini_chat/test_web_search_usage.pytesting/e2e/modules/oagw/mock_upstream.py
Comment on lines
+328
to
+334
| @$(call start_server_and_wait,cargo run --bin hyperspot-server $(E2E_ARGS) -- --config config/quickstart.yaml,$(OPENAPI_URL),300); \ | ||
| echo "Fetching OpenAPI spec..."; \ | ||
| mkdir -p $$(dirname "$(OPENAPI_OUT)"); \ | ||
| curl -fsS "$(OPENAPI_URL)" -o "$(OPENAPI_OUT)"; \ | ||
| echo "OpenAPI spec saved to $(OPENAPI_OUT)"; \ | ||
| echo "Stopping hyperspot-server..."; \ | ||
| kill $$SERVER_PID >/dev/null 2>&1 || true; \ | ||
| wait $$SERVER_PID 2>/dev/null || true | ||
| echo "Sorting OpenAPI JSON for deterministic ordering..."; \ | ||
| python3 scripts/sort_openapi_json.py "$(OPENAPI_OUT)"; \ | ||
| echo "OpenAPI spec saved to $(OPENAPI_OUT)" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether openapi recipe uses semicolon chaining without fail-fast.
rg -n -A8 -B2 '^openapi:' Makefile
rg -n 'set -e' MakefileRepository: cyberfabric/cyberfabric-core
Length of output: 718
🏁 Script executed:
# Verify the complete openapi target to confirm all lines and context
rg -n -A10 '^openapi:' MakefileRepository: cyberfabric/cyberfabric-core
Length of output: 709
Add set -e to fail fast on fetch/sort errors.
This command chain uses semicolons without set -e, so a failed curl or python3 command will still allow subsequent echo statements to execute and mask the failure with a successful exit code. Since openapi is part of the default build, this creates a risk of silently corrupted builds.
Proposed fix
- @$(call start_server_and_wait,cargo run --bin hyperspot-server $(E2E_ARGS) -- --config config/quickstart.yaml,$(OPENAPI_URL),300); \
+ `@set` -e; \
+ $(call start_server_and_wait,cargo run --bin hyperspot-server $(E2E_ARGS) -- --config config/quickstart.yaml,$(OPENAPI_URL),300); \
echo "Fetching OpenAPI spec..."; \
mkdir -p $$(dirname "$(OPENAPI_OUT)"); \
curl -fsS "$(OPENAPI_URL)" -o "$(OPENAPI_OUT)"; \
echo "Sorting OpenAPI JSON for deterministic ordering..."; \
python3 scripts/sort_openapi_json.py "$(OPENAPI_OUT)"; \
echo "OpenAPI spec saved to $(OPENAPI_OUT)"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @$(call start_server_and_wait,cargo run --bin hyperspot-server $(E2E_ARGS) -- --config config/quickstart.yaml,$(OPENAPI_URL),300); \ | |
| echo "Fetching OpenAPI spec..."; \ | |
| mkdir -p $$(dirname "$(OPENAPI_OUT)"); \ | |
| curl -fsS "$(OPENAPI_URL)" -o "$(OPENAPI_OUT)"; \ | |
| echo "OpenAPI spec saved to $(OPENAPI_OUT)"; \ | |
| echo "Stopping hyperspot-server..."; \ | |
| kill $$SERVER_PID >/dev/null 2>&1 || true; \ | |
| wait $$SERVER_PID 2>/dev/null || true | |
| echo "Sorting OpenAPI JSON for deterministic ordering..."; \ | |
| python3 scripts/sort_openapi_json.py "$(OPENAPI_OUT)"; \ | |
| echo "OpenAPI spec saved to $(OPENAPI_OUT)" | |
| `@set` -e; \ | |
| $(call start_server_and_wait,cargo run --bin hyperspot-server $(E2E_ARGS) -- --config config/quickstart.yaml,$(OPENAPI_URL),300); \ | |
| echo "Fetching OpenAPI spec..."; \ | |
| mkdir -p $$(dirname "$(OPENAPI_OUT)"); \ | |
| curl -fsS "$(OPENAPI_URL)" -o "$(OPENAPI_OUT)"; \ | |
| echo "Sorting OpenAPI JSON for deterministic ordering..."; \ | |
| python3 scripts/sort_openapi_json.py "$(OPENAPI_OUT)"; \ | |
| echo "OpenAPI spec saved to $(OPENAPI_OUT)" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 328 - 334, The recipe that fetches and sorts the
OpenAPI spec should fail fast so curl/python errors don't get masked; update the
Makefile recipe that runs start_server_and_wait and the subsequent commands (the
block invoking curl -fsS "$(OPENAPI_URL)" -o "$(OPENAPI_OUT)" and python3
scripts/sort_openapi_json.py "$(OPENAPI_OUT)") to enable errexit (e.g., add set
-e or set -euo pipefail at the start of the shell invocation) or chain commands
with && so any failure aborts the target, ensuring failures in curl or the sort
script surface immediately.
Comment on lines
+44
to
+57
| try: | ||
| with open(file_path, 'r', encoding='utf-8') as f: | ||
| data = json.load(f) | ||
| except json.JSONDecodeError as e: | ||
| print(f"Error: Failed to parse JSON: {e}", file=sys.stderr) | ||
| sys.exit(1) | ||
|
|
||
| # Sort all keys recursively | ||
| sorted_data = sort_dict_recursive(data) | ||
|
|
||
| # Write back with consistent formatting | ||
| with open(file_path, 'w', encoding='utf-8') as f: | ||
| json.dump(sorted_data, f, indent=2, ensure_ascii=False) | ||
| f.write('\n') # Add trailing newline |
Contributor
There was a problem hiding this comment.
Handle file I/O errors explicitly for stable CLI behavior.
The script currently handles JSON parsing errors but not read/write OSError cases, so users can still get raw tracebacks for common failures (e.g., directory path, permission denied).
💡 Proposed fix
- try:
- with open(file_path, 'r', encoding='utf-8') as f:
- data = json.load(f)
- except json.JSONDecodeError as e:
- print(f"Error: Failed to parse JSON: {e}", file=sys.stderr)
- sys.exit(1)
+ try:
+ with open(file_path, 'r', encoding='utf-8') as f:
+ data = json.load(f)
+ except json.JSONDecodeError as e:
+ print(f"Error: Failed to parse JSON: {e}", file=sys.stderr)
+ sys.exit(1)
+ except OSError as e:
+ print(f"Error: Failed to read '{file_path}': {e}", file=sys.stderr)
+ sys.exit(1)
@@
- with open(file_path, 'w', encoding='utf-8') as f:
- json.dump(sorted_data, f, indent=2, ensure_ascii=False)
- f.write('\n') # Add trailing newline
+ try:
+ with open(file_path, 'w', encoding='utf-8') as f:
+ json.dump(sorted_data, f, indent=2, ensure_ascii=False)
+ f.write('\n') # Add trailing newline
+ except OSError as e:
+ print(f"Error: Failed to write '{file_path}': {e}", file=sys.stderr)
+ sys.exit(1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try: | |
| with open(file_path, 'r', encoding='utf-8') as f: | |
| data = json.load(f) | |
| except json.JSONDecodeError as e: | |
| print(f"Error: Failed to parse JSON: {e}", file=sys.stderr) | |
| sys.exit(1) | |
| # Sort all keys recursively | |
| sorted_data = sort_dict_recursive(data) | |
| # Write back with consistent formatting | |
| with open(file_path, 'w', encoding='utf-8') as f: | |
| json.dump(sorted_data, f, indent=2, ensure_ascii=False) | |
| f.write('\n') # Add trailing newline | |
| try: | |
| with open(file_path, 'r', encoding='utf-8') as f: | |
| data = json.load(f) | |
| except json.JSONDecodeError as e: | |
| print(f"Error: Failed to parse JSON: {e}", file=sys.stderr) | |
| sys.exit(1) | |
| except OSError as e: | |
| print(f"Error: Failed to read '{file_path}': {e}", file=sys.stderr) | |
| sys.exit(1) | |
| # Sort all keys recursively | |
| sorted_data = sort_dict_recursive(data) | |
| # Write back with consistent formatting | |
| try: | |
| with open(file_path, 'w', encoding='utf-8') as f: | |
| json.dump(sorted_data, f, indent=2, ensure_ascii=False) | |
| f.write('\n') # Add trailing newline | |
| except OSError as e: | |
| print(f"Error: Failed to write '{file_path}': {e}", file=sys.stderr) | |
| sys.exit(1) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/sort_openapi_json.py` around lines 44 - 57, Wrap file read and write
operations around explicit OSError handling so filesystem errors don't produce
raw tracebacks: when opening/reading the file_path before json.load and when
opening/writing the file_path for json.dump, catch OSError (or IOError) and
print a clear error to stderr (including the exception message), then
sys.exit(1); keep existing JSONDecodeError handling and continue using
sort_dict_recursive and json.dump as before but ensure both read and write I/O
are guarded.
Collaborator
|
windows test will be fixed at the my PR (real problem of two simulteneous cases for single select! - biased will be used) |
MikeFalcon77
approved these changes
Apr 4, 2026
Collaborator
But it will be better to do it in this PR. @Artifizer could you please do the following: pub fn poker(
interval: Duration,
cancel: CancellationToken,
) -> (Arc<Notify>, tokio::task::JoinHandle<()>) {
let notify = Arc::new(Notify::new());
let n = notify.clone();
let handle = tokio::spawn(async move {
loop {
tokio::select! {
+ biased;
() = cancel.cancelled() => break,
() = tokio::time::sleep(interval) => { n.notify_one(); }
}
}
});
(notify, handle)
} |
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
2 participants
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.
fix(openapi): fix the problem of keeping the server running after
make openapibuild(openapi): make generated api.json deterministically ordered
fix(tests): allow mini-chat tests to be running on Python 3.9 to fix 'make all' on macOS
chore: Makefile - add clear congratulations message in the end of 'make all'
The PR has
breaking-api-acknowledgedbecause it now has properly regenerated API spec fileSummary by CodeRabbit