feat(poc): generate tsoa openapi spec on deployment#252
feat(poc): generate tsoa openapi spec on deployment#252calthejuggler wants to merge 6 commits intomasterfrom
Conversation
calthejuggler
requested review from
bmsilva,
marcio-absmartly and
mario-silva
as code owners
✅ Deploy Preview for absmartly-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Office API documentation generation and integration: a GitHub Actions step runs a new shell script that clones an external repository branch with a sparse checkout of office sources, runs npm installs and Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/generate-office-api.sh`:
- Around line 9-10: The temp checkout created in REPO_DIR and the EXIT trap
remove the copied office-api-spec.json because the script later runs commands
with OLDPWD set to the temp dir; to fix, preserve the original working directory
(e.g., save it to a variable like ORIGINAL_PWD before creating REPO_DIR) or use
pushd/popd so subsequent cp calls target the real repo root, and update the copy
commands that reference office-api-spec.json (and the sections around
trap/REPO_DIR usage) to write to "$ORIGINAL_PWD/office-api-spec.json" (or use
pushd/popd) instead of relying on OLDPWD so the file survives after the EXIT
trap.
- Around line 19-31: The script currently hardcodes BRANCH="main" and then runs
git checkout "$BRANCH", which causes docs to be built from main instead of a
released Office backend; change the logic so BRANCH is resolved to the latest
release branch or tag (or read from an explicit environment variable like
OFFICE_TSOA_BRANCH) instead of defaulting to "main", and fail loudly if no
release branch/tag can be determined; update the code paths that reference
BRANCH and the git checkout call to use this resolved release identifier so
generated docs always come from a released Office backend.
- Around line 4-7: The script currently treats a missing ABS_REPO_ACCESS_TOKEN
as success; change scripts/generate-office-api.sh so that if
ABS_REPO_ACCESS_TOKEN is missing it fails the pipeline (exit 1) to avoid
silently publishing stale office-api-spec.json, but allow an explicit opt-in
override (e.g., respect a SKIP_OFFICE_API_GENERATION or UNTRUSTED_PR flag) to
no-op when intentionally requested; update the echo message to indicate why it
failed and mention office-api-spec.json to make the failure clear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 188ad704-7be3-44a4-a468-e583158cae17
📒 Files selected for processing (6)
.github/workflows/build.ymldocs/APIs-and-SDKs/Office-API/_category_.jsondocusaurus.config.jsnetlify.tomloffice-api-spec.jsonscripts/generate-office-api.sh
| if [ -z "${ABS_REPO_ACCESS_TOKEN:-}" ]; then | ||
| echo "No ABS_REPO_ACCESS_TOKEN set, skipping Office API spec generation" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
Skip only when missing-token generation is explicitly optional.
exit 0 here turns a missing or revoked token into a green build. Because office-api-spec.json is committed, the rest of the pipeline can still publish stale or empty Office docs instead of failing loudly. Please only no-op for opt-in cases such as untrusted PRs.
🛠️ Possible fix
if [ -z "${ABS_REPO_ACCESS_TOKEN:-}" ]; then
- echo "No ABS_REPO_ACCESS_TOKEN set, skipping Office API spec generation"
- exit 0
+ echo "ABS_REPO_ACCESS_TOKEN is not set"
+ if [ "${ALLOW_MISSING_OFFICE_API_SPEC:-0}" = "1" ]; then
+ echo "Skipping Office API spec generation"
+ exit 0
+ fi
+ exit 1
fi📝 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.
| if [ -z "${ABS_REPO_ACCESS_TOKEN:-}" ]; then | |
| echo "No ABS_REPO_ACCESS_TOKEN set, skipping Office API spec generation" | |
| exit 0 | |
| fi | |
| if [ -z "${ABS_REPO_ACCESS_TOKEN:-}" ]; then | |
| echo "ABS_REPO_ACCESS_TOKEN is not set" | |
| if [ "${ALLOW_MISSING_OFFICE_API_SPEC:-0}" = "1" ]; then | |
| echo "Skipping Office API spec generation" | |
| exit 0 | |
| fi | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/generate-office-api.sh` around lines 4 - 7, The script currently
treats a missing ABS_REPO_ACCESS_TOKEN as success; change
scripts/generate-office-api.sh so that if ABS_REPO_ACCESS_TOKEN is missing it
fails the pipeline (exit 1) to avoid silently publishing stale
office-api-spec.json, but allow an explicit opt-in override (e.g., respect a
SKIP_OFFICE_API_GENERATION or UNTRUSTED_PR flag) to no-op when intentionally
requested; update the echo message to indicate why it failed and mention
office-api-spec.json to make the failure clear.
| # TODO: Once tsoa is in a release branch, switch to auto-detecting latest release: | ||
| # LATEST=$(git branch -r \ | ||
| # | sed 's|origin/||; s/^[[:space:]]*//' \ | ||
| # | grep -E '^release/[0-9]+-[0-9]+$' \ | ||
| # | sed 's|release/||' \ | ||
| # | sort -t'-' -k1,1rn -k2,2rn \ | ||
| # | head -1) | ||
| # BRANCH="release/$LATEST" | ||
| BRANCH="main" | ||
| echo "Using branch: $BRANCH" | ||
|
|
||
| git sparse-checkout set office/backend office/shared | ||
| git checkout "$BRANCH" |
There was a problem hiding this comment.
Avoid sourcing the published Office spec from main.
Merging this as-is means the live docs are generated from whatever happens to be on main, not from the released Office backend. Once main drifts, the docs can advertise endpoints or schemas that are not available in production.
🛠️ Possible stopgap until auto-detection lands
- BRANCH="main"
+ : "${ABS_OFFICE_API_BRANCH:?Set ABS_OFFICE_API_BRANCH to the Office release branch}"
+ BRANCH="$ABS_OFFICE_API_BRANCH"📝 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.
| # TODO: Once tsoa is in a release branch, switch to auto-detecting latest release: | |
| # LATEST=$(git branch -r \ | |
| # | sed 's|origin/||; s/^[[:space:]]*//' \ | |
| # | grep -E '^release/[0-9]+-[0-9]+$' \ | |
| # | sed 's|release/||' \ | |
| # | sort -t'-' -k1,1rn -k2,2rn \ | |
| # | head -1) | |
| # BRANCH="release/$LATEST" | |
| BRANCH="main" | |
| echo "Using branch: $BRANCH" | |
| git sparse-checkout set office/backend office/shared | |
| git checkout "$BRANCH" | |
| # TODO: Once tsoa is in a release branch, switch to auto-detecting latest release: | |
| # LATEST=$(git branch -r \ | |
| # | sed 's|origin/||; s/^[[:space:]]*//' \ | |
| # | grep -E '^release/[0-9]+-[0-9]+$' \ | |
| # | sed 's|release/||' \ | |
| # | sort -t'-' -k1,1rn -k2,2rn \ | |
| # | head -1) | |
| # BRANCH="release/$LATEST" | |
| : "${ABS_OFFICE_API_BRANCH:?Set ABS_OFFICE_API_BRANCH to the Office release branch}" | |
| BRANCH="$ABS_OFFICE_API_BRANCH" | |
| echo "Using branch: $BRANCH" | |
| git sparse-checkout set office/backend office/shared | |
| git checkout "$BRANCH" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/generate-office-api.sh` around lines 19 - 31, The script currently
hardcodes BRANCH="main" and then runs git checkout "$BRANCH", which causes docs
to be built from main instead of a released Office backend; change the logic so
BRANCH is resolved to the latest release branch or tag (or read from an explicit
environment variable like OFFICE_TSOA_BRANCH) instead of defaulting to "main",
and fail loudly if no release branch/tag can be determined; update the code
paths that reference BRANCH and the git checkout call to use this resolved
release identifier so generated docs always come from a released Office backend.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
scripts/generate-office-api.sh (2)
21-33:⚠️ Potential issue | 🟠 MajorDon’t publish the Office spec from
main.This POC is meant to generate the spec from the latest release branch, but
BRANCH="main"means the published docs can drift ahead of the released Office backend. Until auto-detection lands, please require an explicit release branch and fail if it is not provided.Suggested stopgap
- BRANCH="main" + : "${ABS_OFFICE_API_BRANCH:?Set ABS_OFFICE_API_BRANCH to the Office release branch}" + BRANCH="$ABS_OFFICE_API_BRANCH" echo "Using branch: $BRANCH"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate-office-api.sh` around lines 21 - 33, The script currently hardcodes BRANCH="main" and checks out that branch; change it to require an explicit release branch (e.g., via an environment variable or script argument) and fail fast if none is supplied: remove the BRANCH="main" default, read BRANCH from an input (or $1 / $RELEASE_BRANCH), validate it matches the expected release pattern (e.g., release/X-Y), and exit non-zero with a clear message if validation fails before running git sparse-checkout set or git checkout "$BRANCH".
6-8:⚠️ Potential issue | 🟠 MajorFail loudly unless skipping generation is explicitly intended.
exit 0here still turns a missing or revokedABS_REPO_ACCESS_TOKENinto a green build, so the pipeline can keep publishing the committedoffice-api-spec.jsoninstead of a freshly generated spec. Please only no-op behind an explicit override.Suggested stopgap
if [ -z "${ABS_REPO_ACCESS_TOKEN:-}" ]; then - echo "No ABS_REPO_ACCESS_TOKEN set, skipping Office API spec generation" - exit 0 + echo "ABS_REPO_ACCESS_TOKEN is not set; cannot refresh office-api-spec.json" + if [ "${ALLOW_MISSING_OFFICE_API_SPEC:-0}" = "1" ]; then + echo "Skipping Office API spec generation by explicit override" + exit 0 + fi + exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate-office-api.sh` around lines 6 - 8, The script currently treats a missing ABS_REPO_ACCESS_TOKEN as OK by exiting 0; change this to fail the build unless an explicit override is set: check ABS_REPO_ACCESS_TOKEN and if missing, only no-op when a new explicit flag like SKIP_OFFICE_API_GENERATION is set to "true" (e.g., if [ "${SKIP_OFFICE_API_GENERATION:-}" = "true" ]; then echo "Skipping Office API spec generation by explicit override"; exit 0; else echo "ABS_REPO_ACCESS_TOKEN missing; set ABS_REPO_ACCESS_TOKEN or SKIP_OFFICE_API_GENERATION=true to skip" >&2; exit 1; fi). Update the message to document the override and ensure ABS_REPO_ACCESS_TOKEN remains the gating variable referenced in the condition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/generate-office-api.sh`:
- Around line 21-33: The script currently hardcodes BRANCH="main" and checks out
that branch; change it to require an explicit release branch (e.g., via an
environment variable or script argument) and fail fast if none is supplied:
remove the BRANCH="main" default, read BRANCH from an input (or $1 /
$RELEASE_BRANCH), validate it matches the expected release pattern (e.g.,
release/X-Y), and exit non-zero with a clear message if validation fails before
running git sparse-checkout set or git checkout "$BRANCH".
- Around line 6-8: The script currently treats a missing ABS_REPO_ACCESS_TOKEN
as OK by exiting 0; change this to fail the build unless an explicit override is
set: check ABS_REPO_ACCESS_TOKEN and if missing, only no-op when a new explicit
flag like SKIP_OFFICE_API_GENERATION is set to "true" (e.g., if [
"${SKIP_OFFICE_API_GENERATION:-}" = "true" ]; then echo "Skipping Office API
spec generation by explicit override"; exit 0; else echo "ABS_REPO_ACCESS_TOKEN
missing; set ABS_REPO_ACCESS_TOKEN or SKIP_OFFICE_API_GENERATION=true to skip"
>&2; exit 1; fi). Update the message to document the override and ensure
ABS_REPO_ACCESS_TOKEN remains the gating variable referenced in the condition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67a1ee68-9ad5-46ed-9400-7f9fbf2bac9c
📒 Files selected for processing (1)
scripts/generate-office-api.sh
calthejuggler
force-pushed
the
poc/generate-tsoa-openapi-spec
branch
from
3ab862e to
b23952e
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/generate-office-api.sh (1)
12-12: Use single quotes for the trap to defer variable expansion.With double quotes,
$REPO_DIRis expanded when the trap is defined, not when it fires. While this works here since the variable doesn't change, single quotes are the idiomatic approach and prevent subtle bugs if the script is refactored.🛠️ Suggested fix
-trap "rm -rf $REPO_DIR" EXIT +trap 'rm -rf "$REPO_DIR"' EXIT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate-office-api.sh` at line 12, The trap currently uses double quotes causing $REPO_DIR to be expanded immediately; change the trap invocation to use single quotes so the shell defers expansion until the trap runs (update the trap command that references REPO_DIR in scripts/generate-office-api.sh to use single-quoted string), ensuring the cleanup uses the value at trap execution time and avoids future subtle bugs if REPO_DIR changes.office-api-spec.json (1)
1-9: Security rules should be defined in the tsoa source, not this placeholder.The static analysis tool flags the missing
securityfield (CKV_OPENAPI_4). Since this file is auto-generated by tsoa from the Office backend, ensure the security definitions are configured in the tsoa specification source (typicallytsoa.jsonor controller decorators) rather than manually editing this placeholder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@office-api-spec.json` around lines 1 - 9, The generated OpenAPI spec is missing security definitions; don't edit office-api-spec.json directly—add the security configuration in the TSOA source (e.g., update tsoa.json with components.securitySchemes and global "security" or annotate controllers/methods with `@Security` decorators) so TSOA emits the securitySchemes and security arrays into the generated spec, then regenerate using scripts/generate-office-api.sh to update office-api-spec.json accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@office-api-spec.json`:
- Around line 1-9: The generated OpenAPI spec is missing security definitions;
don't edit office-api-spec.json directly—add the security configuration in the
TSOA source (e.g., update tsoa.json with components.securitySchemes and global
"security" or annotate controllers/methods with `@Security` decorators) so TSOA
emits the securitySchemes and security arrays into the generated spec, then
regenerate using scripts/generate-office-api.sh to update office-api-spec.json
accordingly.
In `@scripts/generate-office-api.sh`:
- Line 12: The trap currently uses double quotes causing $REPO_DIR to be
expanded immediately; change the trap invocation to use single quotes so the
shell defers expansion until the trap runs (update the trap command that
references REPO_DIR in scripts/generate-office-api.sh to use single-quoted
string), ensuring the cleanup uses the value at trap execution time and avoids
future subtle bugs if REPO_DIR changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 749994f7-c2a0-4f87-8284-4ca833a406e0
📒 Files selected for processing (6)
.github/workflows/build.ymldocs/APIs-and-SDKs/Office-API/_category_.jsondocusaurus.config.jsnetlify.tomloffice-api-spec.jsonscripts/generate-office-api.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/build.yml
- docs/APIs-and-SDKs/Office-API/category.json
- docusaurus.config.js
- netlify.toml
calthejuggler
force-pushed
the
poc/generate-tsoa-openapi-spec
branch
from
b23952e to
59b1ecc
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
scripts/generate-office-api.sh (2)
21-33:⚠️ Potential issue | 🟠 MajorDo not generate the published spec from a feature branch.
Line 29 hard-codes
vk/d96f-migrate-one-set, which does not match the PR objective of using the latest release branch and will also break once that branch is renamed or deleted. Resolve the release ref dynamically, or require an explicit release branch/tag via environment and fail loudly when it is missing.Stopgap fix
- BRANCH="vk/d96f-migrate-one-set" + : "${ABS_OFFICE_API_BRANCH:?Set ABS_OFFICE_API_BRANCH to the Office release branch}" + BRANCH="$ABS_OFFICE_API_BRANCH" echo "Using branch: $BRANCH"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate-office-api.sh` around lines 21 - 33, The script currently hard-codes BRANCH="vk/d96f-migrate-one-set"; change it to resolve the release ref dynamically or require an explicit env var: check for a provided environment variable (e.g., RELEASE_BRANCH or RELEASE_TAG) and use it if present, otherwise implement the previous LATEST-detection logic (the git sparse-checkout + git checkout flow should then use the resolved BRANCH variable); if neither the env var nor a discovered release branch is available, exit with a non-zero status and a clear error message so the published spec is never generated from a feature branch.
6-8:⚠️ Potential issue | 🟠 MajorFail by default when the token is missing.
Line 8 exits successfully, so a production build can keep using the checked-in
office-api-spec.jsoninstead of a freshly generated spec. Please only no-op when that skip is explicitly requested for non-deploy contexts.Possible fix
if [ -z "${ABS_REPO_ACCESS_TOKEN:-}" ]; then - echo "No ABS_REPO_ACCESS_TOKEN set, skipping Office API spec generation" - exit 0 + echo "ABS_REPO_ACCESS_TOKEN is not set; cannot refresh office-api-spec.json" + if [ "${SKIP_OFFICE_API_GENERATION:-0}" = "1" ]; then + echo "Skipping Office API spec generation by request" + exit 0 + fi + exit 1 fiBased on learnings, only the
masterbranch is deployed to Netlify's production context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate-office-api.sh` around lines 6 - 8, The script currently no-ops when ABS_REPO_ACCESS_TOKEN is missing (exit 0), allowing checked-in specs to slip into production; change the logic so that when ABS_REPO_ACCESS_TOKEN is unset the script exits non-zero (exit 1) to fail the build by default, but allow an explicit opt-out via a new SKIP_OFFICE_API_GENERATION env var (if SKIP_OFFICE_API_GENERATION is set, print a skip message and exit 0). Update the echo message to reflect the failure case and reference ABS_REPO_ACCESS_TOKEN and SKIP_OFFICE_API_GENERATION so reviewers can find/modify the logic.
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
23-26: Scope this step to deploy builds only.This workflow also runs on pull requests and
development, so the new step adds a private-repo clone plus two extra installs to builds that do not publish the production docs. If the goal is deployment-time generation, I would move this to the deploy pipeline or guard it to production/master builds.Based on learnings, only the
masterbranch is deployed to Netlify's production context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build.yml around lines 23 - 26, The "Generate Office API spec" workflow step currently runs for PRs and development; restrict it to deploy-only by moving the step into the deploy workflow or adding a run condition such as gating the step by branch (e.g., only run when github.ref is refs/heads/master) so scripts/generate-office-api.sh and the ABS_REPO_ACCESS_TOKEN are invoked only on production/master deploys; update the step named "Generate Office API spec" to include that branch guard or relocate it to the deploy pipeline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/generate-office-api.sh`:
- Around line 21-33: The script currently hard-codes
BRANCH="vk/d96f-migrate-one-set"; change it to resolve the release ref
dynamically or require an explicit env var: check for a provided environment
variable (e.g., RELEASE_BRANCH or RELEASE_TAG) and use it if present, otherwise
implement the previous LATEST-detection logic (the git sparse-checkout + git
checkout flow should then use the resolved BRANCH variable); if neither the env
var nor a discovered release branch is available, exit with a non-zero status
and a clear error message so the published spec is never generated from a
feature branch.
- Around line 6-8: The script currently no-ops when ABS_REPO_ACCESS_TOKEN is
missing (exit 0), allowing checked-in specs to slip into production; change the
logic so that when ABS_REPO_ACCESS_TOKEN is unset the script exits non-zero
(exit 1) to fail the build by default, but allow an explicit opt-out via a new
SKIP_OFFICE_API_GENERATION env var (if SKIP_OFFICE_API_GENERATION is set, print
a skip message and exit 0). Update the echo message to reflect the failure case
and reference ABS_REPO_ACCESS_TOKEN and SKIP_OFFICE_API_GENERATION so reviewers
can find/modify the logic.
---
Nitpick comments:
In @.github/workflows/build.yml:
- Around line 23-26: The "Generate Office API spec" workflow step currently runs
for PRs and development; restrict it to deploy-only by moving the step into the
deploy workflow or adding a run condition such as gating the step by branch
(e.g., only run when github.ref is refs/heads/master) so
scripts/generate-office-api.sh and the ABS_REPO_ACCESS_TOKEN are invoked only on
production/master deploys; update the step named "Generate Office API spec" to
include that branch guard or relocate it to the deploy pipeline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4cd77332-7291-492f-92df-d84ebc9625ce
📒 Files selected for processing (6)
.github/workflows/build.ymldocs/APIs-and-SDKs/Office-API/_category_.jsondocusaurus.config.jsnetlify.tomloffice-api-spec.jsonscripts/generate-office-api.sh
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/APIs-and-SDKs/Office-API/category.json
- docusaurus.config.js
- netlify.toml
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/generate-office-api.sh (2)
35-38: Consider using absolute paths for robustness.The relative
cdcommands work but are fragile if the directory structure changes. Using absolute paths based on$REPO_DIRwould make the script more maintainable.♻️ Suggested refactor
-cd office/shared/lib +cd "$REPO_DIR/office/shared/lib" npm ci -cd ../../backend +cd "$REPO_DIR/office/backend" npm ci --ignore-scripts npx tsoa spec🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate-office-api.sh` around lines 35 - 38, The script uses fragile relative cd commands; change them to absolute paths using the repository root variable (e.g., $REPO_DIR) to avoid breakage: replace `cd office/shared/lib` with `cd "$REPO_DIR/office/shared/lib"` and `cd ../../backend` with `cd "$REPO_DIR/backend"`, then run the same `npm ci` and `npm ci --ignore-scripts` commands in those absolute locations to preserve behavior.
12-12: Use single quotes in trap to defer variable expansion.With double quotes,
$REPO_DIRexpands immediately when the trap is set. Whilst this works here sinceREPO_DIRis never reassigned, single quotes are the idiomatic pattern and prevent subtle bugs if the script evolves.🛠️ Suggested fix
-trap "rm -rf $REPO_DIR" EXIT +trap 'rm -rf "$REPO_DIR"' EXIT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate-office-api.sh` at line 12, Update the trap so the shell expands $REPO_DIR at trap execution time rather than when the trap is defined: change the quoted argument to the trap command (refer to the existing trap "rm -rf $REPO_DIR" EXIT) to use single quotes so $REPO_DIR is deferred, keeping the rest of the trap behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@scripts/generate-office-api.sh`:
- Around line 35-38: The script uses fragile relative cd commands; change them
to absolute paths using the repository root variable (e.g., $REPO_DIR) to avoid
breakage: replace `cd office/shared/lib` with `cd "$REPO_DIR/office/shared/lib"`
and `cd ../../backend` with `cd "$REPO_DIR/backend"`, then run the same `npm ci`
and `npm ci --ignore-scripts` commands in those absolute locations to preserve
behavior.
- Line 12: Update the trap so the shell expands $REPO_DIR at trap execution time
rather than when the trap is defined: change the quoted argument to the trap
command (refer to the existing trap "rm -rf $REPO_DIR" EXIT) to use single
quotes so $REPO_DIR is deferred, keeping the rest of the trap behavior
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0f607bf4-a372-4573-bdd8-d935ac3f8b89
📒 Files selected for processing (2)
docusaurus.config.jsscripts/generate-office-api.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- docusaurus.config.js
1 participant
This PR is a POC for auto-generating our open api spec, based on the latest release branch BE package.
Summary by CodeRabbit
New Features
Chores