feat: generated OpenAPI, /docs page, FAQ, and Swagger UI#968
Merged
Conversation
Contributor
Summary by CodeRabbit
WalkthroughThis PR adds backend OpenAPI generation and guardrails, documents public REST endpoints in a committed spec, wires validation into lint and CI, and adds a frontend Docs page with embedded API reference, FAQ rendering, and navigation updates. ChangesOpenAPI Documentation and Docs Page
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
application/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. application/frontend/src/const.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. application/frontend/src/pages/Docs/Docs.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
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 |
![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: 5
🧹 Nitpick comments (4)
application/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsx (2)
61-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winError fallback link is hardcoded to
docs/faq.md.The component accepts a generic
srcprop, but the error-state GitHub link is hardcoded todocs/faq.md. If this component is reused for other markdown sources, the error link will be incorrect. Consider accepting an optionalerrorHrefprop or deriving the link fromsrc.♻️ Proposed refactor to make error link configurable
interface MarkdownFromRepoProps { src: string; className?: string; + errorHref?: string; } -export const MarkdownFromRepo = ({ src, className = '' }: MarkdownFromRepoProps) => { +export const MarkdownFromRepo = ({ src, className = '', errorHref }: MarkdownFromRepoProps) => {And in the error JSX:
- <a href="https://github.com/OWASP/OpenCRE/blob/main/docs/faq.md" target="_blank" rel="noreferrer"> + <a href={errorHref ?? `https://github.com/OWASP/OpenCRE/blob/main/${src.replace(/^\//, '')}`} target="_blank" rel="noreferrer">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsx` around lines 61 - 65, The error-state GitHub link in MarkdownFromRepo is hardcoded to docs/faq.md instead of using the component’s src input. Update MarkdownFromRepo.tsx so the fallback link is configurable, either by adding an optional errorHref prop or deriving the GitHub URL from src, and use that value in the error JSX where the source link is rendered.
22-50: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse
AbortControllerto cancel in-flight fetches on unmount and add a timeout.The
cancelledflag prevents state updates after unmount, but the network request continues consuming resources. AnAbortControllerwould cancel the fetch itself and allow adding a timeout so the component doesn't hang indefinitely on a slow server.♻️ Proposed refactor with AbortController and timeout
useEffect(() => { - let cancelled = false; + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 10_000); + setLoading(true); setError(null); - fetch(src) + fetch(src, { signal: controller.signal }) .then((res) => { if (!res.ok) { throw new Error(`Failed to load markdown (${res.status})`); } return res.text(); }) .then((markdown) => { - if (cancelled) return; const rendered = DOMPurify.sanitize(marked.parse(markdown), { USE_PROFILES: { html: true }, }); setHtml(rendered); }) .catch((err: Error) => { - if (!cancelled) { + if (err.name !== 'AbortError') { setError(err.message); } }) .finally(() => { - if (!cancelled) { + clearTimeout(timeout); + // Only update if not aborted + if (!controller.signal.aborted) { setLoading(false); } }); return () => { - cancelled = true; + controller.abort(); + clearTimeout(timeout); }; }, [src]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsx` around lines 22 - 50, The MarkdownFromRepo effect currently uses a cancelled flag in the fetch/src workflow, but the request itself is never aborted. Update the fetch logic in MarkdownFromRepo.tsx to use an AbortController so cleanup in the effect aborts the in-flight request, and add a timeout-based abort path to prevent hanging on slow servers. Make sure the existing error/loading handling around setError, setHtml, and setLoading continues to ignore abort-related cancellations while preserving normal failure behavior.application/web/web_main.py (1)
662-672: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFAQ endpoint path resolution is fragile but functional.
The
../../docsrelative path fromapplication/web/works for the current layout. Ifweb_main.pyis ever moved, this will silently break. Consider using a project-root constant orapp.root_pathfor robustness. Not blocking.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/web/web_main.py` around lines 662 - 672, The faq_markdown route currently resolves the docs folder with a fragile relative path, so update the path construction in faq_markdown to use a stable base such as app.root_path or an existing project-root constant instead of os.path.dirname(os.path.realpath(__file__)). Keep the route behavior the same, but make the send_from_directory directory argument resilient to future file moves by locating the docs directory from a project-level anchor.application/tests/openapi_guardrail_test.py (1)
24-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove
generate_openapi.main()call to avoid masking stale specs.The test regenerates the committed spec before running the guardrail, which means a stale spec will always pass. The test should verify the committed spec is already fresh. Additionally,
generate_openapi.main()writes todocs/api/openapi.yamlon disk, leaving the repo dirty if output differs.♻️ Proposed refactor
def test_guardrail_script_passes(self) -> None: import scripts.check_openapi_guardrail as guardrail - # Ensure committed spec matches generator output before guardrail run. - import scripts.generate_openapi as generate_openapi - - generate_openapi.main() + # The committed spec must already be fresh. + # Run `make openapi-generate` if this test fails. self.assertEqual(guardrail.main(), 0)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/tests/openapi_guardrail_test.py` around lines 24 - 31, The guardrail test currently calls generate_openapi.main() inside test_guardrail_script_passes, which masks stale committed specs and can dirty docs/api/openapi.yaml during test execution. Remove that regeneration step from the test and have test_guardrail_script_passes only invoke scripts.check_openapi_guardrail.main() after importing scripts.check_openapi_guardrail, so the test verifies the committed OpenAPI spec is already up to date.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@application/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsx`:
- Around line 31-33: The MarkdownFromRepo render path is passing the potentially
async return type from marked.parse() directly into DOMPurify.sanitize(), which
can break typing. Update the MarkdownFromRepo component’s render logic so
marked.parse(markdown) is explicitly synchronous (for example by using the
async=false option or by narrowing the result to string before sanitize), then
keep the sanitized output flowing through DOMPurify.sanitize() as before.
In `@application/web/openapi_registry.py`:
- Around line 373-412: The _operation_from_path helper is adding query-schema
parameters on top of the path parameters without de-duplicating names, which
causes the same field to appear as both in: path and in: query. Update the
parameter assembly in _operation_from_path to track the names from
_path_parameters(path_spec.path) and filter
_marshmallow_query_parameters(path_spec.query_schema) so any query field already
used by the path is skipped before assigning operation["parameters"].
In `@docs/api/openapi.yaml`:
- Around line 201-230: The `standard` query parameter in `map_analysis` is
documented as exactly two names, but its array schema in `openapi.yaml` does not
enforce that contract. Update the `standard` parameter schema to add both
`minItems: 2` and `maxItems: 2` so the OpenAPI definition matches the intended
behavior and client validation can catch invalid request sizes.
- Around line 935-947: The NodeListResponseSchema definition leaves page untyped
with page: {}, which allows any value instead of a page number. Update the
NodeListResponseSchema properties so page is explicitly declared as type:
integer, matching total_pages and the intended numeric pagination shape.
In `@scripts/check_openapi_guardrail.py`:
- Around line 84-91: The guardrail flow in check_openapi_guardrail.py still
opens COMMITTED_SPEC after _check_freshness can report a missing file, causing
an unhandled FileNotFoundError. Update the main checks sequence so the committed
spec is only opened after confirming it exists, or gate the yaml.safe_load/open
call behind a file-existence check and turn the missing-spec case into a normal
failed check. Keep the behavior in _check_freshness, _check_validity, and
_check_route_coverage consistent so all failures are collected and reported
without crashing.
---
Nitpick comments:
In `@application/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsx`:
- Around line 61-65: The error-state GitHub link in MarkdownFromRepo is
hardcoded to docs/faq.md instead of using the component’s src input. Update
MarkdownFromRepo.tsx so the fallback link is configurable, either by adding an
optional errorHref prop or deriving the GitHub URL from src, and use that value
in the error JSX where the source link is rendered.
- Around line 22-50: The MarkdownFromRepo effect currently uses a cancelled flag
in the fetch/src workflow, but the request itself is never aborted. Update the
fetch logic in MarkdownFromRepo.tsx to use an AbortController so cleanup in the
effect aborts the in-flight request, and add a timeout-based abort path to
prevent hanging on slow servers. Make sure the existing error/loading handling
around setError, setHtml, and setLoading continues to ignore abort-related
cancellations while preserving normal failure behavior.
In `@application/tests/openapi_guardrail_test.py`:
- Around line 24-31: The guardrail test currently calls generate_openapi.main()
inside test_guardrail_script_passes, which masks stale committed specs and can
dirty docs/api/openapi.yaml during test execution. Remove that regeneration step
from the test and have test_guardrail_script_passes only invoke
scripts.check_openapi_guardrail.main() after importing
scripts.check_openapi_guardrail, so the test verifies the committed OpenAPI spec
is already up to date.
In `@application/web/web_main.py`:
- Around line 662-672: The faq_markdown route currently resolves the docs folder
with a fragile relative path, so update the path construction in faq_markdown to
use a stable base such as app.root_path or an existing project-root constant
instead of os.path.dirname(os.path.realpath(__file__)). Keep the route behavior
the same, but make the send_from_directory directory argument resilient to
future file moves by locating the docs directory from a project-level anchor.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 9377036f-a66a-4acb-955a-70f4cf68bc23
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (25)
.github/workflows/linter.yml.gitignoreMakefileREADME.mdapplication/__init__.pyapplication/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsxapplication/frontend/src/components/MarkdownFromRepo/markdownFromRepo.scssapplication/frontend/src/const.tsapplication/frontend/src/pages/Docs/Docs.tsxapplication/frontend/src/pages/Docs/docs.scssapplication/frontend/src/pages/Search/Search.tsxapplication/frontend/src/routes.tsxapplication/frontend/src/scaffolding/Header/Header.tsxapplication/frontend/www/bundle.jsapplication/frontend/www/bundle.js.LICENSE.txtapplication/tests/openapi_guardrail_test.pyapplication/web/openapi_registry.pyapplication/web/openapi_schemas.pyapplication/web/web_main.pydocs/api/openapi.yamldocs/faq.mdpackage.jsonrequirements.txtscripts/check_openapi_guardrail.pyscripts/generate_openapi.py
northdpole
force-pushed
the
feat/docs-openapi-page
branch
2 times, most recently
from
77b1e3a to
c9ee70d
Compare
Pa04rth
approved these changes
Jul 9, 2026
Add Marshmallow schemas and a registry-driven OpenAPI generator for all public GET /rest/v1 routes, with make openapi-generate, make openapi-guardrail, CI enforcement, and @openapi_documented markers on handlers. Co-authored-by: Cursor <cursoragent@cursor.com>
Add docs/faq.md as the single FAQ source of truth, expose it via Flask, track it in git, and link it from the README. Co-authored-by: Cursor <cursoragent@cursor.com>
Introduce the in-app documentation route with interactive API reference, FAQ rendering from /docs/faq.md, header navigation, and homepage footer links. Co-authored-by: Cursor <cursoragent@cursor.com>
Use stable repo-root docs path for FAQ route and enforce exactly-two standards via marshmallow Length validation on map analysis queries. Co-authored-by: Cursor <cursoragent@cursor.com>
northdpole
force-pushed
the
feat/docs-openapi-page
branch
from
6580914 to
ee4ece1
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
application/tests/openapi_guardrail_test.py (1)
13-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
NO_LOGINis set on the process env but never restored.
setUpmutatesos.environ["NO_LOGIN"]and there is notearDownto restore the prior value, so this leaks into subsequent tests running in the same process. Consider snapshotting and restoring, or usingunittest.mock.patch.dict.♻️ Proposed fix
def setUp(self) -> None: - os.environ["NO_LOGIN"] = "1" - self.app = create_app(mode="test") + self._env = unittest.mock.patch.dict(os.environ, {"NO_LOGIN": "1"}) + self._env.start() + self.app = create_app(mode="test") + + def tearDown(self) -> None: + self._env.stop()(add
import unittest.mockaccordingly)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/tests/openapi_guardrail_test.py` around lines 13 - 15, The setUp method in openapi_guardrail_test mutates os.environ["NO_LOGIN"] without restoring it, which can leak state into later tests. Update the test class to preserve the prior environment value and restore it after create_app(mode="test") runs, or use unittest.mock.patch.dict around the NO_LOGIN assignment so the change is scoped to this test setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@application/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsx`:
- Around line 28-65: The MarkdownFromRepo useEffect in MarkdownFromRepo.tsx
leaves the UI stuck in loading state when FETCH_TIMEOUT_MS aborts the request
because the AbortError path skips both setError and setLoading(false). Update
the fetch flow to distinguish a timeout abort from cleanup/unmount by using an
explicit active flag in the effect cleanup, and when the timeout controller
triggers, surface a user-visible timeout error instead of suppressing it. Keep
the existing fetch/marked.parse/DOMPurify logic, but ensure setLoading(false)
runs for timeout failures while still preventing state updates after unmount.
In `@application/web/openapi_registry.py`:
- Around line 166-178: The `map_analysis` OpenAPI entry currently describes
behavior that no longer matches production. Update the `PathSpec` for
`map_analysis` in `openapi_registry.py` so its description reflects cache-only
production behavior: it serves precomputed gap analysis from Postgres and does
not enqueue work on cache miss, which should be documented as returning 404
instead. Keep the `summary` and response schema references intact, but make the
`description` and any related response text consistent with the actual
`map_analysis` endpoint behavior.
- Around line 257-271: The deeplink PathSpec entries still describe a success
response instead of the redirect actually returned at runtime. Update the
openapi_registry deeplink definitions for both resolve-by-name and
resolve-by-type endpoints to include a 302 response with a Location header, so
the generated OpenAPI and docs/api/openapi.yaml match the redirect behavior.
---
Nitpick comments:
In `@application/tests/openapi_guardrail_test.py`:
- Around line 13-15: The setUp method in openapi_guardrail_test mutates
os.environ["NO_LOGIN"] without restoring it, which can leak state into later
tests. Update the test class to preserve the prior environment value and restore
it after create_app(mode="test") runs, or use unittest.mock.patch.dict around
the NO_LOGIN assignment so the change is scoped to this test setup.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 7581f2a3-ff9c-4b38-949d-48cb30b88ef0
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (25)
.github/workflows/linter.yml.gitignoreMakefileREADME.mdapplication/__init__.pyapplication/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsxapplication/frontend/src/components/MarkdownFromRepo/markdownFromRepo.scssapplication/frontend/src/const.tsapplication/frontend/src/pages/Docs/Docs.tsxapplication/frontend/src/pages/Docs/docs.scssapplication/frontend/src/pages/Search/Search.tsxapplication/frontend/src/routes.tsxapplication/frontend/src/scaffolding/Header/Header.tsxapplication/frontend/www/bundle.jsapplication/frontend/www/bundle.js.LICENSE.txtapplication/tests/openapi_guardrail_test.pyapplication/web/openapi_registry.pyapplication/web/openapi_schemas.pyapplication/web/web_main.pydocs/api/openapi.yamldocs/faq.mdpackage.jsonrequirements.txtscripts/check_openapi_guardrail.pyscripts/generate_openapi.py
✅ Files skipped from review due to trivial changes (2)
- docs/faq.md
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (15)
- application/frontend/src/const.ts
- application/frontend/src/pages/Search/Search.tsx
- application/frontend/src/scaffolding/Header/Header.tsx
- README.md
- application/frontend/src/components/MarkdownFromRepo/markdownFromRepo.scss
- application/frontend/src/pages/Docs/docs.scss
- .github/workflows/linter.yml
- application/frontend/src/pages/Docs/Docs.tsx
- application/frontend/src/routes.tsx
- package.json
- application/init.py
- requirements.txt
- Makefile
- application/web/web_main.py
- application/web/openapi_schemas.py
Comment on lines
+28
to
+65
| useEffect(() => { | ||
| const controller = new AbortController(); | ||
| const timeout = window.setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS); | ||
|
|
||
| setLoading(true); | ||
| setError(null); | ||
|
|
||
| fetch(src, { signal: controller.signal }) | ||
| .then((res) => { | ||
| if (!res.ok) { | ||
| throw new Error(`Failed to load markdown (${res.status})`); | ||
| } | ||
| return res.text(); | ||
| }) | ||
| .then((markdown) => { | ||
| const parsed = marked.parse(markdown, { async: false }); | ||
| const rendered = DOMPurify.sanitize(String(parsed), { | ||
| USE_PROFILES: { html: true }, | ||
| }); | ||
| setHtml(rendered); | ||
| }) | ||
| .catch((err: Error) => { | ||
| if (err.name !== 'AbortError') { | ||
| setError(err.message); | ||
| } | ||
| }) | ||
| .finally(() => { | ||
| window.clearTimeout(timeout); | ||
| if (!controller.signal.aborted) { | ||
| setLoading(false); | ||
| } | ||
| }); | ||
|
|
||
| return () => { | ||
| controller.abort(); | ||
| window.clearTimeout(timeout); | ||
| }; | ||
| }, [src]); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Timeout aborts leave the component stuck on “Loading…” forever.
When the 10s timeout fires (Line 30), controller.abort() rejects the fetch with an AbortError. The catch skips setError because err.name === 'AbortError' (Line 50), and finally skips setLoading(false) because controller.signal.aborted is true (Line 56). The net effect on a slow/failed network is a permanent “Loading…” spinner with no error and no GitHub fallback link.
The signal.aborted guard is trying to suppress state updates after unmount, but it can't distinguish an unmount-abort from a timeout-abort. Use an explicit active flag set to false only in cleanup, and surface a timeout error to the user.
🐛 Proposed fix
useEffect(() => {
const controller = new AbortController();
+ let active = true;
const timeout = window.setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS);
setLoading(true);
setError(null);
fetch(src, { signal: controller.signal })
.then((res) => {
if (!res.ok) {
throw new Error(`Failed to load markdown (${res.status})`);
}
return res.text();
})
.then((markdown) => {
const parsed = marked.parse(markdown, { async: false });
const rendered = DOMPurify.sanitize(String(parsed), {
USE_PROFILES: { html: true },
});
- setHtml(rendered);
+ if (active) {
+ setHtml(rendered);
+ }
})
.catch((err: Error) => {
- if (err.name !== 'AbortError') {
- setError(err.message);
- }
+ if (!active) {
+ return;
+ }
+ setError(err.name === 'AbortError' ? 'Request timed out' : err.message);
})
.finally(() => {
window.clearTimeout(timeout);
- if (!controller.signal.aborted) {
+ if (active) {
setLoading(false);
}
});
return () => {
+ active = false;
controller.abort();
window.clearTimeout(timeout);
};
}, [src]);📝 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
| useEffect(() => { | |
| const controller = new AbortController(); | |
| const timeout = window.setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS); | |
| setLoading(true); | |
| setError(null); | |
| fetch(src, { signal: controller.signal }) | |
| .then((res) => { | |
| if (!res.ok) { | |
| throw new Error(`Failed to load markdown (${res.status})`); | |
| } | |
| return res.text(); | |
| }) | |
| .then((markdown) => { | |
| const parsed = marked.parse(markdown, { async: false }); | |
| const rendered = DOMPurify.sanitize(String(parsed), { | |
| USE_PROFILES: { html: true }, | |
| }); | |
| setHtml(rendered); | |
| }) | |
| .catch((err: Error) => { | |
| if (err.name !== 'AbortError') { | |
| setError(err.message); | |
| } | |
| }) | |
| .finally(() => { | |
| window.clearTimeout(timeout); | |
| if (!controller.signal.aborted) { | |
| setLoading(false); | |
| } | |
| }); | |
| return () => { | |
| controller.abort(); | |
| window.clearTimeout(timeout); | |
| }; | |
| }, [src]); | |
| useEffect(() => { | |
| const controller = new AbortController(); | |
| let active = true; | |
| const timeout = window.setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS); | |
| setLoading(true); | |
| setError(null); | |
| fetch(src, { signal: controller.signal }) | |
| .then((res) => { | |
| if (!res.ok) { | |
| throw new Error(`Failed to load markdown (${res.status})`); | |
| } | |
| return res.text(); | |
| }) | |
| .then((markdown) => { | |
| const parsed = marked.parse(markdown, { async: false }); | |
| const rendered = DOMPurify.sanitize(String(parsed), { | |
| USE_PROFILES: { html: true }, | |
| }); | |
| if (active) { | |
| setHtml(rendered); | |
| } | |
| }) | |
| .catch((err: Error) => { | |
| if (!active) { | |
| return; | |
| } | |
| setError(err.name === 'AbortError' ? 'Request timed out' : err.message); | |
| }) | |
| .finally(() => { | |
| window.clearTimeout(timeout); | |
| if (active) { | |
| setLoading(false); | |
| } | |
| }); | |
| return () => { | |
| active = false; | |
| controller.abort(); | |
| window.clearTimeout(timeout); | |
| }; | |
| }, [src]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@application/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsx`
around lines 28 - 65, The MarkdownFromRepo useEffect in MarkdownFromRepo.tsx
leaves the UI stuck in loading state when FETCH_TIMEOUT_MS aborts the request
because the AbortError path skips both setError and setLoading(false). Update
the fetch flow to distinguish a timeout abort from cleanup/unmount by using an
explicit active flag in the effect cleanup, and when the timeout controller
triggers, surface a user-visible timeout error instead of suppressing it. Keep
the existing fetch/marked.parse/DOMPurify logic, but ensure setLoading(false)
runs for timeout failures while still preventing state updates after unmount.
Comment on lines
+166
to
+178
| PathSpec( | ||
| "/rest/v1/map_analysis", | ||
| "map_analysis", | ||
| tags=["Gap analysis"], | ||
| summary="Map analysis between two standards", | ||
| description="Returns cached gap analysis or enqueues a background job.", | ||
| query_schema=schemas.MapAnalysisQuerySchema, | ||
| response_schema=schemas.MapAnalysisResponseSchema, | ||
| extra_responses={ | ||
| "400": {"description": "Fewer than two standards provided"}, | ||
| "503": {"description": "Gap analysis unavailable"}, | ||
| }, | ||
| ), |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Update map_analysis description to reflect production cache-only behavior.
The description "Returns cached gap analysis or enqueues a background job" implies computation on cache miss, but on production the endpoint is cache-only and returns 404 on cache miss. This public API documentation will mislead consumers who expect job-enqueue behavior.
As per coding guidelines: "On Heroku/opencreorg production, gap analysis is cache-only: serve precomputed rows from Postgres; do not compute gap analysis on production (cache miss → 404)".
📝 Proposed fix
summary="Map analysis between two standards",
- description="Returns cached gap analysis or enqueues a background job.",
+ description="Returns cached gap analysis. Returns 404 if no cached result is available.",📝 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
| PathSpec( | |
| "/rest/v1/map_analysis", | |
| "map_analysis", | |
| tags=["Gap analysis"], | |
| summary="Map analysis between two standards", | |
| description="Returns cached gap analysis or enqueues a background job.", | |
| query_schema=schemas.MapAnalysisQuerySchema, | |
| response_schema=schemas.MapAnalysisResponseSchema, | |
| extra_responses={ | |
| "400": {"description": "Fewer than two standards provided"}, | |
| "503": {"description": "Gap analysis unavailable"}, | |
| }, | |
| ), | |
| PathSpec( | |
| "/rest/v1/map_analysis", | |
| "map_analysis", | |
| tags=["Gap analysis"], | |
| summary="Map analysis between two standards", | |
| description="Returns cached gap analysis. Returns 404 if no cached result is available.", | |
| query_schema=schemas.MapAnalysisQuerySchema, | |
| response_schema=schemas.MapAnalysisResponseSchema, | |
| extra_responses={ | |
| "400": {"description": "Fewer than two standards provided"}, | |
| "503": {"description": "Gap analysis unavailable"}, | |
| }, | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@application/web/openapi_registry.py` around lines 166 - 178, The
`map_analysis` OpenAPI entry currently describes behavior that no longer matches
production. Update the `PathSpec` for `map_analysis` in `openapi_registry.py` so
its description reflects cache-only production behavior: it serves precomputed
gap analysis from Postgres and does not enqueue work on cache miss, which should
be documented as returning 404 instead. Keep the `summary` and response schema
references intact, but make the `description` and any related response text
consistent with the actual `map_analysis` endpoint behavior.
Source: Coding guidelines
Comment on lines
+257
to
+271
| PathSpec( | ||
| "/rest/v1/deeplink/{name}", | ||
| "deeplink", | ||
| tags=["Deeplink"], | ||
| summary="Resolve REST deeplink by name", | ||
| description="Redirects to an external hyperlink when found.", | ||
| not_found=True, | ||
| ), | ||
| PathSpec( | ||
| "/rest/v1/deeplink/{ntype}/{name}", | ||
| "deeplink", | ||
| tags=["Deeplink"], | ||
| summary="Resolve REST deeplink by type and name", | ||
| not_found=True, | ||
| ), |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the actual deeplink view response behavior.
# Find the view function and check if it returns a redirect.
rg -nP -A5 'def deeplink\b' --type=py application/web/web_main.pyRepository: OWASP/OpenCRE
Length of output: 403
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== openapi_registry.py around deeplink PathSpecs =="
sed -n '230,300p' application/web/openapi_registry.py
echo
echo "== deeplink view body =="
sed -n '774,860p' application/web/web_main.py
echo
echo "== search for redirect or response overrides related to deeplink =="
rg -n "deeplink|response_override|response_schema|Location|redirect" application/web/openapi_registry.py application/web/web_main.py docs/api/openapi.yamlRepository: OWASP/OpenCRE
Length of output: 11698
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '56,100p' docs/api/openapi.yamlRepository: OWASP/OpenCRE
Length of output: 1163
Document the deeplink redirect response
Both deeplink endpoints return redirects, but the generated OpenAPI still advertises 200: Success. Add a 302 response with a Location header so the spec matches runtime behavior and docs/api/openapi.yaml.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@application/web/openapi_registry.py` around lines 257 - 271, The deeplink
PathSpec entries still describe a success response instead of the redirect
actually returned at runtime. Update the openapi_registry deeplink definitions
for both resolve-by-name and resolve-by-type endpoints to include a 302 response
with a Location header, so the generated OpenAPI and docs/api/openapi.yaml match
the redirect behavior.
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.
Summary
make openapi-generate,make openapi-guardrail, and CI enforcement (Docs: OpenAPI spec, dedicated docs page, API links, and homepage FAQ #961 OpenAPI slice).docs/faq.mdas the FAQ source of truth, served at/docs/faq.mdand linked from the README./docspage with Swagger UI, FAQ rendering, header nav, and homepage footer links (Docs: OpenAPI spec, dedicated docs page, API links, and homepage FAQ #961 docs slice).Commits
feat(openapi): generate public REST spec with deterministic guardraildocs: add FAQ markdown and serve from /docs/faq.mdfeat(frontend): add /docs page with Swagger UI and nav linksTest plan
make openapi-guardrailpython -m unittest application.tests.openapi_guardrail_testpython -m unittest application.tests.web_main_testmake frontend/docs,/docs#api-reference,/docs#faq/docsand/docs#api-referenceCloses #961
Made with Cursor