[antoinetoussaint-byte]

What changed

ReviewOffer looked up a target's installation by the stable id the core mints (Config.InstallationID). A host that installs modules itself (Config.HostInstallsModules, for example module-saas-starter accounts) gives each module installation its own id. On such a host, a module page's review came back with an empty installation. The module_page_roles case added in #61 catches this: "the administrator reviews roles [installer viewer] at installation """.

On such a host, the installation is now found by its target (installationOf), the same way held() finds it. Solutions and the local authority are unchanged: the stable id still answers first.

Classification

This is a fix at the owning place, the core's offer review.

Tests

• New test TestAHostNamedModulePageInstallationIsReviewed. It fails when the change is reverted (checked).
• go test ./..., go vet and gofmt are clean in authority-core, conformance and the local authority.
• module-saas-starter#1282 pins this head and runs the full conformance suite against it.

Review

An independent review is still pending.

🤖 Generated with Claude Code

[antoinetoussaint-byte]

#62 review, head 5e276bf (base 9577d43)

No blockers. The fix is in the right place, it is complete, and no test was weakened (the diff only adds lines). The new test fails when the fix is reverted (I checked that; the revert was undone). go test ./... passes in authority-core and in the local authority.

Findings, most important first:

1. Follow-up: two lookups can pick different installations. At consent.go:86, the stable id is still read first, and any record found under it wins, tombstoned or not. held() (issue.go:41, through installationOf, service.go:403) prefers the active one. On a host that installs modules, the core never writes stable-id records for modules (lifecycle.go:109,158). So the two disagree only if a stale stable-id record is left over. A simpler and consistent fix: on such hosts, use installationOf for module pages.
2. Follow-up: an empty org matches every org. installationOf(tx, "", …) asks the store for installations with org "", and the in-memory store (memstore.go:320) returns every org's for that. A non-admin with org="" reaches the lookup, gets a refusal of the "denied" kind, then takes the member path. holdsRoleOn (consent.go:136, inst.Organization != org) refuses, so nothing leaks. Still, add an explicit guard for an empty org, since how a host store handles "" is not tested.
3. Follow-up (pre-existing): several active installations do not fail closed. When more than one is active, the first one in store order is used. Review and issuance at least use the same rule.
4. No issue: tenancy and name collisions. The lookup is bound to the org. A module and a solution cannot share a name (ModuleNamed keys on the name), so a target cannot match across kinds.
5. No issue: completeness. Issuance, current, delegation, client and attachment already use installationOf. roles.go:40 and installationIn take an installation id from the caller and check its org.

VERDICT: merge — the lookup is bound to the org, matches what issuance already does, and the member path still refuses on the org check.