Repository navigation
Fix production OpenFGA model rollout - #119
Conversation
📝 WalkthroughWalkthroughOpenFGA deployment now persists a model SHA-256 digest, conditionally writes models when the digest changes, restores prior configuration on rollback, and verifies bootstrap, rollout, no-op, and rollback behavior in CI. ChangesOpenFGA model bootstrap
Conditional deployment rollout
Rollout and rollback verification
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant deploy.sh
participant OpenFGA
participant openfga-model-write
participant ApplicationStack
deploy.sh->>OpenFGA: Start and await readiness
deploy.sh->>openfga-model-write: Write model when SHA changes
openfga-model-write-->>deploy.sh: Return authorization model ID
deploy.sh->>ApplicationStack: Start release with pinned model configuration
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@infrastructure/deployment/compose.production.yaml`:
- Around line 199-201: Parameterize the OpenFGA compose service’s model volume
mount and the `/model/model.fga` argument to use the same overridable model-file
variable consumed by `deploy.sh` and `bootstrap-openfga.sh`, defaulting to the
current repository path. Ensure the digest input and model bytes written to
OpenFGA always come from the same file.
- Around line 190-218: Update the ORGMEMORY_OPENFGA_STORE_ID interpolation in
the openfga-model-write command to use the file’s required-variable fail-fast
syntax with a clear “Set ORGMEMORY_OPENFGA_STORE_ID” message, instead of
silently defaulting to an empty value. Preserve the existing command and service
configuration.
In `@infrastructure/deployment/scripts/deploy.sh`:
- Around line 105-137: Extract the duplicated env-file upsert awk logic from
update_openfga_model_configuration() in
infrastructure/deployment/scripts/deploy.sh (105-137) into a shared helper under
infrastructure/deployment/scripts/lib/env-file.sh that accepts key/value pairs
and the target file. Update
infrastructure/deployment/scripts/bootstrap-openfga.sh (81-119) so
update_environment_model() sources and uses this helper instead of its local awk
implementation; update_openfga_model_configuration() should use the same helper
while preserving replacement and append-at-EOF behavior.
- Around line 190-209: Make the model-write flow idempotent across failures
between openfga-model-write and update_openfga_model_configuration. Persist or
recover the successfully created model ID before retrying, and reuse it when the
repository digest is unchanged instead of creating another immutable model.
Update the logic around model_write_json, new_openfga_model_id, and
update_openfga_model_configuration while preserving the existing digest
comparison.
- Around line 190-205: The OpenFGA model-write command captured in
model_write_json must not allocate a pseudo-TTY, since its output is parsed as
JSON. Update the compose run invocation for openfga-model-write to include the
no-TTY option while preserving the existing profile, cleanup, and dependency
flags.
🪄 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.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 94a39c38-f313-4a90-9a30-60325bb52e95
⛔ Files ignored due to path filters (7)
docs/decisions/0017-pin-openfga-models-to-product-releases.mdis excluded by!docs/**docs/increments/active/2026-07-29-openfga-model-rollout/design.mdis excluded by!docs/**docs/increments/active/2026-07-29-openfga-model-rollout/plan.mdis excluded by!docs/**docs/roadmap.mdis excluded by!docs/**docs/runbooks/production-zm-deployment.mdis excluded by!docs/**docs/specs/domains/ai-model-control-plane.mdis excluded by!docs/**docs/tests/domains/ai-model-control-plane.mdis excluded by!docs/**
📒 Files selected for processing (7)
.github/workflows/ci.ymlARCHITECTURE.mdinfrastructure/deployment/compose.production.yamlinfrastructure/deployment/production.env.exampleinfrastructure/deployment/scripts/bootstrap-openfga.shinfrastructure/deployment/scripts/deploy.shinfrastructure/deployment/scripts/test-deploy-openfga-model-rollout.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Deployment contracts
- GitHub Check: Public docs · Node 24
🧰 Additional context used
📓 Path-based instructions (4)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
**/*: Always read the repository guidance and relevant sections ofARCHITECTURE.md; before changing a domain, read its specification, test-coverage document, and binding decision filenames.
Treat the repository as the engineering system of record; current repository and runtime evidence take precedence over chat or Northstar.
Readdocs/guidelines/agent-safety.mdbefore retrieval, AI, MCP, permission, upload, graph, or export work. Never commit secrets or customer data.
Files:
infrastructure/deployment/production.env.exampleARCHITECTURE.mdinfrastructure/deployment/compose.production.yamlinfrastructure/deployment/scripts/bootstrap-openfga.shinfrastructure/deployment/scripts/test-deploy-openfga-model-rollout.shinfrastructure/deployment/scripts/deploy.sh
ARCHITECTURE.md
📄 CodeRabbit inference engine (CLAUDE.md)
Keep
ARCHITECTURE.mdlimited to implemented facts, current project-wide facts, and commands; do not use it for intended or unimplemented behavior.
Files:
ARCHITECTURE.md
**/*.{java,gradle,gradle.kts,properties,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Before using unfamiliar Spring Boot 4, Spring Modulith 2, Spring AI 2, or Gradle APIs, consult current official documentation, Context7, and the relevant project verification skill.
Files:
infrastructure/deployment/compose.production.yaml
.github/**/*.{yml,yaml}
⚙️ CodeRabbit configuration file
.github/**/*.{yml,yaml}: Require least-privilege permissions, explicit release tags for actions,
bounded job timeouts, concurrency cancellation, frozen lockfiles, and no
secrets in pull-request workflows. GitHub Actions are intentionally not
pinned to commit SHAs; Dependabot owns their scheduled version updates.
Files:
.github/workflows/ci.yml
🧠 Learnings (2)
📚 Learning: 2026-07-27T14:53:53.633Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 92
File: infrastructure/deployment/compose.production.yaml:304-310
Timestamp: 2026-07-27T14:53:53.633Z
Learning: For OrgMemory’s Spring Boot SCIM configuration, the `application.yml`/`application-prod.yml` map `orgmemory.security.scim.*` properties via `${ORGMEMORY_SCIM_*}` placeholders. Therefore, in deployment Compose files and related environment/CI templates, set environment variables using the `ORGMEMORY_SCIM_*` names (e.g., `ORGMEMORY_SCIM_VERIFIER_KEY`) rather than “relaxed-binding-derived” names such as `ORGMEMORY_SECURITY_SCIM_*`. This is required to ensure Spring resolves the intended SCIM configuration properties.
Applied to files:
infrastructure/deployment/compose.production.yaml
📚 Learning: 2026-07-24T22:52:57.466Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 40
File: .github/workflows/ci.yml:126-126
Timestamp: 2026-07-24T22:52:57.466Z
Learning: In this repository’s GitHub Actions workflows, the `uses:` field may intentionally reference GitHub Actions by explicit release tags (not immutable commit SHAs) per the project’s OrgMemory policy. Do not flag tag-based `uses:` references as “unpinned” if they are release-tag-based (e.g., `owner/repovX.Y.Z`) and follow the repo’s Dependabot-owned scheduled updates approach.
Applied to files:
.github/workflows/ci.yml
🪛 ast-grep (0.45.0)
infrastructure/deployment/scripts/test-deploy-openfga-model-rollout.sh
[warning] 225-225: set +e (or set +o errexit) disables the shell's errexit option, so the script keeps running after a command fails. This masks failures of security-critical operations (downloads, signature/checksum verification, permission changes, cleanup of secrets), letting the script proceed with a bad or insecure state. Leave errexit enabled (set -e / set -euo pipefail), or handle failures explicitly with if/|| and an explicit exit instead of globally turning off failure detection.
Context: set +e
Note: [CWE-754] Improper Check for Unusual or Exceptional Conditions.
(set-plus-e-error-masking-bash)
🔇 Additional comments (9)
infrastructure/deployment/scripts/test-deploy-openfga-model-rollout.sh (2)
225-233: Static analysis false positive onset +e.The
set +ehere is immediately paired with capturingstatus="$?", re-enablingset -e, and explicitly checking the captured status — the standard idiom for capturing an expected-failure exit code undererrexit. This isn't masking an unusual condition; it's deliberately testing the rollback path. No change needed.Source: Linters/SAST tools
1-251: LGTM!infrastructure/deployment/production.env.example (1)
29-31: LGTM!infrastructure/deployment/scripts/bootstrap-openfga.sh (2)
4-7: LGTM!
121-123: LGTM!infrastructure/deployment/scripts/deploy.sh (2)
10-16: LGTM!
168-169: LGTM!Also applies to: 187-189, 211-224
ARCHITECTURE.md (1)
416-422: LGTM! Accurately reflects the implemented bootstrap/deploy/rollback behavior verified elsewhere in this PR..github/workflows/ci.yml (1)
527-529: LGTM!
| openfga-model-write: | ||
| image: openfga/cli:v0.7.19@sha256:2e0e250043ef480a9162623dbf1ff7a62a1a2cb96a79cb20577b144994ab114d | ||
| profiles: | ||
| - ops | ||
| command: | ||
| - model | ||
| - write | ||
| - --store-id | ||
| - ${ORGMEMORY_OPENFGA_STORE_ID:-} | ||
| - --file | ||
| - /model/model.fga | ||
| - --format | ||
| - fga | ||
| - --api-url | ||
| - http://openfga:8080 | ||
| depends_on: | ||
| openfga-ready: | ||
| condition: service_completed_successfully | ||
| networks: | ||
| - orgmemory-internal | ||
| volumes: | ||
| - ../../integrations/authorization-openfga/src/main/openfga/model.fga:/model/model.fga:ro | ||
| restart: "no" | ||
| read_only: true | ||
| security_opt: | ||
| - no-new-privileges:true | ||
| cap_drop: | ||
| - ALL | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
ORGMEMORY_OPENFGA_STORE_ID should fail fast like every other required variable in this file.
Line 198 uses ${ORGMEMORY_OPENFGA_STORE_ID:-} (silently empty default) while all other required variables in this file use :?Set VAR (e.g. line 63, 95, 291). If the store ID is ever empty (bootstrap failure, stale env, manual .env edit), fga model write --store-id "" will fail with an opaque CLI/API error mid-rollout instead of a clear pre-flight message, complicating incident response during exactly the kind of production rollout this PR is meant to make safer.
🛠️ Proposed fix
- --store-id
- - ${ORGMEMORY_OPENFGA_STORE_ID:-}
+ - ${ORGMEMORY_OPENFGA_STORE_ID:?Set ORGMEMORY_OPENFGA_STORE_ID}📝 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.
| openfga-model-write: | |
| image: openfga/cli:v0.7.19@sha256:2e0e250043ef480a9162623dbf1ff7a62a1a2cb96a79cb20577b144994ab114d | |
| profiles: | |
| - ops | |
| command: | |
| - model | |
| - write | |
| - --store-id | |
| - ${ORGMEMORY_OPENFGA_STORE_ID:-} | |
| - --file | |
| - /model/model.fga | |
| - --format | |
| - fga | |
| - --api-url | |
| - http://openfga:8080 | |
| depends_on: | |
| openfga-ready: | |
| condition: service_completed_successfully | |
| networks: | |
| - orgmemory-internal | |
| volumes: | |
| - ../../integrations/authorization-openfga/src/main/openfga/model.fga:/model/model.fga:ro | |
| restart: "no" | |
| read_only: true | |
| security_opt: | |
| - no-new-privileges:true | |
| cap_drop: | |
| - ALL | |
| openfga-model-write: | |
| image: openfga/cli:v0.7.19@sha256:2e0e250043ef480a9162623dbf1ff7a62a1a2cb96a79cb20577b144994ab114d | |
| profiles: | |
| - ops | |
| command: | |
| - model | |
| - write | |
| - --store-id | |
| - ${ORGMEMORY_OPENFGA_STORE_ID:?Set ORGMEMORY_OPENFGA_STORE_ID} | |
| - --file | |
| - /model/model.fga | |
| - --format | |
| - fga | |
| - --api-url | |
| - http://openfga:8080 | |
| depends_on: | |
| openfga-ready: | |
| condition: service_completed_successfully | |
| networks: | |
| - orgmemory-internal | |
| volumes: | |
| - ../../integrations/authorization-openfga/src/main/openfga/model.fga:/model/model.fga:ro | |
| restart: "no" | |
| read_only: true | |
| security_opt: | |
| - no-new-privileges:true | |
| cap_drop: | |
| - ALL |
🤖 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 `@infrastructure/deployment/compose.production.yaml` around lines 190 - 218,
Update the ORGMEMORY_OPENFGA_STORE_ID interpolation in the openfga-model-write
command to use the file’s required-variable fail-fast syntax with a clear “Set
ORGMEMORY_OPENFGA_STORE_ID” message, instead of silently defaulting to an empty
value. Preserve the existing command and service configuration.
| - --file | ||
| - /model/model.fga | ||
| - --format |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
Model source for digest vs. model write can silently diverge.
deploy.sh/bootstrap-openfga.sh compute the release SHA-256 from an overridable ORGMEMORY_OPENFGA_MODEL_FILE, but this compose service always mounts the hardcoded repo-relative path. If that override is ever used outside the test harness, the pinned digest would describe different bytes than what actually gets written into OpenFGA. Consider parameterizing this volume mount with the same variable (defaulting to the current hardcoded path) to keep the digest and the written model in sync, or document that the override is test-only.
Also applies to: 210-211
🤖 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 `@infrastructure/deployment/compose.production.yaml` around lines 199 - 201,
Parameterize the OpenFGA compose service’s model volume mount and the
`/model/model.fga` argument to use the same overridable model-file variable
consumed by `deploy.sh` and `bootstrap-openfga.sh`, defaulting to the current
repository path. Ensure the digest input and model bytes written to OpenFGA
always come from the same file.
| update_openfga_model_configuration() { | ||
| local model_id="$1" | ||
| local model_sha256="$2" | ||
| local temporary_file | ||
| temporary_file="$(mktemp)" | ||
|
|
||
| awk -v model_id="$model_id" -v model_sha256="$model_sha256" ' | ||
| BEGIN { | ||
| values["ORGMEMORY_OPENFGA_AUTHORIZATION_MODEL_ID"] = model_id | ||
| values["ORGMEMORY_OPENFGA_MODEL_SHA256"] = model_sha256 | ||
| } | ||
| { | ||
| split($0, parts, "=") | ||
| if (parts[1] in values) { | ||
| print parts[1] "=" values[parts[1]] | ||
| seen[parts[1]] = 1 | ||
| } else { | ||
| } | ||
| } | ||
| END { | ||
| for (key in values) { | ||
| if (!seen[key]) { | ||
| print key "=" values[key] | ||
| } | ||
| } | ||
| } | ||
| ' "$environment_file" > "$temporary_file" | ||
|
|
||
| install -m 0600 "$temporary_file" "$environment_file" | ||
| rm -f "$temporary_file" | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Extract the env-file key-upsert awk logic into a shared helper. Both new functions implement the identical "replace matching KEY= lines via awk, else append at EOF" pattern for persisting OpenFGA identifiers, duplicating the same logic (and echoing the pre-existing replace_image_references() in deploy.sh) across two scripts.
infrastructure/deployment/scripts/deploy.sh#L105-L137: extractupdate_openfga_model_configuration()'s awk body into a small shared shell library (e.g.infrastructure/deployment/scripts/lib/env-file.sh) that takes a set of key/value pairs and the target file.infrastructure/deployment/scripts/bootstrap-openfga.sh#L81-L119: source the same shared helper forupdate_environment_model()instead of re-implementing the awk upsert.
📍 Affects 2 files
infrastructure/deployment/scripts/deploy.sh#L105-L137(this comment)infrastructure/deployment/scripts/bootstrap-openfga.sh#L81-L119
🤖 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 `@infrastructure/deployment/scripts/deploy.sh` around lines 105 - 137, Extract
the duplicated env-file upsert awk logic from
update_openfga_model_configuration() in
infrastructure/deployment/scripts/deploy.sh (105-137) into a shared helper under
infrastructure/deployment/scripts/lib/env-file.sh that accepts key/value pairs
and the target file. Update
infrastructure/deployment/scripts/bootstrap-openfga.sh (81-119) so
update_environment_model() sources and uses this helper instead of its local awk
implementation; update_openfga_model_configuration() should use the same helper
while preserving replacement and append-at-EOF behavior.
| if [[ "$openfga_model_sha256" != "$release_model_sha256" ]]; then | ||
| model_write_json="$( | ||
| "${compose[@]}" --profile ops run --rm --no-deps openfga-model-write | ||
| )" | ||
| new_openfga_model_id="$( | ||
| MODEL_WRITE_JSON="$model_write_json" python3 - <<'PY' | ||
| import json | ||
| import os | ||
|
|
||
| payload = json.loads(os.environ["MODEL_WRITE_JSON"]) | ||
| model_id = payload.get("authorization_model_id") or payload.get("id") | ||
| if not model_id: | ||
| raise SystemExit("OpenFGA CLI response did not contain an authorization model id") | ||
| print(model_id) | ||
| PY | ||
| )" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does docker compose run allocate a pseudo-TTY by default, and what flag disables it?
💡 Result:
By default, docker compose run automatically detects whether to allocate a pseudo-TTY [1][2][3]. It attempts to allocate a TTY if the command is run in an interactive terminal session [2]. If the input is not a terminal (for example, when piped or running in a CI/CD environment), it will default to disabling TTY allocation [2]. To explicitly disable pseudo-TTY allocation, you can use the -T or --no-tty flag [1][3].
Citations:
- 1: https://docs.docker.com/reference/cli/docker/compose/run/
- 2: https://github.com/docker/compose/blob/main/cmd/compose/run.go
- 3: https://manpages.debian.org/trixie/docker-compose/docker-compose-run.1.en.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repository guidance files =="
git ls-files | rg '(^|/)ARCHITECTURE\.md$|(^|/)agent-safety\.md$|(^|/)specification\.|test-coverage|binding decision|deploy\.sh$|docker-compose|compose' || true
echo
echo "== deploy.sh context =="
if [ -f infrastructure/deployment/scripts/deploy.sh ]; then
wc -l infrastructure/deployment/scripts/deploy.sh
sed -n '160,230p' infrastructure/deployment/scripts/deploy.sh | nl -ba -v160
fi
echo
echo "== openfga compose references =="
rg -n "openfga-model-write|docker compose|compose\\[" -S . || trueRepository: kl3inIT/OrgMemory
Length of output: 551
🌐 Web query:
Docker Compose run default TTY allocation documentation --no-TTY --no-tty
💡 Result:
In Docker Compose, the docker compose run command manages pseudo-TTY allocation with specific behaviors designed for interactive use [1]. TTY Allocation and Defaults By default, docker compose run attempts to allocate a pseudo-TTY if the command is run in an interactive environment [1]. However, to ensure compatibility with non-interactive environments (such as when input is being piped or redirected), Docker Compose performs automatic detection [1]. If the tool detects that it is not running in a terminal (i.e., dockerCli.Out().IsTerminal() is false), it will automatically disable TTY allocation [1]. The --no-tty and --no-TTY flags The --no-tty flag (with the shorthand -T) is used to explicitly disable pseudo-TTY allocation [2][3]. Regarding the variations --no-TTY and --no-tty: - Current versions of Docker Compose have normalized the flag to --no-tty [4]. - Historically, some versions and documentation variations used --no-TTY [5][4]. While recent updates have standardized this to the lowercase --no-tty to align with other commands like docker exec and improve consistency, many systems maintain support for both to avoid breaking existing scripts or workflows [4]. Summary of behavior: - Default: Auto-detected (allocates TTY if a terminal is present; disables it if not) [2][1]. - Explicit disable: Use --no-tty (or -T) to force-disable TTY allocation [2]. - Compatibility: Both --no-tty and --no-TTY generally function to achieve the same result in contemporary versions due to normalization efforts [4].
Citations:
- 1: https://github.com/docker/compose/blob/main/cmd/compose/run.go
- 2: https://docs.docker.com/reference/cli/docker/compose/run/
- 3: https://manpages.debian.org/trixie/docker-compose/docker-compose-run.1.en.html
- 4: fix(run): normalize --no-TTY flag to --no-tty docker/compose#13885
- 5: https://github.com/docker/compose/blob/17682c58db8986f1d1492559d72b3960b86f5f05/docs/reference/run.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== deploy.sh relevant lines =="
sed -n '1,230p' infrastructure/deployment/scripts/deploy.sh | awk '{printf "%6d\t%s\n", NR, $0}'
echo
echo "== compose files with openfga/model write services =="
rg -n "openfga|model-write|model_write|shell_escape|compose\\(" infrastructure apps compose.yaml docs/guidelines || true
echo
echo "== docker compose run flag docs in installed compose? =="
if command -v docker >/dev/null 2>&1; then
docker compose version 2>/dev/null || true
docker compose run --help 2>/dev/null | rg -i "tty|no-tty|no-TTY|detach" || true
else
echo "docker not available"
fiRepository: kl3inIT/OrgMemory
Length of output: 19207
Disable pseudo-TTY for the OpenFGA model-write capture.
docker compose run allocates a pseudo-TTY by default in interactive sessions, but model_write_json is parsed as JSON. Add the no-TTY flag to the capture command, e.g. openfga-model-write --no-tty, so the JSON remains valid when this rollout is run manually.
🤖 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 `@infrastructure/deployment/scripts/deploy.sh` around lines 190 - 205, The
OpenFGA model-write command captured in model_write_json must not allocate a
pseudo-TTY, since its output is parsed as JSON. Update the compose run
invocation for openfga-model-write to include the no-TTY option while preserving
the existing profile, cleanup, and dependency flags.
| if [[ "$openfga_model_sha256" != "$release_model_sha256" ]]; then | ||
| model_write_json="$( | ||
| "${compose[@]}" --profile ops run --rm --no-deps openfga-model-write | ||
| )" | ||
| new_openfga_model_id="$( | ||
| MODEL_WRITE_JSON="$model_write_json" python3 - <<'PY' | ||
| import json | ||
| import os | ||
|
|
||
| payload = json.loads(os.environ["MODEL_WRITE_JSON"]) | ||
| model_id = payload.get("authorization_model_id") or payload.get("id") | ||
| if not model_id: | ||
| raise SystemExit("OpenFGA CLI response did not contain an authorization model id") | ||
| print(model_id) | ||
| PY | ||
| )" | ||
| update_openfga_model_configuration \ | ||
| "$new_openfga_model_id" \ | ||
| "$release_model_sha256" | ||
| fi |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial
Model-write is not idempotent across mid-step failures.
If the script fails after the immutable model write succeeds but before update_openfga_model_configuration commits the new ID/digest (e.g. the JSON parse step), the digest in the env file stays stale. A subsequent retry with unchanged repository bytes will write another duplicate immutable model, since the stored digest still won't match. This is low-risk given OpenFGA models are cheap and immutable by design, but worth being aware of for stores that see repeated failed rollout attempts (model list will accumulate inert versions over time).
🤖 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 `@infrastructure/deployment/scripts/deploy.sh` around lines 190 - 209, Make the
model-write flow idempotent across failures between openfga-model-write and
update_openfga_model_configuration. Persist or recover the successfully created
model ID before retrying, and reuse it when the repository digest is unchanged
instead of creating another immutable model. Update the logic around
model_write_json, new_openfga_model_id, and update_openfga_model_configuration
while preserving the existing digest comparison.
1 participant
Root cause
Production retained the authorization model ID created during the first OpenFGA bootstrap. New model relations such as
can_manage_aiwere present in the repository but never written and pinned in the running release, so legitimate organization admins received 403 responses on Language Models and Index Settings.Fix
This does not bypass OpenFGA or infer authorization from the UI role.
Verification
fga model validate: validgit diff --check: passedSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests