fix(llmo-akamai): PATCH-delta deploy + fast plan + deploy-status reconciliation for large Akamai properties - #3013
fix(llmo-akamai): PATCH-delta deploy + fast plan + deploy-status reconciliation for large Akamai properties#3013ABHA61 wants to merge 27 commits into
Conversation
plan() drops the slow dry-run validation — it now only fetches the tree and merges in memory, returning the preview fast. PAPI's validateRules pass scales with total rule-tree size and on a large property runs 45-60s, past the ~15s CDN first-byte timeout in front of the Lambda, so the old dry-run made Review hang and then 503. deploy() remains the sole validator (its real PUT keeps validateRules=true), and the CUSTOM-SSL scope gate — the only common, pre-deploy-detectable failure — is still enforced in memory with no round-trip. Adds GET .../akamai/deploy-status: read-only, re-reads live Akamai state to report whether the managed Optimize-at-Edge rule landed in a version. This is the source of truth when a deploy's own HTTP response was lost to the CDN timeout — the Lambda keeps running (900s) and the write may complete after the browser gets a 503, so the caller polls this to learn the real outcome instead of showing a false failure or minting a duplicate version. deploy() also logs rule-tree complexity + measured write duration as telemetry (not a gate), so the real complexity-vs-latency curve builds from production. - new helpers estimateRuleTreeComplexity + detectManagedRuleNames (+ unit tests) - controller tests for deploy-status and the no-dry-run plan - OpenAPI spec + regenerated bundled docs Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…vert before merge) Depends on @adobe/spacecat-shared-akamai-client from a tarball built off the fix/akamai-client-rule-tree-timeout branch (PR #1864: 60s rule-tree timeout + quoted If-Match), hosted as a raw gist URL, so this branch can be deployed to dev and the fast-plan / deploy-status reconciliation flow tested against the real 60s client BEFORE #1864 publishes. Revert this commit and restore the published version once #1864 is released. NOTE: the gist tarball still pulls the published @adobe/spacecat-shared-utils (1.122.1), whose tracingFetch timeout-error message hardcodes "10000ms" — so a timeout will still LOG "10000ms" even though the abort now fires at 60s. That message bug is cosmetic (fixed separately in #1864's utils change) and the FE reconciliation does not parse it; judge the timeout by span duration instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every route in src/routes/index.js must be listed in both the FACS and the required-capabilities registries; the new GET .../akamai/deploy-status was missing, which failed the route-registration guard tests in CI. Add it next to the other read-only akamai onboarding routes (same LLMO-admin, x-akamai-* credential surface). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolves conflicts: - llmo-akamai.js: keep this branch's no-dry-run plan + deploy telemetry (complexity, putMs), and adopt main's new audit-log fields (severity, host) on the plan-ok / deploy papi-rejected / deployed lines. - package.json: keep this branch's pre-publish akamai-client gist tarball (TEMPORARY test dep), take main's newer athena-client 1.11.0. - package-lock.json + docs/index.html: regenerated from the merged state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
This PR will trigger a minor release when merged. |
getRouteHandlers's static/dynamic route segregation test asserts the exact set of registered routes; add GET .../akamai/deploy-status to the expected list and give the mock akamai controller a deployStatus stub, matching the new route. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…oy-status deploy-status's `deployed` flag alone is ambiguous on a re-onboard: a freshly-created version is a clone of its base, so if the base already carried a managed rule from a previous onboard, the clone shows the rule even if THIS deploy's write never persisted. Add an optional `baseVersion` query param — when supplied, deploy-status compares the per-deploy fetcher key (minted fresh every deploy) between the target version and the base clone and returns `freshWrite`: true = this deploy's write landed (keys differ), false = unwritten clone. The key value is compared server-side only, never returned. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The rule-tree PUT on a large property holds the connection open while Akamai runs its whole-tree validation before responding, and can exceed the shared client's 60s default. The browser is already released at the ~15s CDN cutoff and polling deploy-status, so the Lambda can wait longer — bump the rule-tree timeout to 120s (AKAMAI_RULE_TREE_TIMEOUT_MS overrides) so a slow-but-succeeding write isn't aborted before Akamai commits. api-service-only (passed at client construction), no shared-client release needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Akamai refuses to activate a version that has BLOCKING validation errors, returning a 400 on the activations endpoint (no activation is created, so the timeout-recovery probe finds nothing). Previously that fell through to a generic 502. Detect it and return a specific, actionable 422 (code version_has_validation_errors) telling the operator to fix / re-create the version first — this is the activation-time gate that stops a rule tree with errors from being pushed live. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… up versions deploy() ignored retryVersion and always called createVersion, so every retry minted a fresh version (478 → 479 → 480 …), each a potential empty orphan. Honor it: when the caller passes retryVersion (the version a prior attempt created), RESUME by writing directly into it and skip createVersion. Combined with the FE checking deploy-status first (resume the empty version, or report success if the rule already landed), a retry never creates another version. Validated as a positive integer; the 'deployed' audit line now carries `resumed`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolves conflicts in generated files only: - package.json auto-merged (kept the pre-publish akamai-client gist tarball, took main's newer dependency versions). - package-lock.json + docs/index.html: regenerated from the merged state (npm install + npm run docs:build). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The rule-tree PUT on a large property was still aborting at 120s before Akamai committed (observed ~124s aborts on the inflated test property). Give it a 300s budget (still well under the 900s Lambda cap) so a slow-but-succeeding write — especially a large FIRST onboard where the version only shows the rule once the write lands — isn't cut off. Env-overridable via AKAMAI_RULE_TREE_TIMEOUT_MS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…errors Add an opt-in validate flag to deploy-status: when set, it runs PAPI validation on the checked version (getRuleTree validateRules) and returns activatable, errorCount, a bounded errors array, and warningCount — so the Activate step can disable activation and show the errors up front instead of after a rejected attempt. Off by default (the reconciliation poll keeps the cheap presence read). Also refreshes the pre-publish akamai-client gist (now carries getRuleTree validateRules). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…status' into fix/akamai-oae-fast-plan-deploy-status
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The deploy write PUT the entire merged rule tree (~324 KB on a large property) as its request body. Proven this session on the live test property: the byte-identical PUT completes in ~31s direct but exceeds 300s from the api-service Lambda and aborts at the timeout — so a new version gets created (clone of base) but never receives the OAE rule. The bottleneck is the 324 KB egress, not Akamai's validation (which is server-side, size-independent, and a constant ~31s across 3 samples). Switch deploy() to patchRuleTree: build the OAE delta with the existing buildRuleTreePatch and apply it to the freshly-cloned version. Same validateRules pass, byte-identical committed result, but a ~4 KB body (~74x smaller). Direct replication: PATCH lands OAE with 0 errors in ~32s. No new gist needed — patchRuleTree ships in the pinned 1.2.0. Also raise the deploy rule-tree timeout ceiling 300s → 600s as a safety net (AKAMAI_RULE_TREE_TIMEOUT_MS-overridable); with PATCH the write should stay ~30-90s, so this ceiling should not normally be hit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 600s ceiling was set to test whether the old 324 KB full-tree PUT would finish given more time. The switch to a ~4 KB PATCH delta made that moot: the write now measures ~30s. 300s keeps ~10x headroom over that (and covers a max-size property, whose validation would still be well under it) while lining up with the FE's ~300s deploy-status poll window and failing a genuinely stuck deploy in 5 min instead of 10. Still env-overridable via AKAMAI_RULE_TREE_TIMEOUT_MS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lan-deploy-status # Conflicts: # docs/index.html
MysticatBot
left a comment
There was a problem hiding this comment.
Hey @ABHA61,
Verdict: Request changes - one blocking item remains (gist tarball swap before merge); one new defense-in-depth gap to fix.
Complexity: HIGH - large diff; API surface + dependency + FACS routing changes.
Changes: Replaces the full-tree PUT deploy with a JSON-Patch delta, drops the slow plan dry-run, adds a deploy-status polling endpoint with retry-resume support, and gates activation on PAPI validation errors (13 files).
Note: Recommend a human read before merge - this change modifies shared API contracts (OpenAPI specs). The bot review is a complement to, not a replacement for, a human read here.
Must fix before merge
- [Important] deploy-status validation errors returned to client without secret redaction -
src/controllers/llmo/llmo-akamai.js:895(details inline) - [Important] Gist tarball dependency must be swapped to published npm version before merge -
package.json:92(details inline)
Non-blocking (4): minor issues and suggestions
- nit:
redactPapiErrorscalled with emptyextraSecretsin activation 400 handler whenapiKey/fetcherKeymay be in scope -src/controllers/llmo/llmo-akamai.js:1082 - nit: Missing boundary test for
retryVersion == baseVersion(the exact equality case of the guard) -test/controllers/llmo/llmo-akamai.test.js:390 - suggestion: Activation gate test should assert on
body.papiErrorscontent to verify regex extraction + redaction work end-to-end -test/controllers/llmo/llmo-akamai.test.js:806 - suggestion: Add a
redactPapiErrorstest with a plain object input to document the JSON round-trip path behavior -test/controllers/llmo/llmo-akamai-utils.test.js
Previously flagged, now resolved
- retryVersion guard: now rejects
retryVersion <= baseVersionwith an actionable 400. - PAPI error redaction:
redactPapiErrorsutility scrubs secrets from deploy 422 and activation 422 response paths.
MysticatBot
left a comment
There was a problem hiding this comment.
Hey @ABHA61,
Verdict: Request changes - one blocking item remains (gist tarball swap before merge); one new defense-in-depth gap to fix.
Complexity: HIGH - large diff; API surface + dependency + FACS routing changes.
Changes: Replaces the full-tree PUT deploy with a JSON-Patch delta, drops the slow plan dry-run, adds a deploy-status polling endpoint with retry-resume support, and gates activation on PAPI validation errors (13 files).
Note: Recommend a human read before merge - this change modifies shared API contracts (OpenAPI specs). The bot review is a complement to, not a replacement for, a human read here.
Must fix before merge
- [Important] deploy-status validation errors returned to client without secret redaction -
src/controllers/llmo/llmo-akamai.js:895(details inline) - [Important] Gist tarball dependency must be swapped to published npm version before merge -
package.json:92(details inline)
Non-blocking (4): minor issues and suggestions
- nit:
redactPapiErrorscalled with emptyextraSecretsin activation 400 handler whenapiKey/fetcherKeymay be in scope -src/controllers/llmo/llmo-akamai.js:1082 - nit: Missing boundary test for
retryVersion == baseVersion(the exact equality case of the guard) -test/controllers/llmo/llmo-akamai.test.js:390 - suggestion: Activation gate test should assert on
body.papiErrorscontent to verify regex extraction + redaction work end-to-end -test/controllers/llmo/llmo-akamai.test.js:806 - suggestion: Add a
redactPapiErrorstest with a plain object input to document the JSON round-trip path behavior -test/controllers/llmo/llmo-akamai-utils.test.js
Previously flagged, now resolved
- retryVersion guard: now rejects
retryVersion <= baseVersionwith an actionable 400. - PAPI error redaction:
redactPapiErrorsutility scrubs secrets from deploy 422 and activation 422 response paths.
| // the injected secret header values, so they're safe to surface. | ||
| ...(validate ? { | ||
| activatable, | ||
| errorCount, |
There was a problem hiding this comment.
issue (blocking): The errors array from PAPI validation is returned to the client unredacted. While the code comment says these describe rules by name/path, PAPI can echo back behavior option values (e.g. headerValue entries) in structured detail fields when a specific behavior config triggers a validation error. Since the checked version already contains the real API key and minted fetcher key baked into modifyIncomingRequestHeader behaviors, a validation error referencing that behavior could surface those values.
The redactPapiErrors utility already exists and is used at the two other response sites (deploy 422, activation 422). Applying it here is a one-line fix for defense-in-depth consistency:
errors: redactPapiErrors((errors ?? []).slice(0, 25), []),Pass [] for extraSecrets since no explicit secrets are in local scope here (deploy-status only reads); the header-name regex and 64-hex pattern still catch the primary leakage shapes.
There was a problem hiding this comment.
Good catch — you're right, this was the one response site I'd missed. Fixed in 311b62b:
errors: redactPapiErrors((errors ?? []).slice(0, 25), []),Applied exactly as suggested ([] for extraSecrets — deploy-status only reads, no secrets in local scope; the header-name regex + 64-hex pattern still catch the leak shapes). Also corrected the stale "safe to surface" comment above it and added a test asserting a fetcher key echoed in an error detail is redacted in the response.
| "@adobe/mysticat-shared-seo-client": "1.8.0", | ||
| "@adobe/spacecat-helix-content-sdk": "1.4.33", | ||
| "@adobe/spacecat-shared-akamai-client": "1.2.0", | ||
| "@adobe/spacecat-shared-akamai-client": "https://gist.githubusercontent.com/ABHA61/844cff6ac7045380040b26b7eb6a4dbe/raw/adobe-spacecat-shared-akamai-client-1.2.0.tgz", |
There was a problem hiding this comment.
issue (blocking): The gist tarball dependency (@adobe/spacecat-shared-akamai-client) bypasses npm registry integrity checks and audit. The PR body acknowledges this as a merge-order constraint: merge adobe/spacecat-shared#1864 first, then swap to the published version. Flagging to confirm this swap happens before this PR merges to main.
There was a problem hiding this comment.
Confirmed — this is a deliberate, temporary pre-merge constraint and the swap will happen before this PR merges to main. The order is:
- Merge fix(akamai-client): configurable rule-tree timeout + patchRuleTree + accurate timeout message spacecat-shared#1864 and let it publish
@adobe/spacecat-shared-akamai-client. - Swap this
package.jsondependency from the gist tarball to that published version (+ refreshpackage-lock.json). - Only then merge this PR.
The gist tarball exists solely so the branch could be deployed to dev for end-to-end validation ahead of #1864's release; it will not reach main. This is tracked as a blocking merge-order item on our side.
|
Mysticat review failed: Claude CLI crashed (exit 1): stderr= stdout={"type":"result","subtype":"error_max_budget_usd","duration_ms":9,"duration_api_ms":1396504,"is_error":true,"num_turns":1,"stop_reason":null,"session_id":"a037bcd1-0a15-4f9d-aed4-e8d08326c620","total_cost_usd":10.074207000000003,"usage":{"input_tokens":0,"cache_creation_input_tokens":0,"cache_read_i |
The deploy-status endpoint (validate=true) returned the PAPI errors array unredacted. PAPI can echo a behavior's option values (e.g. headerValue) in a validation error's structured detail, so a rejection referencing the managed rule could surface the baked-in LLMO API key / minted fetcher key. Apply the same redactPapiErrors already used at the deploy-422 and activate-422 sites, and correct the stale "safe to surface" comment. Adds a redaction test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Hey @ABHA61,
Verdict: Request changes - one blocking item remains (gist tarball swap before merge).
Complexity: HIGH - large diff; API surface + dependency + FACS routing changes.
Changes: Replaces the full-tree PUT deploy with a JSON-Patch delta, drops the slow plan dry-run, adds a deploy-status polling endpoint with retry-resume support, and gates activation on PAPI validation errors (13 files).
Note: Recommend a human read before merge - this change modifies shared API contracts (OpenAPI specs). The bot review is a complement to, not a replacement for, a human read here.
Must fix before merge
- [Important] Gist tarball dependency must be swapped to published npm version before merge -
package.json:92(details inline)
Non-blocking (4): minor issues and suggestions
- nit: OpenAPI
errorsfield description says secrets are "never included" but PAPI can echo them and the server redacts them; the spec should say "redacted server-side" to prevent future developers skipping redaction -docs/openapi/llmo-api.yaml - nit:
redactPapiErrorscalled with emptyextraSecretsin activation 400 handler whenapiKey/fetcherKeyare in scope at that call site -src/controllers/llmo/llmo-akamai.js:1082 - suggestion: Add a description to the plan response
validatedfield in the OpenAPI spec explaining validation now occurs only at deploy time -docs/openapi/llmo-api.yaml - suggestion: Missing boundary test for
retryVersion == baseVersion(the exact equality case of the<=guard) -test/controllers/llmo/llmo-akamai.test.js:390
Previously flagged, now resolved
- deploy-status PAPI errors now redacted via
redactPapiErrors(commit 311b62b). - retryVersion guard rejects
retryVersion <= baseVersionwith an actionable 400. - PAPI error redaction utility scrubs secrets from deploy 422 and activation 422 response paths.
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 32s | Cost: $9.25 | Commit: 311b62bd0a677266696cf2a9f4a836eff93d0daa
If this code review was useful, please react with 👍. Otherwise, react with 👎.
| "@adobe/mysticat-shared-seo-client": "1.8.0", | ||
| "@adobe/spacecat-helix-content-sdk": "1.4.33", | ||
| "@adobe/spacecat-shared-akamai-client": "1.2.0", | ||
| "@adobe/spacecat-shared-akamai-client": "https://gist.githubusercontent.com/ABHA61/844cff6ac7045380040b26b7eb6a4dbe/raw/adobe-spacecat-shared-akamai-client-1.2.0.tgz", |
There was a problem hiding this comment.
issue (blocking): The gist tarball dependency (@adobe/spacecat-shared-akamai-client) bypasses npm registry integrity checks and audit. The PR body and your inline response both acknowledge this as a merge-order constraint: merge adobe/spacecat-shared#1864 first, then swap to the published version. Re-raising because the code has not yet changed and the PR cannot merge with this in place. Once the published version is available, swap and update package-lock.json.
There was a problem hiding this comment.
Done — the gist tarball is swapped for the published registry version. adobe/spacecat-shared#1864 merged and released @adobe/spacecat-shared-akamai-client@1.2.1, so package.json now pins "@adobe/spacecat-shared-akamai-client": "1.2.1" and package-lock.json resolves it from registry.npmjs.org with an integrity hash (no more gist URL anywhere in the tree). 1.2.1 contains the same API this branch uses (patchRuleTree, ruleTreeTimeoutMs, getRuleTree({ validateRules })); 168 controller/utils/routes tests pass against it. This was the last blocking merge-order item.
| * site's own domain. Idempotent by rule name (trimmed): re-running replaces prior managed rules | ||
| * rather than duplicating them. | ||
| * | ||
| * `retryVersion`: if a PRIOR attempt already created a property version but failed to write the |
There was a problem hiding this comment.
once we save a chnage in property and a new version is created, I dont thik there's a way to reuse the same version to perform additional update. it should be frozen against a set of changes. can you pls confirm?
There was a problem hiding this comment.
This i have observed that the version cannot reuse when changing through the console but through PAPI we can patch the same version's rules multiple times. A version is only frozen once it's activated.
…accurate timeout message (#1864) ## What `AkamaiClient` gains the pieces the LLMO Akamai onboarding needs for large properties, plus a latent message-bug fix in `spacecat-shared-utils`: - **Configurable `ruleTreeTimeoutMs`** (default `60000`), applied only to the three calls whose latency scales with rule-tree size — `getRuleTree`, `updateRuleTree`, `patchRuleTree`. Cheap metadata calls (`getLatestVersion`, `createVersion`, `activate`, …) keep tracingFetch's 10s default. - **New `patchRuleTree`** — applies a JSON Patch (RFC 6902) to a version's stored tree with `validateRules=true`. `If-Match`, when supplied, is sent as an RFC 7232 quoted etag (PAPI rejects a bare token with a generic 400). - **`getRuleTree` `options.validateRules`** — opt-in full validation pass that returns `errors`/`warnings` (used to tell whether a version is activatable). - **`index.d.ts`** updated for all of the above. - **tracing-fetch message fix** — the timeout error always reported `"10000ms"` regardless of the configured timeout (it read a non-existent `_ms` off `@adobe/fetch`'s signal). The abort always fired at the right time; only the message was wrong. The real value is now threaded through. ## Why On a large Akamai property, PAPI's `validateRules` pass over the whole tree genuinely takes longer than tracingFetch's 10s default, so the rule-tree write was aborted client-side before Akamai could respond — the version got created (a cheap call) but its rule content never landed. ## Testing - `akamai-client.test.js`: the three rule-tree calls use the configured `ruleTreeTimeoutMs` (not the 10s default); cheap calls unaffected; `patchRuleTree` content-type / quoted If-Match / validateRules; `getRuleTree` validateRules returns errors/warnings. - `tracing-fetch.test.js`: asserts the timeout message reports the configured value. - `npm test` + `npm run lint` clean in `spacecat-shared-akamai-client` and `spacecat-shared-utils`. ## Merge order **Merge this first.** adobe/spacecat-api-service#3013 consumes the published release of this package (its PATCH deploy uses `patchRuleTree`; its deploy-status uses `getRuleTree({ validateRules })`). Once this releases, adobe/spacecat-api-service#3013 swaps its temporary gist-tarball dependency for the published version. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Akash Bhardwaj <akbhardwaj@Akashs-MacBook-Pro.local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
## [@adobe/spacecat-shared-akamai-client-v1.2.1](https://github.com/adobe/spacecat-shared/compare/@adobe/spacecat-shared-akamai-client-v1.2.0...@adobe/spacecat-shared-akamai-client-v1.2.1) (2026-08-12) ### Bug Fixes * **akamai-client:** configurable rule-tree timeout + patchRuleTree + accurate timeout message ([#1864](#1864)) ([2aec2be](2aec2be)), closes [adobe/spacecat-api-service#3013](adobe/spacecat-api-service#3013) [adobe/spacecat-api-service#3013](adobe/spacecat-api-service#3013)
## [@adobe/spacecat-shared-utils-v1.125.2](https://github.com/adobe/spacecat-shared/compare/@adobe/spacecat-shared-utils-v1.125.1...@adobe/spacecat-shared-utils-v1.125.2) (2026-08-12) ### Bug Fixes * **akamai-client:** configurable rule-tree timeout + patchRuleTree + accurate timeout message ([#1864](#1864)) ([2aec2be](2aec2be)), closes [adobe/spacecat-api-service#3013](adobe/spacecat-api-service#3013) [adobe/spacecat-api-service#3013](adobe/spacecat-api-service#3013)
Swap the temporary pre-publish gist tarball for the registry version now that adobe/spacecat-shared#1864 is merged and released. 1.2.1 contains the same API this branch depends on (patchRuleTree, ruleTreeTimeoutMs, getRuleTree validateRules). Clears the last blocking merge-order item. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lan-deploy-status # Conflicts: # docs/index.html
|
Mysticat review failed: Claude CLI crashed (exit 1): stderr= stdout={"type":"result","subtype":"error_max_budget_usd","duration_ms":4,"duration_api_ms":1390058,"is_error":true,"num_turns":1,"stop_reason":null,"session_id":"9296ee3a-c239-49ba-91c6-979a3df70dd6","total_cost_usd":10.22122415,"usage":{"input_tokens":0,"cache_creation_input_tokens":0,"cache_read_input_to |
There was a problem hiding this comment.
Hey @ABHA61,
Verdict: Approve - all prior blockers resolved; dependency swap and merge from main are clean.
Complexity: HIGH - large diff; API surface + dependency + FACS routing changes.
Changes: Replaces the full-tree PUT deploy with a JSON-Patch delta, drops the slow plan dry-run, adds a deploy-status polling endpoint with retry-resume support, and gates activation on PAPI validation errors (13 files).
Note: Recommend a human read before merge - this change modifies shared API contracts (OpenAPI specs). The bot review is a complement to, not a replacement for, a human read here.
Non-blocking (1): minor issues and suggestions
- nit: deploy-status OpenAPI security scheme uses
ims_keybut every other endpoint in llmo-api.yaml usessession_token-docs/openapi/llmo-api.yaml:6672
Previously flagged, now resolved
- Gist tarball dependency swapped to published
@adobe/spacecat-shared-akamai-client@1.2.1(commit 573cebb). - deploy-status PAPI errors redacted via
redactPapiErrors(commit 311b62b). - retryVersion guard rejects
retryVersion <= baseVersionwith an actionable 400. - PAPI error redaction utility scrubs secrets from deploy 422 and activation 422 response paths.
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 49s | Cost: $4.80 | Commit: e1e984d37221dfee1905f2c4a5776cbc5e785201
If this code review was useful, please react with 👍. Otherwise, react with 👎.
Problem
Large Akamai properties need a slow rule-tree write, but there's a ~15s CDN first-byte timeout in front of the API Lambda. OAE onboarding hung and then failed with an opaque 503, and retries minted duplicate empty property versions (prod incident LLMO-6799).
Root cause (proven this cycle)
A deploy did
createVersion(clone) thenupdateRuleTree— a full-tree PUT whose body is the entire rule tree (~324 KB+ on a large property). That upload — not Akamai'svalidateRules(server-side, ~constant) — is what made the write exceed 300s from the Lambda: the byte-identical PUT runs ~31s from a direct connection but >300s from the Lambda (VPC egress).Approach
patchRuleTreesends only the OAE ops (~4 KB) for Akamai to apply to the freshly-cloned version under the samevalidateRules— a byte-identical committed result with a ~74× smaller body. Removes the egress bottleneck.plan()drops the slow dry-run — fetch + in-memory merge only (~4s); the CUSTOM-SSL scope gate is still enforced in memory.deploy()is the sole validator.GET …/akamai/deploy-status— read-only reconcile that re-reads live Akamai state (deployed,freshWrite, and withvalidate=trueactivatable/errorCount). The FE polls it when a deploy's own response is lost to the CDN timeout, instead of showing a false failure.deployacceptsretryVersionto resume writing into an already-created version instead of minting a new one on every retry. Guarded soretryVersionmust be greater thanbaseVersion(a resumed version is always a clone of the base, so it can never be ≤ it). Confirmed against live PAPI that a non-activated version is freely re-editable — deploy never activates, so the resumed version is always writable.AKAMAI_RULE_TREE_TIMEOUT_MS); with PATCH the write is ~30–65s, so this is just headroom.putMs.Secret redaction (review hardening)
PAPI can echo the rules we sent back inside its validation errors — including the injected LLMO API key / minted fetcher-key header values.
redactPapiErrorsscrubs those (explicit secret values + secret-header-name pattern + 32-byte-hex minted-key pattern) at all three response sites: deploy422, activate422, and the deploy-statuserrorsarray.Validated end-to-end (dev)
Onboarded synthetic properties at complexity 600 / 1298 / 2498 through the wizard: PATCH deploy,
putMs30–65s,deployed:true,freshWrite:true, OAE landed — default and custom rule placement.Dependency / merge order — resolved
adobe/spacecat-shared#1864 is merged and released as
@adobe/spacecat-shared-akamai-client@1.2.1. This branch now pins that published registry version (the earlier pre-publish gist tarball has been removed;package-lock.jsonresolves 1.2.1 fromregistry.npmjs.orgwith an integrity hash). No remaining merge-order constraint.Tests
estimateRuleTreeComplexity/detectManagedRuleNames/buildRuleTreePatch/redactPapiErrorsunit-tested; controller tests for PATCH deploy, fast plan, deploy-status (incl. secret redaction), retry-resume, activation gate. Controller/utils/routes suites pass; lint + docs:lint clean.Supersedes #3000. Pairs with the FE (adobe/project-elmo-ui#2729) and the shared client (adobe/spacecat-shared#1864). LLMO-6799.
🤖 Generated with Claude Code