feat: eXIP-7.3.0.18 Spaces list portlet EXO-89465

[MayTekayaa]

Integration of the User Spaces List eXip (eXIP7.3.0.18) from feature/devx — this repo's delivery commits cherry-picked in their original order (#6066, #6067), verified patch-identical except the OtpRestTest import of InitContainerTestSuite, which #6067 removed on feature/devx where that test does not exist and which feature/mips needs; restored in the review round 1 fix commit. Technical Specification: Tribe note 50524.

Related PRs of the same delivery: Meeds-io/meeds#4281, Meeds-io/analytics#446, Meeds-io/kudos#628, Meeds-io/gamification#2002. Release order: this PR first (the addons build on social).

Index serving the membership predicate (spec §4 assumption, "named in the PR description"): IDX_SOC_SPACES_MEMBERS_01 (SPACE_ID, USER_ID, STATUS), changeset 1.0.0-100 of social-rdbms.db.changelog-1.0.0.xml, matches the EXISTS sub-query column for column; the owner axis is served by IDX_SOC_SPACES_MEMBERS_03 (USER_ID, STATUS). No new index, no entity, no changeset.

Behaviour changes on the deprecated GET /v1/social/users/{id}/spaces (decision D3 — kept for compatibility, hardened, not restricted): hidden spaces the caller is not a member of are no longer returned, and an external connection now receives the common spaces only (the viewer-axis matrix applies to this endpoint too); an explicit limit is capped at 500; the access check (owner, super user, or a CONFIRMED connection) now lives in SpaceService.checkUserSpacesAccess, REST only maps the status — so a deleted profile owner answers 404 for every caller where it used to answer 200 (super user) or 403 (non-connection); an unknown username still answers 400. {userId}/spaces/{profileId} keeps its check in REST until removal. Both endpoints carry @Deprecated + @DeprecatedAPI(insist = true) pointing at GET /social/rest/users/{username}/spaces.

Knowledge: Meeds-io/eng-standards PR to be opened before merge (number to be filled in here) — domain-doc refresh of domains/social.md (§6, §7, §10, §11, §16), the SpaceFilterKey paragraph of backend-spring.md §2, and the late-install caveat of the profiles page pattern in frontend-vue.md; the patch is drafted and reviewer-checked (knowledge-update.patch from the self-review close-out).

Classification: N1 — approver must be an Architect/Senior Developer (author ≠ approver); no auto-merge on AI review alone.

[boubaker]

AI review — Round #1

Cross-repo review of eXIP 7.3.0.18 "User Spaces List" against the approved Technical Specification (Tribe note 50524, revision 6) and the layer-1 norms: Meeds-io/social#6085, Meeds-io/analytics#446, Meeds-io/meeds#4281, Meeds-io/kudos#628, Meeds-io/gamification#2002. Reviewed as one delivery. First round, so there is no status table.

Findings are inline. Two are 🔴 and both are in test wiring rather than in the feature — but on an N1 delivery that is precisely where they hurt: the whole component/service test module does not compile, so UserSpacesRestTest and UserRestResourcesTest.testGetSpacesOfUserHidesTheHiddenSpacesAConnectionIsNotIn — the evidence for the REST contract and for the hidden-space filter on the deprecated endpoint — have never executed. Verified by running mvn -o -pl component/api,component/core,component/service -am test-compile, which fails with four errors; the base bcb585a608 compiles. component/api and component/core do compile, so the ACL matrix and the cache tests genuinely run.

One finding that cannot be anchored inline (untouched file)

🟡 Dead Less selectors for a dynamic container this eXIP removes

ProfileAboutMe/Style.less:24 and :48 style .profile-after-about-me-container, one of the seven addonContainers Meeds-io/meeds#4281 drops from the profile page. Grepping all five checkouts for profile-right-container and the six profile-before/after-* names across *.less *.css *.vue *.xml *.js *.jsp *.java returns exactly these two hits — nothing in meeds, analytics, gamification or kudos references any of the seven any more. Both blocks, including the @media override, are unreachable once the upgrade runs.

Fix: delete the .profile-after-about-me-container block and its @media counterpart in this PR — same eXIP, same release.

Verified conform

Checked and correct, so they need not be re-litigated:

• The ACL four-case matrix, on the viewer axis. getEffectiveUserSpacesScope branches on the viewer (viewerIdentity.isExternal()), before the requested scope is ever consulted; own profile → ALL; external owner → COMMON; an unresolvable viewer takes the restrictive path rather than failing open. The isExternalUser(profileOwner) bug the specification records as fixed is fixed. I mutation-verified the pin rather than trusting it: reintroducing the owner-vs-viewer branch fails exactly UserSpacesServiceTest.testExternalViewerCannotOptOutOfCommonScope (expected:<1> but was:<3> — the external viewer receives the permissive listing), so that test is real evidence and not a test that passes against its own bug.
• SpaceFilterKey is the compliant form. viewerId is a private final field on the Lombok @Data class, hence equals-compared, and the scope is carried by getType() through USER_SPACES_COMMON / USER_SPACES_ALL. The explicitly rejected alternative — letting the discriminators reach the key only through the folded Objects.hash(filter) int — was genuinely avoided. Only the sorting rides the hash, and it does not decide what a viewer may see.
• Cache eviction: all three required assertions are real, against the real cache. UserSpacesStorageTest runs on the SocialStorageCacheService from AbstractCoreTest — the Kernel ExoCache harness the spec prescribes, not the app-center @Cacheable pattern — and SpaceStorage resolves to CachedSpaceStorage in the test container, so the assertions are not vacuous. Two viewers never share an entry; eviction on a membership change of either side; eviction when a space turns hidden. Two more cover the disclosing direction (wide-cached-then-narrow-requested, and missing-scope-after-wide).
• The predicate. EXISTS rather than a second membership join, so no row multiplication and no interference with the sort; (s.visibility <> :hiddenVisibility OR EXISTS(...)) for ALL and the bare EXISTS for COMMON, on top of the owner axis. PRIVATE stays listed and only HIDDEN is filtered; only the MEMBER role counts; sub-spaces stay flat; the two scopes get distinct query-name suffixes so no cached JPQL string is shared between them. The count query carries the same predicate.
• The index the specification's assumption asked to be named: IDX_SOC_SPACES_MEMBERS_01 (SPACE_ID, USER_ID, STATUS), changeset 1.0.0-100 of social-rdbms.db.changelog-1.0.0.xml. It matches the EXISTS sub-query column for column, so "no new index is required" holds; the owner axis is served by _03 (USER_ID, STATUS). Please paste that name into the PR description, which is where the spec asked for it.
• No new entity, no changeset, no schema change. @Secured("users") per decision D5. DTOs in .rest.model per the package norm. Status contract holds: unknown owner → 404, bad offset/limit → 400 with the message code, an unusable scope narrowed rather than refused. isMember in toUserSpace is in-memory, so no N+1 is added at the REST layer. Interface additions follow social's default-throwing convention. getOrCreateUserIdentity on a client-supplied {username} cannot mint identity rows — it saves only when the org provider resolves the user, else returns null → 404. Nothing new was added to social/webapp: no vue-apps/user-spaces-list, no portlet class, no gatein-resources.xml / portlet.xml / webpack entry — the only frontend addition is the shared SpaceService.getUserSpaces, which the spec sanctions. URLUtilsTest's pin stubs the remote user precisely so it cannot pass against the bug it targets.

What this delivery does well

The caching work is the strongest part, and strong in the way that matters. The two access-control discriminators are real key fields; the same normalizeUserSpacesScope feeds both the SQL predicate and the cache key, so the two cannot drift apart on what an absent scope means — and a drift there would be an access-control defect, not a staleness one; the storage refuses a blank viewer before the cache is consulted, so the exception is not surfaced wrapped from inside a loader; and the tests attack the key in the disclosing direction, not only the convenient one. That is a reviewer's test suite, not an author's. The offset > 0 bypass also shows the spec's open cardinality assumption was actually re-checked before merge, with the stated fallback applied rather than waived.

The comments throughout carry the reason rather than a restatement — why getStreamOwnerId cannot be getCurrentUser, why a failed listing hides the block instead of painting "no common spaces" over an error, why the profile filter runs at import time and what that costs. Several are things a later reader would otherwise get wrong, and one of them is the finding below that I would not have reached as quickly without it.

Process

• The title carries no EXO- id (dev-lifecycle.md §4 wants feat: TITLE EXO-<TRIBE_TASK_ID>); the commits carry EXO-89465, so the id exists — it is just not in the title. Same on Meeds-io/analytics#446.
• No Knowledge: line in the body. These are the feature/mips integration PRs, which is exactly where dev-lifecycle.md §3b step 5 gates it. One concrete item it owes: backend-spring.md §2 names SpaceFilterKey (this.hash = Objects.hash(filter)) as the anti-example of a folded ACL-bearing cache key — this delivery fixes it, so that paragraph now describes code that no longer exists. And domains/social.md §10 holds nothing on spacesCache's budget, its key families or its eviction policy.
• PO deferred item 6 is still open: the hidden-space behaviour change on GET /v1/social/users/{id}/spaces rode in under EXO-89465 with no board story of its own, and the spec called that out as blocking the PR that carries it.

Classification

N1 — confirmed on the diff, not merely inherited from the spec: cross-user ACL decision code, a client-supplied scope narrowed in the Service, an ACL discriminator inside a shared cache key, a new REST surface plus a behaviour change on a live public endpoint. Max-severity aggregation makes the whole five-PR set N1.

This PR must be validated by an Architect/Senior Developer who knows it is N1 — not auto-merged on AI review alone, and author != approver applies. Note that until the two 🔴s are fixed, none of the service-layer evidence for this eXIP has ever run.

🤖 Generated with Claude Code

[MayTekayaa]

AI review — Round #2 (follow-up)

Cross-repo review of eXIP 7.3.0.18 "User Spaces List" against the approved Technical Specification (Tribe note 50524, revision 6), the board (project 8368) and the layer-1 norms: Meeds-io/social#6085, Meeds-io/analytics#446, Meeds-io/meeds#4281, Meeds-io/kudos#628, Meeds-io/gamification#2002, reviewed as one delivery. Head reviewed: d38c1b5153. The two commits pushed since round 1 (15ead30156, d38c1b5153) were re-reviewed in full — the second one moves an access check and therefore re-entered classification as an N1 hunk.

Status of round #1

#	Finding	Status
1	🔴 UserSpacesRestTest on the Boot 3 test API	✅ Fixed — UserSpacesRestTest.java:42-46 now imports org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc / AutoConfigureWebMvc and org.springframework.test.context.bean.override.mockito.MockitoBean; @MockitoBean at :97.
2	🔴 InitContainerTestSuite lost the OtpRestTest import	✅ Fixed — :23 import io.meeds.social.security.rest.OtpRestTest; restored beside the new import; OtpRestTest.class still listed at :79 and the file exists.
3	🟡 Dead .profile-after-about-me-container Less	✅ Fixed — ProfileAboutMe/Style.less keeps only its two @imports.
4	🟡 Wrong cache budget citation in CachedSpaceStorage	✅ Fixed — :369-373 cites plf-meeds-extension/.../conf/meeds/cache-configuration.xml, ${meeds.cache.social.SpacesCache.MaxNodes:4000} and FIFO; re-verified: no <implementation> on that region, so CacheServiceImpl yields SimpleExoCache extends ConcurrentFIFOExoCache.
5	🟡 Own-profile sort vs spec §4	➖ Accepted — settled in-thread: the code follows the board (US05 alphabetical); the specification owes a /resync-spec of §4 and deferred item 1 (Architect item, not this PR).
6	🟡 Access check left in REST	✅ Fixed — SpaceService.checkUserSpacesAccess (interface :1200-1217, default-throwing per convention) implemented at SpaceServiceImpl.java:430-452; UserRest.java:1703-1709 only maps ObjectNotFoundException → 404 / IllegalAccessException → 403; @Operation now says "the super user". Trace below.
7	🟡 Missing @DeprecatedAPI	✅ Fixed — UserRest.java:1661 and :1750, both insist = true, pointing at UserSpacesRest.
8	🟢 Undocumented 500 cap; 100 vs 500 bound	✅ Documented (:1665 "An explicit limit is capped at 500") / ➖ the two bounds still differ — acceptable on a deprecated endpoint.
9	Process — index name in the PR body	❌ Still open — the body does not name IDX_SOC_SPACES_MEMBERS_01 (SPACE_ID, USER_ID, STATUS), which is where the spec's assumption asked for it. One line to add.

The ACL move (d38c1b5153), traced end to end

Compared with the pre-change REST code (719e11bd25:…/UserRest.java): the three admitted callers are unchanged — the profile owner, userAcl.getSuperUser(), and a viewer in a CONFIRMED relationship with the owner (relationshipManager.get(viewer, owner)); PENDING is refused both ways; an anonymous or unresolvable viewer is refused like a stranger. The one behavioural delta is deliberate and stated in the commit message: a deleted owner now answers 404 for every caller (an unknown username still hits the pre-existing 400 at :1697 first). Both usernames are parameters — no ConversationState read below REST. The super-user branch keeps the unfiltered getMemberSpaces(id) listing (:1717); connections go through getUserSpaces(..., ALL, ...) and lose the hidden spaces they are not in. RelationshipManagerImpl(IdentityManager, IdentityStorage, RelationshipStorage) and its storages depend on nothing space-related, so the new constructor injection creates no bean cycle. Four Service tests pin the gate (UserSpacesServiceTest:973-1010) and UserRestResourcesTest still asserts the 403 through REST; a no-op gate fails the refusal test, a dropped super-user branch fails checkUserSpacesAccess("root", OWNER). Not re-run here (no Maven in this session, CI carries it).

Verified conform

• ACL matrix and forced scope unchanged from round 1 and still pinned: own profile → ALL; viewer null or isExternal() → COMMON before the requested scope is read; external owner → COMMON; anonymous → empty/0.
• Cache key: viewerId and type are equals-compared fields of the Lombok @Data SpaceFilterKey; normalizeUserSpacesScope(null) = COMMON is the single method feeding both the predicate and the key; every write path (saveSpace, renameSpace, deleteSpace, ignoreSpace, clearSpaceCached) ends in clearSpaceCache(). UserSpacesStorageTest runs against the real SocialStorageCacheService: two viewers never share an entry, the disclosing direction, the missing-scope case, eviction on either side's membership change and on a space turning hidden, first-page-only caching.
• Predicate: EXISTS on SocSpaceMember (entity name and fields verified), only the MEMBER role, HIDDEN filtered and PRIVATE kept, count query built from the same predicates, distinct named-query suffixes per scope; hiddenVisibility bound at SpaceDAO.java:248.
• Cache-served spaces carry their member arrays (SpaceData.members), so member and membersCount in UserSpace are correct on a cache hit and isMember stays in-memory — no N+1.
• REST contract of UserSpacesRest: @Secured("users") alone (D5); offset < 0 / limit out of 1..100 → 400 with message code; unknown owner → 404; scope forwarded untouched; count only with returnSize; UserSpaceList omits size when null.
• URLUtils.getStreamOwnerId() and its six-case test unchanged; SpaceService.js getUserSpaces matches the controller path and parameter names.
• Nothing added to social/webapp beyond the shared JS service (D1/D2).

What this delivery does well

The access decision for the legacy endpoint now lives in one Service method whose interface entry documents the three admitted callers and both exceptions, and REST is reduced to status mapping — the layering the spec asked for, with a test set argued by mutation rather than by coverage. The cache-budget javadoc now cites the real registrar, the real default and the real eviction policy, which is what makes the first-page-only fallback checkable by the next reader.

Classification

N1 — unchanged: checkUserSpacesAccess and getEffectiveUserSpacesScope are ACL and identity-resolution hunks (ai-review-and-merge.md §2), the viewer-keyed cache is an ACL-bearing surface, and d38c1b5153 is itself an ACL move that re-entered classification. Its approver must be an Architect/Senior Developer who knows it is N1 and is not the author; no auto-merge on AI review alone. The Knowledge: line still reads "to follow" — the feature/mips gate needs the eng-standards PR reference before merge.

Still moving, so not the final round: items 9 above, the two nits, the Knowledge: reference, and — outside this PR — the spec resync (Architect) and PO items 3, 5 and 6.

🤖 Generated with Claude Code

[MayTekayaa]

Self-review close-out — #6085 (eXIP 7.3.0.18, head 611bd90f41)

Three review rounds plus a closing round by a fresh reviewer over the five PRs of the delivery (Meeds-io/social#6085, Meeds-io/analytics#446, Meeds-io/meeds#4281, Meeds-io/kudos#628, Meeds-io/gamification#2002), against Tribe note 50524 (rev. 6), board 8368 and the layer-1 norms. Ledgers: .review/Merge-exip-7.3.0.18/round-{1,2,3}.md (local).

#	Finding	Status
1	🔴 UserSpacesRestTest on the Boot 3 test API (@MockBean, old AutoConfigureMockMvc package)	✅ Fixed 15ead30156 — @MockitoBean, org.springframework.boot.webmvc.test.autoconfigure.*; suite compiles and runs
2	🔴 InitContainerTestSuite lost the OtpRestTest import	✅ Fixed 15ead30156
3	🟡 Dead .profile-after-about-me-container Less	✅ Fixed 15ead30156
4	🟡 CachedSpaceStorage javadoc: wrong cache budget, file and eviction policy	✅ Fixed 15ead30156 — plf-meeds-extension/.../cache-configuration.xml, MaxNodes:4000, FIFO (SimpleExoCache extends ConcurrentFIFOExoCache)
5	🟡 Own-profile sort order vs spec §4	➖ Waived — PO decision of 26/08/2026 (board US01/US02/US05: alphabetical); spec §4 resync owed by the spec author
6	🟡 Access check of GET /v1/social/users/{id}/spaces left in REST	✅ Fixed d38c1b5153 — SpaceService.checkUserSpacesAccess, REST maps 404/403 only; four Service tests pin the gate
7	🟡 Missing @DeprecatedAPI on the two deprecated endpoints	✅ Fixed 15ead30156 (insist = true)
8	🟢 500 cap undocumented; 100 vs 500 bound	✅ Documented / ➖ bounds left different on a deprecated endpoint
9	🟢 Two cosmetic hunks outside the eXIP (SearchPageCard.vue, GroupMembersList.vue)	✅ Fixed 97272a81d2 — files back to feature/mips content
10	🟢 Stale "owes that clamp" javadoc	✅ Fixed 97272a81d2 — cites UserSpacesRest.MAX_LIMIT
11	🟢 Sibling getCommonSpacesOfUser keeps its gate in REST	❌ Open — Architect/PO scope call; documented in this body as kept until removal
12	🟢 Unknown owner 400 vs deleted owner 404	❌ Open — Architect/PO compatibility call; both now documented in the @Operation and this body
13	🟡 Count query javadoc attributed to the drawer, which does not issue it	✅ Fixed 611bd90f41 — offered via returnSize, no product screen sends it
14	🟡 External confirmed connection narrowed to common spaces on the deprecated endpoint, undocumented	✅ Documented 611bd90f41 (@Operation, this body); behaviour unchanged (recall-first, matrix row "external → common only"); D3 wording to be confirmed by PO/Architect
15	Process — EXO- id in title, index name and Knowledge: line in body	✅ title / ✅ index name / see Knowledge: line above

Verified conform (closing round, in source at 611bd90f41): the four-case ACL matrix on the viewer axis in SpaceServiceImpl.getEffectiveUserSpacesScope (own → ALL; external or unresolvable viewer → COMMON forced before the requested scope is read; external owner → COMMON), pinned by UserSpacesServiceTest (17 tests, the external-viewer pin fails the owner-vs-viewer mutant); SpaceFilterKey carries viewerId and the scope (SpaceType.USER_SPACES_*) as explicit equals-compared fields, the same normalizeUserSpacesScope feeding predicate and key; UserSpacesStorageTest (19 tests) runs against the real SocialStorageCacheService — two viewers never share an entry, eviction on either side's membership change and on a space turning hidden, first page only; SpaceDAO EXISTS predicate, HIDDEN filtered and PRIVATE kept, served by IDX_SOC_SPACES_MEMBERS_01; UserSpacesRest contract (@Secured("users"), 400 message codes, 404 unknown owner, in-memory member); no entity, changeset or index added; nothing added to social/webapp beyond SpaceService.getUserSpaces.

Still open for humans: Architect/PO — items 11, 12 and the D3 wording for external connections; spec author — /resync-spec of note 50524 (own-profile order in three places, deleted-owner 404, external-connection narrowing, upgrade "asynchronously", drawer count sentence); PO — deferred item 6 (board story for the users/{id}/spaces behaviour change) and the description of delivery task 90074.

Classification: N1 — cross-user ACL decision code (getEffectiveUserSpacesScope, checkUserSpacesAccess), a client-supplied scope narrowed in the Service, an ACL discriminator in a shared cache key, a new REST surface plus a behaviour change on a live public endpoint. This PR must be validated by an Architect/Senior Developer who knows it is N1 — not auto-merged on AI review alone, author ≠ approver. Release order 1 of 4.

🤖 Generated with Claude Code

[boubaker]

AI review — Round #2 (follow-up)

Re-review of Meeds-io/social#6085 at 611bd90f41 (the branch was rebased onto the current feature/mips since round 1, so the round-1 fix commits cited in the thread replies no longer exist under those SHAs — everything below was verified in the source at this head, and by execution, not from the replies).

Status of round-1 findings

#	Finding	Status
1	🔴 UserSpacesRestTest on the Boot 3 test API	✅ Fixed — org.springframework.boot.webmvc.test.autoconfigure.* + @MockitoBean at :96. mvn -o -pl component/api,component/core,component/service -am test-compile is green, and the class ran for the first time: 9 tests, 0 failures
2	🔴 InitContainerTestSuite lost the OtpRestTest import	✅ Fixed — both imports at :23-24, OtpRestTest.class at :79, file present. UserRestResourcesTest.testGetSpacesOfUserHidesTheHiddenSpacesAConnectionIsNotIn — the Contract-2 pin — executed and passes
3	🟡 Cache javadoc: wrong budget, file, eviction policy	✅ Fixed — plf-meeds-extension/…/conf/meeds/cache-configuration.xml, MaxNodes:4000, FIFO; the "owes that clamp" residue now cites UserSpacesRest.MAX_LIMIT
4	🟡 Own-profile sort order vs spec §4	➖ Accepted — board US01 (89465) and US05 (89468) say alphabetical, read directly from the board this round; /resync-spec of note 50524 §4 and deferred item 1 stays with the spec author
5	🟡 Access check of GET /v1/social/users/{id}/spaces left in REST	✅ Fixed — SpaceService.checkUserSpacesAccess (SpaceServiceImpl.java:430), REST maps 404/403 only; 4 new Service pins cover owner, super user, CONFIRMED (both directions), PENDING, anonymous, unresolvable and unknown owner. UserSpacesServiceTest: 17 tests green. Mutation-verified: accepting a PENDING relationship fails exactly testLegacyListingAccessIsRefusedWithoutAConfirmedRelationship — the pin is evidence, not decoration
6	🟡 Missing @DeprecatedAPI	✅ Fixed — insist = true on both getSpacesOfUser (:1661) and getCommonSpacesOfUser (:1750)
7	🟡 Dead .profile-after-about-me-container Less	✅ Fixed — no occurrence left in ProfileAboutMe/Style.less
8	🟢 scope without defaultValue	➖ Not taken — still required = false, no default (UserSpacesRest.java:85). Harmless, as stated in round 1
9	🟢 500 cap undocumented; 100 vs 500	✅ Documented in the @Operation / ➖ bounds kept different, fine on a deprecated endpoint
10	🟢 Two formatting-only hunks	✅ Fixed — both files out of the diff
11	🟢 member vs isMember naming	➖ spec-side, no change expected
—	Process: EXO- id in the title / index named in the body / Knowledge: line	✅ / ✅ (IDX_SOC_SPACES_MEMBERS_01) / ❌ still open — the body says "to be opened before merge (number to be filled in here)". The feature/mips gate (dev-lifecycle.md §3b step 5) needs either the Meeds-io/eng-standards#NN reference or none — <reason>; a promise is neither

New this round

The fix for #5 is new ACL code and re-entered full review (ai-review-and-merge.md §3 rule 5). It is correct; three residuals are anchored inline — two of them are decisions for the Architects Lead / PO rather than defects, and I am stating them as such:

• the unknown-owner 400 still decided in REST before the Service's 404 — two existence outcomes, two statuses (inline at UserRest.java:1704);
• an external CONFIRMED connection is now narrowed to common spaces on the deprecated endpoint — right by the matrix, but a step beyond what decision D3 promised, and it has no endpoint-level pin (inline at the @Operation, :1665);
• 🟢 the super-user "unfiltered listing" branch is still a REST-side business rule (inline at :1717).

Verified conform, by execution this time

component/api, component/core, component/service test-compile; UserSpacesServiceTest 17/17; UserSpacesRestTest 9/9; the UserRestResourcesTest hidden-space pin 1/1. checkUserSpacesAccess traced: resolves the owner first (deleted or unknown → ObjectNotFoundException), admits the owner and userAcl.getSuperUser(), resolves the viewer itself rather than trusting the caller, refuses anonymous and unresolvable viewers exactly like a stranger, and refuses anything but Relationship.Type.CONFIRMED. SpaceService gained the method as a default throwing UnsupportedOperationException, per the repo convention. Nothing else in the socle part of the diff moved: getEffectiveUserSpacesScope, SpaceFilterKey, the EXISTS predicate, the cache-key derivation and UserSpacesRest are byte-identical to round 1.

Not clean yet, but nothing left is a code blocker: one process item (the Knowledge: PR number) and three decisions. Once the decisions are recorded — in the spec's Functional divergences confirmed or in the code — the next round can be the close-out.

Classification: N1 — unchanged and re-confirmed on the fix commits themselves (checkUserSpacesAccess is ACL decision code). Architect/Senior Developer validation, author != approver, no auto-merge on AI review alone.

🤖 Generated with Claude Code

[MayTekayaa]

AI review — Round #3 (final)

Cross-repo review of eXIP 7.3.0.18 "User Spaces List" against the approved Technical Specification (Tribe note 50524, revision 6), the board (project 8368) and the layer-1 norms: Meeds-io/social#6085, Meeds-io/analytics#446, Meeds-io/meeds#4281, Meeds-io/kudos#628, Meeds-io/gamification#2002, reviewed as one delivery. Head reviewed: 611bd90f41 (rebased over feature/mips; the PR diff was compared hunk by hunk with the round-2 diff — the only deltas are the two fixes below and javadoc/Swagger wording, no code change).

Status of every finding

Round	#	Finding	Status
1	1	🔴 UserSpacesRestTest on the Boot 3 test API	✅ Fixed (15ead30156, now 81c365f689)
1	2	🔴 InitContainerTestSuite lost the OtpRestTest import	✅ Fixed
1	3	🟡 Dead .profile-after-about-me-container Less	✅ Fixed
1	4	🟡 Wrong cache budget citation in CachedSpaceStorage	✅ Fixed — 4000 / plf-meeds-extension/.../cache-configuration.xml / FIFO, each re-verified
1	5	🟡 Own-profile sort vs spec §4	➖ Accepted — the code follows the board (US05 alphabetical); /resync-spec of §4 and deferred item 1 owed by the spec author
1	6	🟡 Access check left in REST	✅ Fixed (d38c1b5153, now 81c3717772) — SpaceService.checkUserSpacesAccess, REST maps 404/403 only
1	7	🟡 Missing @DeprecatedAPI	✅ Fixed — insist = true on both legacy endpoints
1	8	🟢 Undocumented 500 cap; 100 vs 500	✅ Documented / ➖ bounds kept distinct on a deprecated endpoint
1	9	Process — index name in the body	✅ Fixed — body names IDX_SOC_SPACES_MEMBERS_01 (SPACE_ID, USER_ID, STATUS) and _03 for the owner axis
2	1	🟢 Two cosmetic hunks unrelated to the eXIP	✅ Fixed (97272a81d2) — GroupMembersList.vue and SearchPageCard.vue are out of the diff (23 files, was 25)
2	2	🟢 Stale "owes that clamp" javadoc	✅ Fixed (97272a81d2) — now "the REST endpoint bounds limit to UserSpacesRest.MAX_LIMIT"

The last commit (611bd90f41) also corrects two statements this review had relied on: the count query is offered through returnSize and no product screen issues it (the drawer paginates with a lookahead row), and the legacy endpoint's @Operation now says that an external connection receives common spaces only and that a deleted user yields 404 — both verified against getEffectiveUserSpacesScope and checkUserSpacesAccess.

All findings from previous rounds are resolved; nothing outstanding from the AI review side.

Verified conform (closing pass by a fresh reviewer at this head)

Eviction audit: every write path that can change this listing — saveSpace (18 call sites in SpaceServiceImpl), renameSpace, deleteSpace — goes through a CachedSpaceStorage override that calls clearSpaceCache(); ignoreSpace and updateSpaceAccessed write only statuses this listing does not read, and SpaceMemberDAO has no writer outside SpaceStorage. Owner-axis predicate always added (statusList set unconditionally, branch SpaceWithStatuses), distinct named queries per scope, null sorting on the count path safe through SpaceFilter.getSorting()'s default. @JsonInclude from com.fasterxml.jackson.annotation is honoured by the Jackson 3 introspector on the platform. ACL matrix on the viewer axis with the forced scope; viewerId and scope as equals-compared cache-key fields with the shared null-scope normalisation; first-page-only caching against a 4000-entry FIFO region; EXISTS predicate on MEMBER role, HIDDEN filtered, PRIVATE kept, count parity; REST status contract 404 → 403 → 400 with message codes; @Secured("users") alone (D5); nothing added to social/webapp beyond the shared JS service; real-cache storage tests and the four-row matrix tests, plus the four tests pinning the relationship gate.

What remains for humans, outside this review

• Merge gate (dev-lifecycle.md §3b step 5): the body's Knowledge: line still says the eng-standards PR is to be opened; the patch exists (knowledge-update.patch, domain-doc refresh of domains/social.md §6/§7/§10/§11/§16, the SpaceFilterKey paragraph of backend-spring.md §2, the late-install caveat in frontend-vue.md). Open it and put its number here before merge — the gate reads that field.
• Spec resync (Architect, /resync-spec note 50524): §4 own-profile sort bullet and deferred item 1 (alphabetical everywhere, not LASTVISITED); §4 "executed once, asynchronously" — the plugin runs synchronously (UpgradeProductPlugin.asyncUpgradeExecution defaults to false and the meeds configuration sets no plugin.upgrade.async.execution for it); §2 query budget "the drawer does [issue the count query]" — the drawer paginates with a lookahead row and never sends returnSize, so no product screen issues the count today (a cheaper outcome than specified, worth recording).
• PO: deferred items 3 (where weekly points and rank resurface), 5 (the block's title on the profile page — the control is back in the settings drawer), 6 (a board story and EXO- id for the behaviour change on GET /v1/social/users/{id}/spaces).

Norm capture — proposed corpus additions from these rounds

Norms invoked across the three rounds that the corpus does not hold, or holds incompletely; each is one eng-standards change fragment:

1. @DeprecatedAPI(insist = true) beside @Deprecated on a deprecated REST endpoint in social — backend-spring.md §6 asks only for the annotation and the comment; the runtime-surfacing aspect (DeprecatedAPIAspect, PropertyManager.isDevelopping()) is social's convention with 22 precedents and belongs in domains/social.md with a pointer from §6.
2. The profiles attribute is evaluated at page import/store time, not at render (Container.buildChildren()), so an addon installed after the upgrade never gets its cell back without a layout-editor restore — frontend-vue.md "Placing portlets on pages" (in the drafted knowledge patch).
3. The profile name a page cell must use is the WAR's add.profiles value, not its container dependency or context name — gamification's profile is gamification while its dependency is gamification-portlets; no domain doc mentions profiles= or add.profiles today (gap found in round 2). Same section of frontend-vue.md.
4. social.SpacesCache is a FIFO region of 4000 entries registered in meeds → plf-meeds-extension/.../cache-configuration.xml; the exo.cache.social.SpacesCache.* keys of plf-configuration are inert — domains/social.md §10 (in the drafted patch) with a pointer from domains/meeds.md.
5. An UpgradeProductPlugin runs synchronously unless plugin.upgrade.async.execution=true — worth one sentence next to the LayoutUpgradePlugin reference in frontend-vue.md, since a spec repeated the opposite.
6. Integration-PR hygiene on N1: cosmetic or lint hunks outside the spec's scope do not ride an N1 integration PR — candidate line for the N1 checklist of ai-review-and-merge.md §6.

Classification

N1 — final: ACL and identity-resolution hunks (checkUserSpacesAccess, getEffectiveUserSpacesScope), an ACL-bearing cache key and a new REST surface. Its approver must be an Architect/Senior Developer who knows it is N1 and is not the author; no auto-merge on AI review alone. Release order 1 of 5.

🤖 Generated with Claude Code

[MayTekayaa]

AI review — correction to Round #3

The Round #3 (final) comment above (15:52 UTC) was drafted from a check made at 15:27 UTC and did not see the Round #2 (follow-up) posted by the other reviewing session at 15:38 UTC. Its "nothing outstanding from the AI review side" statement is therefore withdrawn for this PR: the review reopens on the three findings of that round — the two "owner does not exist" statuses (400 in REST vs 404 from the Service), the narrowing of an external CONFIRMED connection to common spaces on this endpoint (spec row under D3 + a REST-level pin owed), and the super-user listing rule still decided in REST (follow-up nit).

Everything else in Round #3 stands: the round-1 and round-2 items are fixed at 611bd90f41, the norm-capture list, the remaining human items, and the N1 classification (Architect/Senior Developer approval, author ≠ approver, no auto-merge on AI review alone).

🤖 Generated with Claude Code

[boubaker]

AI review — Round #3 (follow-up)

Re-review of Meeds-io/social#6085 at 0cc720ac1c — two linear commits since round 2 (1625862f53, 0cc720ac1c), 2 files, +51/−2, no rebase this time. Verified in source and by execution.

Status of round-2 findings

#	Finding	Status
1	🟡 Unknown owner 400 (REST) vs deleted owner 404 (Service)	➖ Option (b) implemented, pending the Architects Lead's word — the pre-check stays; the @Operation now states both statuses and why ("an unknown username keeps answering 400, as this operation always has, for compatibility with existing integrations"), and the code comment above the check carries the D3 reasoning. Verified at :1695-1700 / :1665. The spec row under Functional divergences confirmed is the /resync-spec item. My recommendation to the Architects Lead is (b): on a deprecated, integrator-facing endpoint, D3's own logic — preserve the live contract, record the divergence — applies to the status as much as to the relationship gate
2	🟡 External CONFIRMED connection narrowed to common spaces — beyond D3, unpinned	✅ Fixed — UserRestResourcesTest.testGetSpacesOfUserNarrowsAnExternalConnectionToCommonSpaces (:785-820): demo (external, CONFIRMED) receives the one common space, mary (internal, same relationship) receives both; Profile.EXTERNAL set and reset in a finally. Executed at this head: baseline 2/2 green; mutation-verified — with the isExternal() branch of getEffectiveUserSpacesScope disabled, exactly this test fails at :806 (expected:<1> but was:<2>), while the internal-connection pin still passes, so the external flag is the only thing this test depends on. The PR body no longer says "behaviour unchanged" — it names the narrowing as a behaviour change. The D3 wording and PO item 6's story remain human items
3	🟢 Super-user "unfiltered listing" rule decided in REST	➖ Accepted as follow-up, for a reason better than mine — moving the rule into getEffectiveUserSpacesScope would change what the new endpoint returns to root on any profile, i.e. it adds a super-user row to the spec's four-case matrix, which has none. That is a spec decision, not a refactoring; the branch keeps the documented legacy behaviour until the spec says otherwise
—	Process: Knowledge: line	❌ Still a placeholder — "to be opened before merge (number to be filled in here)". This is now the only item on this PR that is a fix rather than a recorded decision, and it is the feature/mips merge gate (dev-lifecycle.md §3b step 5)

Nothing new this round: the delta is one Swagger sentence, one code comment and one test, all read in full; no code path changed.

Where this leaves the PR

No code finding is open. What remains, by owner:

• Developer — open the eng-standards knowledge PR and put its number in the Knowledge: line. That is the close-out condition for this PR.
• Architects Lead — confirm (b) on item 1, or ask for (a) (drop the pre-check, pin the 404; two lines).
• Architects Lead + PO — confirm the D3 narrowing wording; PO — the board story for deferred item 6 (this endpoint's behaviour changes: hidden filter, external narrowing, deleted → 404, 500 cap — one story now covers four documented changes).
• Spec author — /resync-spec of note 50524: §4 own-profile sort and deferred item 1; D3 rows for items 1 and 2 above; §4 "asynchronously" (the plugin runs synchronously); §2 "the drawer does [issue the count]" (no product screen does). The self-review's norm-capture list on this PR is a sound starting point for the corpus PR — the add.profiles-vs-context-name point and the FIFO/4000 registrar fact in particular.

The next round can be the close-out once the Knowledge: number is in; per the org's review model it should be run by a fresh reviewer.

Classification: N1 — unchanged; the round-3 delta touches no decision code. Architect/Senior Developer validation, author != approver, no auto-merge on AI review alone.

🤖 Generated with Claude Code

The merge-base changed after approval.

[sonarqubecloud]

Quality Gate passed

Issues
6 New issues
2 Accepted issues

Measures
0 Security Hotspots
92.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud