feat(user-manager-client): per-workspace AI resource accounting + /resources reads (LLMO-5616)#1767
Conversation
…sources reads (LLMO-5616)
Replace the parent-only "free pool" quota model in the UM mock with the live Semrush
`{ used, drafted, total }` model, and wire the two `NewWorkspaceResources` read endpoints —
the foundation the dynamic (just-in-time) resource-allocation feature reads/meters against.
Model (live-verified 2026-07-02 against adobe-hackathon.semrush.com; "Gate 0"):
- Per-workspace, per-product `{ used, drafted, total }` for `ai.{projects,prompts}` (+ daily-only
`weekly_prompts`). `free = total - used`.
- A carve (child create, or a transfer that RAISES a child's total) moves units off the MASTER's
`total`; a release (transfer that LOWERS it) returns them. Transfers are ABSOLUTE and idempotent;
over-carving the master -> 422 "insufficient available units in subscription".
- No resources record => unmetered (unlimited), so existing seeds/flows are unaffected.
- quota.js: rewritten (resources/set/canCarve/moveFromMaster/applyTransfer); collection renamed
workspace_pool -> workspace_resources.
- factories.js: add createWorkspaceResourcesMock (typed NewWorkspaceResources).
- routes: new GET /v1/workspaces/{id}/resources and /parent/resources (the latter mirrors live —
it returns the workspace's OWN allocation, not the master pool); child/transfer/__quota updated
to the absolute {used,drafted,total} model.
- seeds.js: buildSeed resources + new parent-with-child-metered seed (shared default untouched).
- Tests: quota/seeds/factories/stateful rewritten (100% branch coverage); +4 e2e for reads,
absolute transfer carve/release, and the over-draw 422.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Hey @byteclimber,
⚠ Degraded review - no spec document was found for this change (searched the PR links, the touched repos' docs, the architecture/guidelines docs, and linked Jira). This review covers code-level quality but could not validate the change against an agreed design, so confidence is reduced. Add a spec link (PR template section 4) and re-request review for a full-confidence pass.
Verdict: Request changes - two documentation files are now actively incorrect after the API rename.
Complexity: HIGH - large diff; API surface change.
Changes: Replaces the parent-pool quota model with per-workspace { used, drafted, total } AI resource accounting, adds GET /resources and /parent/resources read endpoints, and rewrites tests to match (14 files).
Note: CI check "Test" is still pending - confirm it passes before merge.
Must fix before merge
- [Important] Package CLAUDE.md references obsolete
buildSeed({ pools })API and "parent-pool quota metering" terminology -packages/spacecat-shared-user-manager-client/CLAUDE.md(details inline) - [Important]
docs/mock-usage.mdreferences obsoletebuildSeed({ pools }), old__quotasemantics, and stale section title "Parent-pool metering" -packages/spacecat-shared-user-manager-client/docs/mock-usage.md(details inline)
Non-blocking (6): minor issues and suggestions
- nit:
child.jscreates workspace before confirmingapplyTransferresult - if the internalcanCarveinvariant ever diverges, an orphan workspace with no allocation is created and never rolled back -mock/counterfact/routes/v2/workspaces/{id}/child.js:53-59 - suggestion: Add a test proving
draftedis intentionally excluded fromcanCarve(the PR saysfree = total - usedwas live-verified, but no test exercisescanCarvewithdrafted > 0to document this design choice) -test/mock/quota.test.js - nit:
/resources.jsand/parent/resources.jshave identical logic - factor into a shared helper to prevent divergence when the model evolves -mock/counterfact/routes/v1/workspaces/{id}/resources.js - suggestion: Document that an empty transfer body on an unmetered child whose parent IS metered silently creates a zeroed resources record on the child (making it metered) -
mock/quota.js:applyTransfer - suggestion: Add an E2E test for
GET /resourceson an unmetered workspace - the current behavior returns zeroed defaults (indistinguishable from zero-allocation), worth documenting with a test -test/e2e/user-manager-mock.e2e.js - nit: Once metered via
POST /__quota, there is no mechanism to return a workspace to unmetered (the oldnullsemantics are gone) - tests must use__reset/__seed- worth a one-line doc comment on the route -mock/counterfact/routes/__quota.js
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 4m 23s | Cost: $6.86 | Commit: 09f2b38347df5322882b47b37ea9717ea1037707
If this code review was useful, please react with 👍. Otherwise, react with 👎.
…e + add coverage (LLMO-5616) Address MysticatBot review on #1767: - Blocking: update CLAUDE.md and docs/mock-usage.md — obsolete `buildSeed({ pools })`, `workspace_pool`, and "parent-pool metering" terminology replaced with the per-workspace `{ used, drafted, total }` model, `buildSeed({ resources })`, and the carve/release semantics; note there is no path back to unmetered (use __reset/__seed). - Test: prove `canCarve` ignores `drafted` (free = total − used), documenting the design choice. - Test: E2E for GET /resources on an unmetered workspace (zeroed defaults). - Doc: note on `applyTransfer` that a transfer onto an unmetered child makes it metered. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
This PR will trigger a minor release when merged. |
|
Thanks @MysticatBot — addressed in Spec link added (clears the degraded-review note): the design + Gate 0 live-verification is serenity-docs#22. This PR is PR 1 (the mock foundation) of that plan. Blocking — both fixed:
Non-blocking:
Gates green locally: lint, |
There was a problem hiding this comment.
Hey @byteclimber,
Verdict: Request changes - one stale catalog entry in the root CLAUDE.md introduced by this rename.
Complexity: HIGH - large diff; API surface change.
Changes: Replaces the parent-pool quota model with per-workspace { used, drafted, total } AI resource accounting, adds GET /resources and /parent/resources read endpoints, and rewrites tests to match (16 files).
Note: Recommend a human read before merge - this change modifies an architectural document (packages/spacecat-shared-user-manager-client/CLAUDE.md). The bot review is a complement to, not a replacement for, a human read here.
Note: CI checks "Test" and "E2E" are still pending - confirm they pass before merge.
Must fix before merge
- [Important] Root CLAUDE.md catalog entry still references "parent-pool quota metering" - now stale after this PR renamed to "per-workspace AI resource accounting" -
CLAUDE.md(details below)
Root CLAUDE.md catalog entry (not in the diff, so detailed here):
The root /CLAUDE.md Package-Level CLAUDE.md Files section contains:
- [`packages/spacecat-shared-user-manager-client/CLAUDE.md`](...) — mandatory `// @ts-check`, the Counterfact mock + factory-pattern fixtures, parent-pool quota metering + bearer auth, the spec overlay + freshness gate
This PR updated the package's own CLAUDE.md to say "per-workspace AI resource accounting" throughout, but the root catalog entry still says "parent-pool quota metering". A reader following the root index would find stale terminology. Fix: update the root catalog entry to match:
— mandatory `// @ts-check`, the Counterfact mock + factory-pattern fixtures, per-workspace AI resource accounting + bearer auth, the spec overlay + freshness gate
Non-blocking (2): minor issues and suggestions
- suggestion: Consider returning a richer result from
applyTransfer(e.g.{ ok, master, child }) so callers wanting the updated state don't need a secondresources()call -mock/quota.js:applyTransfer - suggestion:
createWorkspaceResourcesMockcould validateused <= totalto catch nonsensical seed data early during debugging -mock/factories.js:createWorkspaceResourcesMock
Previously flagged, now resolved
- Package CLAUDE.md updated: "parent-pool quota metering" replaced with "per-workspace AI resource accounting",
buildSeed({ pools })replaced withbuildSeed({ resources }) docs/mock-usage.mdupdated: section title,buildSeedblurb,__quota/__dumpcontrol rows, transfer semantics, and source-map references all aligned to the new model
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 54s | Cost: $6.91 | Commit: 5d66229420cb1e944765f92b86ebeaaa3dbc0a3e
If this code review was useful, please react with 👍. Otherwise, react with 👎.
… rename (LLMO-5616) Root /CLAUDE.md package index still said 'parent-pool quota metering'; match the package doc's 'per-workspace AI resource accounting'. Addresses MysticatBot on #1767. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Fixed in Non-blocking suggestions — deferred (happy to take either if you'd like):
CI "Test"/"E2E" pending; green locally (170 unit @ 100% branches, 19 e2e). |
There was a problem hiding this comment.
Hey @byteclimber,
Verdict: Approve - prior blocking finding resolved, no new issues.
Complexity: HIGH - large diff; API surface change.
Changes: Replaces the parent-pool quota model with per-workspace { used, drafted, total } AI resource accounting, adds GET /resources and /parent/resources read endpoints, and rewrites tests to match (17 files).
Note: Recommend a human read before merge - this change modifies an architectural document (CLAUDE.md). The bot review is a complement to, not a replacement for, a human read here.
Note: CI checks "Test" and "E2E" are still pending - confirm they pass before merge.
Previously flagged, now resolved
- Root CLAUDE.md catalog entry updated from "parent-pool quota metering" to "per-workspace AI resource accounting" (commit ce2019e)
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 0m 37s | Cost: $2.81 | Commit: ce2019e47ed8c5fc8422e5294c9687091305b148
If this code review was useful, please react with 👍. Otherwise, react with 👎.
rainer-friederich
left a comment
There was a problem hiding this comment.
Approving — this is the User-Manager-mock foundation (PR 1) for dynamic Semrush AI allocation, per the design in https://github.com/adobe/serenity-docs/pull/22 . Reviewed as part of a cross-PR review together with spacecat-api-service PR adobe/spacecat-api-service#2764 .
Scope is test-fixture only (the user-manager-client mock) — no prod runtime impact on the real client. Confirmed against what the allocator actually reads/writes:
product_resources.ai.resources.<dim>.{used, drafted, total}shape matchesreadAiTotals.- transfer is absolute + idempotent, moves the delta to/from the parent, and 422s on over-carve — matches
transferOnce/transferAndSettleand theisPoolExhaustedclassifier. - the
POST /__quotaset-hook + thePARENT_WITH_CHILD_METEREDseed give api-service everything it needs to write the ON-path allocator integration tests without a Project-Engine-mock metering change.
Safe to merge.
…on (LLMO-5616)
Comment-only (no behavior change), mirroring serenity-docs#22 (Rainer 2026-07-08): the mock
deliberately does NOT implement a cross-mock closed loop (a Project-Engine publish decrementing
this User-Manager mock's used) — the two containers share no state; gluing them was evaluated and
rejected. Tests set {used,drafted,total} directly via the set hook / seeds; the live-publish-
consumes-quota question is the live-gateway canary's job. Noted at quota.js + the metered seed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Hey @byteclimber,
Verdict: Approve - prior blocking findings resolved, scope decision well-documented, no new issues.
Complexity: HIGH - large diff; API surface change.
Changes: Documents the no-cross-mock-loop scope decision in quota.js and seeds.js comments, merges from main (17 files).
Note: Recommend a human read before merge - this change modifies an architectural document (CLAUDE.md). The bot review is a complement to, not a replacement for, a human read here.
Note: CI checks "Test" and "E2E" are still pending - confirm they pass before merge.
Previously flagged, now resolved
- Package CLAUDE.md updated to "per-workspace AI resource accounting" terminology and
buildSeed({ resources })API docs/mock-usage.mdaligned to the new model (section title, buildSeed blurb, control rows, transfer semantics)- Root CLAUDE.md catalog entry updated from "parent-pool quota metering" to "per-workspace AI resource accounting"
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 0m 50s | Cost: $2.52 | Commit: 4f6d57aff39c51c177f3194b2e17b90f10f51421
If this code review was useful, please react with 👍. Otherwise, react with 👎.
## [@adobe/spacecat-shared-user-manager-client-v1.5.0](https://github.com/adobe/spacecat-shared/compare/@adobe/spacecat-shared-user-manager-client-v1.4.0...@adobe/spacecat-shared-user-manager-client-v1.5.0) (2026-07-13) ### Features * **user-manager-client:** per-workspace AI resource accounting + /resources reads (LLMO-5616) ([#1767](#1767)) ([436efa8](436efa8)), closes [serenity-docs#22](https://github.com/adobe/serenity-docs/issues/22)
|
🎉 This PR is included in version @adobe/spacecat-shared-user-manager-client-v1.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
…on (LLMO-5616) Addresses Rainer's review item #4 — integration test against the dynamic-allocation feature flag ON, using the existing Semrush mocks (now on the released 1.5.0 metered User Manager mock). - Bump @adobe/spacecat-shared-user-manager-client 1.3.1 → 1.5.0 (adobe/spacecat-shared#1767 merged + released) so the IT pulls the mock image with per-workspace AI resource accounting (SERENITY_UM_MOCK_TAG follows the installed client version). - Give ORG_1 a Semrush parent workspace (SERENITY_ORG_PARENT_WS_ID) so BRAND_1's sub-workspace resolves a non-null parentWorkspaceId — the condition the JIT guard needs to engage — pointed at an existing UM-mock workspace so the advisory pool read resolves. - Add setUmMockQuota / dumpUmMock harness helpers (the mock's __quota / __dump control routes), threaded into the shared serenity test factory. - New IT: with the global SERENITY_DYNAMIC_ALLOCATION env ON and the child seeded at a zero resource total, a real create-market fronts project headroom and tops the child up from 0 via a live /resources transfer to the mock. Asserting the total grew is a positive over-the-wire proof that the flag-ON JIT engaged (a flag-OFF run never transfers) and that the global kill-switch toggle takes effect per request. The prompt write path runs as a smoke; prompt texts×models sizing stays unit-test-covered. Verified locally against the live container stack: serenity IT 36 passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Spec / design: serenity-docs#22 — Dynamic Resource Allocation design (
docs/discovery/brand-semrush-dynamic-resource-allocation.md), live-verified in its Gate 0. This PR is PR 1 (the mock foundation) of that plan.What
Replaces the UM mock's parent-only "free pool" quota model with the live Semrush per-workspace
{ used, drafted, total }model, and wires the twoNewWorkspaceResourcesread endpoints(
GET /v1/workspaces/{id}/resourcesand/parent/resources). This is PR 1 (the foundation) forthe dynamic just-in-time Semrush AI resource-allocation work — the surface
ensureAiHeadroom/releaseAiSurplusinspacecat-api-servicewill read and meter against.Why
Provisioning carves a flat AI allocation up front; the new design tops up just-in-time by reading a
sub-workspace's real headroom and transferring only the delta from the parent. The mock had no
per-workspace resource state and no
/resourcesreads, so none of that could be exercised.Model (live-verified 2026-07-02 against
adobe-hackathon.semrush.com— "Gate 0"){ used, drafted, total }forai.{projects,prompts}(+ daily-onlyweekly_prompts);free = total − used.total) moves units off themaster's
total; a release (transfer that LOWERS it) returns them. Verified: carving a(3,300)child moved the master100→97 / 19920→19620.422 "insufficient available units in subscription"./parent/resourcesmirrors live behaviour: it returns the workspace's own allocation, notthe master pool (a caller wanting the org pool reads the master's own
/resources).Changes
mock/quota.js— rewritten (resources/set/canCarve/moveFromMaster/applyTransfer);collection
workspace_pool→workspace_resources.mock/factories.js—createWorkspaceResourcesMock(typedNewWorkspaceResources).GET .../resources+.../parent/resources;child/transfer/__quotaupdated to the absolute
{used,drafted,total}model.mock/seeds.js—buildSeedresources + newparent-with-child-meteredseed (shared defaultleft untouched).
quota/seeds/factories/statefulrewritten (100% branch coverage) + 4 e2e forthe reads, absolute-transfer carve/release, and the over-draw
422.Test plan
npm test -w packages/spacecat-shared-user-manager-client— 169 passing, 100% lines/branches/statements.npm run test:types— clean.npm run lint— clean.npm run test:e2e -w packages/spacecat-shared-user-manager-client— 18 passing.🤖 Generated with Claude Code