docs: own the Hub roadmap here, and reconcile the LearnStack boundary - #2
Merged
Conversation
The Hub plan lived in LearnStack's phase-02c-hub-foundation.md, and this repository carried a 43-line status mirror that had been stale for four commits. It reflected neither the 18-skill catalogue nor the five P02c-1 design specs that shipped after it. docs/roadmap/ is now the authoritative Hub plan: an index with an intra-Hub dependency map and a cross-repo blocking table in both directions, plus a document per packet carrying Goal, Scope, Deliverables, Completion Criteria, Risks and Phase Exit Decision. hub-billing.md and hub-marketplace.md migrate from LearnStack's phase-09b and phase-12, which become pointers. The P02c-N identifiers are unchanged. They appear in branch names, commit subjects, skill bodies and design specs across both repositories, and renumbering would orphan every one of them. Contract and durability changes tracked from LearnStack: - ADR-0034 replaces the "closed at four endpoints" rule with two invariants: the Hub stores no tenant content, and every crossing goes through a named adapter. P02c-5 accordingly moves certificate material out of the entitlement payload -- host mappings carry the tenant tuple only, and cert material moves by secret-store replication, referenced by path. - P02c-6 adds license_id to the licence payload; revocation depends on it and the documented claim set omitted it. - P02c-1 is recorded as implemented-on-branch and frozen, with the condition that unfreezes it: a tenant must be billed or plan-gated (the ADR-0035 trigger for the Hub entitlement adapter). Stale facts corrected: - Every sibling path now uses the real directory names, ../LearnStack and ../LearnStack-Hub. Roughly 130 references assumed lowercase and resolved only because macOS is case-insensitive; they would break on Linux CI, which cannot catch them because the link audit deliberately skips sibling links. Three files also ascended one level too far. - docs/README.md and AGENTS.md claimed this repository maintains no skill catalogue. It maintains 18 skills. - Program.cs described an 8-step MediatR pipeline; Hub's is 6-step, with no TenantContextBehavior, because it stores no tenant content. - repository-layout.md listed two architecture docs where there are five, and marked docs/modules/ as future when four module specs had shipped. - The Co-Authored-By trailer named a stale model version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ation An independent pass found no blockers and three gaps on the Hub side, each a place where a rule LearnStack's ADR-0034 replaced survived in a file the migration touched only for path casing. - The four-endpoint claim persisted in `docs/glossary.md`, in `contract-with-learnstack.md` -- the file whose entire job is pointing at the authoritative contract spec, and which never mentioned ADR-0034 -- and in `code-review`, `commit-and-pr` and `start-task`, the three skills that gate every change. Also in the PR template and `.env.example`. Two repositories carried one glossary each, stating opposite facts about the same boundary. All now state the two invariants: the Hub stores no tenant content, and every crossing goes through a named adapter. - The Co-Authored-By trailer still named a stale model version in `CONTRIBUTING.md`, `commit-and-pr` and the P02c-1 implementation prompt. - `docs/modules/tenant-lifecycle.md` deferred the hard-delete-with-confirmation flow to "a later packet" without naming it, against a CLAUDE.md hard rule. It is P02c-4, which builds the operator surface that confirms the deletion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…doc corrections Paired with LearnStack's harvest-round commit; same wave, one commit per repository. The blocker: `.github/workflows/ci.yml`'s link audit skips the existence check for cross-repo links because CI cannot reach the sibling checkout, but the skip pattern matched `learnstack` in lowercase only. Bash `=~` is case-sensitive and this workflow sets no `nocasematch`. Commits eb85c2b and f105d07 recapitalised all 215 cross-repo links to `../LearnStack/...`, so every one of them fell through to a check that cannot pass. The pattern now matches both capitalisations explicitly rather than depending on a shell option set elsewhere; the trailing `(/|$)` still keeps a hypothetical `../LearnStack-Hub/...` self-reference out of the skip. Also: - P02c-1's out-of-scope list said "the four HTTPS contract endpoints". ADR-0034 replaced the four-endpoint claim with two invariants over nine endpoints; the sentence needed no count at all. - p02c-5's event table had the custom-domain events inserting and deleting `platform_host_to_tenant` rows directly. The mapping travels over PUT /api/internal/tenants/{id}/host-mappings; the events invalidate the resolver cache. - p02c-6's licence-expiry ladder read as though it should be reconciled with ADR-0034's Hub-outage ladder. They are deliberately different — an outage is a control-plane failure the tenant did not cause, an expired licence is the licence working — and the paragraph now says so, so a later sweep does not "fix" it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e bundle Paired with LearnStack's wave-4 commit; same wave, one commit per repository. ADR-0034 § One auth chain, both directions replaces the per-instance API key on the LearnStack -> Hub direction with the same mTLS + RS256 JWT + HMAC chain the other direction already carries. That leaves a SelfHostedOnline instance unable to phone home at all unless something hands it a client certificate the Hub's CA will validate, and P02c-6 is the only packet that hands a customer-run instance anything. The certificate rides with the .lic file: same license_id, same re-issue command, same expiry cadence, same revocation. One artefact to issue, one to rotate, one to revoke — and revoking a licence takes the transport away before it takes the entitlement away. SelfHostedAirGapped makes no outbound call and is issued none. The completion criterion asserts the negative case at the layer that should catch it: a phone-home with no certificate, an expired one, or one belonging to a revoked licence is refused at the TLS handshake, before any handler runs and before the JWT or the body signature is examined. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…counts
Paired with LearnStack's wave-6 commit; same wave, one commit per
repository.
This file is pointer-only and its pointers had gone stale in a way that
matters more here than anywhere else: it is what a Hub-side agent reads
instead of the LearnStack corpus. It named ADR-0019 as the source for
"the endpoint set + auth chain" — one row for two topics that ADR-0034
now splits, replacing the endpoint framing entirely and extending the
auth chain to the LearnStack -> Hub direction. The invariants row pointed
at Standards 20 and ADR-0019 rather than at the ADR that introduced them.
The packet table said "the two LearnStack -> Hub endpoints" and "the two
Hub -> LearnStack endpoints" — a count that was wrong under ADR-0019 (six
paths) and is wrong under ADR-0034 (nine). Counts removed rather than
corrected; a pointer file that carries a number acquires a maintenance
obligation it exists to avoid.
P02c-5's row still described the custom-domain events as the channel that
writes `platform_host_to_tenant`. They are the cache-invalidation signal;
the mapping travels over `PUT /api/internal/tenants/{id}/host-mappings`,
and certificate material moves by secret-store replication rather than
through the entitlement payload — ADR-0034 superseding ADR-0022
Amendment 1 step 3.
The ADR-0022 row now says which part of Amendment 1 survives, and the
architecture-test enumeration gains
`Hub_Client_Referenced_Only_By_Named_Adapters` with the counts corrected
to seven.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e 11 Paired with LearnStack's wave-7 commit; same wave, one commit per repository. The blocking table had one row pointing P02c-5 at LearnStack Phase 11 for "the LearnStack-side host-mapping handler". LearnStack's 27-custom-domain-tls § 11 puts that handler and the platform_host_to_tenant mirroring in Phase 02c, and only the ACME automation and the edge certificate-installation path in Phase 11 — host resolution is a one-way door, the automation that populates the mapping is additive. Split into two rows so the distinction survives: the Phase 02c row is the paired half of this packet, merged in the same session; the Phase 11 row is the edge half and carries an explicit "P02c-5 does not wait on it", because host resolution works from the platform_host_to_tenant row alone. Without that clause the split silently re-gates P02c-5 on Phase 11 by a different route. The "LearnStack waits on Hub" table's matching row said the mirror handler was Phase 11; corrected to Phase 02c. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Wave 8, Hub half. LearnStack's Documentation Standards § Layout says plainly that "cross-repo references use absolute URLs". This repository had 222 Markdown links written as sibling-relative paths — `../LearnStack/...`, `../../LearnStack/...`, `../../../LearnStack/...` depending on the source file's depth — across 37 files. Every one of them 404s on github.com, where a relative link cannot cross a repository boundary, and every one depends on the reader having a sibling checkout that is present and identically capitalised. The depth-dependence is its own hazard: moving a document one directory deeper silently breaks all its cross-repo links. The sweep rewrites link targets only. Shell commands (`cd ../LearnStack`) and filesystem references in prose and code spans keep the relative path, because those are paths, not links, and the sibling layout is still real. An earlier, broader pass rewrote both and produced `cd https://github.com/...`; it was discarded rather than patched. With the links absolute, the CI link audit's cross-repo exemption is removed rather than kept. It existed because CI cannot reach a sibling checkout — but the external-URL skip already covers absolute URLs, and a relative cross-repo link is now a defect the audit should catch. It fails the build instead of reaching a reader. The four documents that taught the old convention — README, docs/README, CLAUDE.md, AGENTS.md — now separate the two rules that were conflated: links are absolute URLs; shell and filesystem paths stay relative and case-sensitive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…r of Hub operations Paired with LearnStack's waves 9-10 commit. P02c-3 called itself "the only packet in the P02c series that changes both repositories". P02c-5 carries the LearnStack-side host-mapping handler and P02c-6 the SignedLicenseKeyEntitlementProvider skeleton — three packets, not one — and the "unblocked by LearnStack" sentence listed P02c-6 among them. Both corrected, and the two missing rows added to the blocking table: P02c-3 on LearnStack Phase 02b (the outbox processor, the inbox guard, handler-scope context restoration — without which IUsageReporter cannot dispatch through the outbox and the entitlement invalidation consumer has no guard), and P02c-6 on P02a-9's IEntitlementProvider socket. P02c-3 is the only packet gated on the LearnStack spine reaching Phase 02b rather than merely Phase 02a, and the README now says so. Hub production deployment pointed at LearnStack Phase 11, which scopes itself to LearnStack and does not cover the Hub's own database, realm or certificate chain — so the P02c-7 exit gate named a dependency nobody owns. A "Hub Operations" track joins the post-MVP table, deliberately unlinked because no plan document exists yet and a dead link is worse than an unlinked row. P02c-5 gains the inbound custom-domain submission handler as a deliverable — the hop LearnStack's Admin Studio proxies because the Hub rejects a `learnstack` realm token — and its ADR-0022 Amendment 2 citation now says which half of that amendment survives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Paired with LearnStack's waves 11-12 commit. The pipeline arithmetic. cross-cutting-foundation said "LearnStack runs an 8-step pipeline ... Hub runs a 6-step pipeline. The two LearnStack steps that drop out are..." and then listed one. LearnStack's eight-step list counts the Handler; Hub's six does not. Both sides now count behaviors — seven and six — and the document says which convention it uses, so the delta is one behavior and the arithmetic works. p02c-1 and the two derived mentions follow. The glossary was restating rules it does not own. `generation` carried `cached.generation >= received.generation` — the inequality inverted, and a third independent statement of a LearnStack-side acceptance predicate; it now defines the counter and points at the one place the acceptance rule lives. `LicenseKey` named `valid_from` / `valid_until` and "public key hash" where P02c-6's column table says `issued_at` / `expires_at`, `kid` and `entitlement_generation` — a third spelling of the timestamps P02c-6 exists to reconcile. `Entitlement` now says plainly that the Hub-side aggregate is the only writer anywhere and that what crosses the wire is the distinct *Entitlement Projection*. The plan editor was assigned to P02c-4 in the module doc and to Hub Billing in the roadmap; Hub Billing wins, so a plan's feature payload and its price are edited on one form. Soft-limit alerts were described as "produced since P02c-2" — they are produced by LearnStack at the gated call site and merely ingested by P02c-2; this track adds the Hub half. The marketplace row advertised revenue share, which its own plan puts out of scope. P02c-3's reconciliation named IHubTenantSync re-pushing a tenant, but IHubTenantSync is the LearnStack-side handler; the Hub re-issues POST /api/internal/tenants and the handler is idempotent on the Hub-minted id. Polish: Status banners on P02c-4 through P02c-7, which opened straight into Goal while P02c-1 through P02c-3 each carry one; the operator-portal naming ledger entry closed with its three real residues named, and the Keycloak client id explicitly excluded from the rename; and one lowercase sibling path in CONTRIBUTING. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ually has Paired with LearnStack's commit of the same name. This repository carried 248 references to `cemililik/LearnStack` — 222 of them the absolute cross-repo links introduced in the previous commit, the rest in prose and packet documents. Both repositories now live in the HodeTech organisation; `git remote get-url origin` says so. The old URLs still resolve, via GitHub's 301 rename redirect. That is the reason to fix them now rather than later: the redirect holds only while nothing occupies the old path, and `cemililik` is a live personal account. If anything is ever created there, these links resolve to a different repository instead of failing. Left untouched on purpose: `cemililik/leakwatch` in the pre-commit hook and CI (a Go module path, whose canonical form is set by the module's own go.mod — rewriting it can break `go install` rather than redirect it), the Homebrew tap, and `@cemililik` in CODEOWNERS, which names a person. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cemililik, your pull request is larger than the review limit of 150000 diff characters
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe changes align Hub guidance, documentation, architecture references, repository paths, CI link checks, contract invariants, pipeline descriptions, and roadmap packets with the current P02c-1 state and planned cross-repository work. ChangesHub documentation alignment
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideThis PR migrates ownership of the Hub roadmap into the Hub repo, tightens and corrects cross-repo contract documentation versus updated ADRs, converts all cross-repo documentation links to absolute GitHub URLs, and fixes the CI markdown link audit so relative cross-repo links are treated as defects rather than exempted, alongside several roadmap, glossary, and skills-catalogue reconciliations. Sequence diagram for updated CI markdown link audit behavioursequenceDiagram
participant CI as GitHub_Actions_CI
participant LinkAudit as MarkdownLinkAudit
CI->>LinkAudit: Run link_audit_script
loop For each link
LinkAudit->>LinkAudit: Check_scheme(link)
alt External URL (http/https/mailto...)
LinkAudit->>LinkAudit: Skip (no filesystem check)
else Relative path (incl. ../LearnStack/...)
LinkAudit->>LinkAudit: Normalize link_path
LinkAudit->>LinkAudit: Test file existence
alt File missing
LinkAudit->>CI: Report failure (relative link defect)
else File exists
LinkAudit->>LinkAudit: Continue
end
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (13)
docs/roadmap/p02c-4-operator-portal.md-122-131 (1)
122-131: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse one repository-relative path for the operator portal.
The section names
frontend/apps/operator-portalbut later usesapps/operator-portalfor the same application. From the repository root, these are different paths. Usefrontend/apps/operator-portalfor repository-root references, or explicitly labelapps/operator-portalas frontend-workspace-relative.🤖 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 `@docs/roadmap/p02c-4-operator-portal.md` around lines 122 - 131, Standardize all repository-root references to the operator portal on frontend/apps/operator-portal, including the later apps/operator-portal mention in this section. If retaining apps/operator-portal, explicitly identify it as frontend-workspace-relative.README.md-30-34 (1)
30-34: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language to the directory-layout fence.
The Markdown linter reports MD040 because this fenced block has no language. Use
textfor the directory tree.Proposed fix
-``` +```text <parent-dir>/ ├── LearnStack/ (https://github.com/HodeTech/LearnStack) └── LearnStack-Hub/ (this repo)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@README.mdaround lines 30 - 34, Add the text language identifier to the
directory-layout fenced block in README.md by changing its opening fence to useSource: Linters/SAST tools
.github/workflows/ci.yml-174-178 (1)
174-178: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale link-audit comment.
Lines 174-178 still say that CI skips
../LearnStack/paths. The audit now deliberately rejects those paths in Lines 201-208. This comment can mislead maintainers into adding invalid relative documentation links.As per coding guidelines, sibling-relative paths are reserved for filesystem and shell usage; documentation links must use absolute GitHub URLs.
Proposed comment correction
- # relative paths — those links resolve only on the developer's - # workstation (sibling layout). CI can't validate them because the - # LearnStack core repo isn't checked out alongside; the audit skips - # `../LearnStack/` paths explicitly. + # Sibling-relative `../LearnStack/` documentation links are invalid + # on GitHub. The audit rejects them; use absolute GitHub URLs instead.🤖 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 @.github/workflows/ci.yml around lines 174 - 178, Update the link-audit comment near the link validation logic to state that ../LearnStack/ documentation paths are rejected, not skipped. Clarify that documentation links must use absolute GitHub URLs and that sibling-relative paths are reserved for filesystem or shell usage; keep the audit behavior in the relevant validation block unchanged..claude/skills/commit-and-pr/SKILL.md-43-51 (1)
43-51: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse the trailer for the actual contributing agent.
The skill hard-codes
Claude Opus 5and lists onlyCodexas an alternative. Select theCo-Authored-Byvalue from the agent that materially contributed to the commit. Add one trailer per contributing agent.Based on learnings, use the appropriate
Co-Authored-Bycommit trailer for the contributing agent, with one trailer per agent when multiple agents materially contribute.Proposed wording
-- **trailer:** `Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>` (or `Codex …` for Codex sessions). HEREDOC every multi-line message. +- **trailer:** Add one `Co-Authored-By` trailer for each AI agent that materially contributed, using that agent's actual name and email. HEREDOC every multi-line message.🤖 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 @.claude/skills/commit-and-pr/SKILL.md around lines 43 - 51, Update the commit trailer guidance in the commit workflow example to require the actual materially contributing agent’s identity rather than hard-coding Claude Opus 5 or only mentioning Codex. Preserve the HEREDOC format and instruct contributors to add one Co-Authored-By trailer for each agent that materially contributed.Source: Learnings
docs/roadmap/P02c-1-implementation-prompt.md-6-6 (1)
6-6: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse
LearnStack-Hubfor the working-repository name.The prompt uses
learnstack-hubin a filesystem-path instruction. UseLearnStack-Hub/so copied commands and path checks work on case-sensitive systems.As per coding guidelines and retrieved learnings, sibling filesystem paths must use exact
LearnStack/andLearnStack-Hub/capitalization.Proposed fix
-> **The agent runs from `learnstack-hub` root.** +> **The agent runs from `LearnStack-Hub` root.**🤖 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 `@docs/roadmap/P02c-1-implementation-prompt.md` at line 6, Update the working-repository path reference in the prompt to use the exact case-sensitive name LearnStack-Hub/ instead of learnstack-hub, while preserving the existing sibling LearnStack/ path.Sources: Coding guidelines, Learnings
docs/roadmap/p02c-2-internal-api-and-contract.md-37-37 (1)
37-37: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winQualify the no-configuration-secret claim.
Line 37 says that none of the three secrets appears in configuration files.
.env.examplecontains the development-onlyHUB_INTERNAL_API_HMAC_KEYplaceholder. State that production secret material must not appear in configuration, logs, errors, or traces, and identify the example value as non-secret development data.🤖 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 `@docs/roadmap/p02c-2-internal-api-and-contract.md` at line 37, Update the documentation statement around the three secrets to qualify that production secret material must not appear in configuration files, logs, error messages, or traces. Explicitly identify the .env.example HUB_INTERNAL_API_HMAC_KEY placeholder as non-secret, development-only data.docs/roadmap/p02c-1-hub-domain-core.md-28-28 (1)
28-28: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winState the pipeline count as behaviors plus handler.
Line 28 calls the chain “six-step” and includes six behaviors plus
Handler.docs/architecture/cross-cutting-foundation.mdLine 47 counts six behaviors and Line 59 listsHandlerseparately. Change this to “six MediatR behaviors, followed by the Handler” to prevent test and implementation count mismatches.🤖 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 `@docs/roadmap/p02c-1-hub-domain-core.md` at line 28, Update the MediatR pipeline description to say “six MediatR behaviors, followed by the Handler,” while preserving the listed behavior order and existing status details.docs/roadmap/p02c-0-repository-bootstrap.md-50-50 (1)
50-50: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReconcile the architecture-test count.
The Scope section names three placeholder tests. The Deliverables section claims four architecture tests and one smoke test. Name the fourth architecture test or change the count. This delivery record should define one test inventory.
Also applies to: 64-64
🤖 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 `@docs/roadmap/p02c-0-repository-bootstrap.md` at line 50, Reconcile the test inventory in the roadmap document: align the Deliverables count with the three tests listed in the Scope, or explicitly name the missing fourth architecture test. Ensure the Scope and Deliverables sections consistently describe the same architecture-test and smoke-test totals.docs/architecture/entitlement-projection.md-142-149 (1)
142-149: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the superseded fallback rule.
Line 149 still says that the system enters read-only mode past
grace_until. Lines 142-147 define the current per-feature fail-open or fail-closed behavior and the no-throw contract. Remove the old sentence so readers do not implement the superseded mode.🤖 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 `@docs/architecture/entitlement-projection.md` around lines 142 - 149, Remove the superseded “Past `grace_until` → read-only mode” sentence from the `HubEntitlementProvider` unreachable read-path documentation, while preserving the current per-feature-key fail-open/fail-closed behavior and no-throw contract.docs/architecture/repository-layout.md-3-6 (1)
3-6: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse absolute URLs for cross-repository documentation links.
The text says that cross-repository links use
../../../LearnStack/.... Replace this with the absolute GitHub URL convention. Keep relative paths only for filesystem paths or shell commands.As per coding guidelines, cross-repository Markdown links must use absolute GitHub URLs; sibling-relative paths are reserved for shell commands and filesystem paths.
🤖 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 `@docs/architecture/repository-layout.md` around lines 3 - 6, Update the cross-repository link guidance in the repository-layout documentation to require absolute GitHub URLs instead of sibling-relative ../../../LearnStack/... links. Preserve relative paths only for filesystem paths and shell commands, and retain the documented repository capitalization.Source: Coding guidelines
docs/architecture/repository-layout.md-12-12 (1)
12-12: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language identifier to the tree fence.
markdownlint-cli2reports MD040 at Line 12. Addtextto the opening fence.🤖 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 `@docs/architecture/repository-layout.md` at line 12, Update the code fence in the repository layout documentation to specify the text language identifier on its opening fence, resolving the MD040 markdownlint violation.Source: Linters/SAST tools
docs/architecture/repository-layout.md-119-122 (1)
119-122: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClassify
PlanTieras an enum, not an aggregate.
docs/modules/plans.mdLine 15 definesPlanTieras an enum. The planned topology lists it underAggregates. List onlyPlan, or add a separate enum/value-type column.🤖 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 `@docs/architecture/repository-layout.md` around lines 119 - 122, Update the Plans entry in the repository layout so the aggregate column lists only Plan, removing PlanTier from the aggregate classification. Preserve the existing module and specification status information.docs/modules/README.md-39-41 (1)
39-41: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMatch the audit-coverage statement to the module documents.
This section says every module document has a MUST/SHOULD/MAY table.
docs/modules/entitlements.mdLines 54-56 uses prose for its Audit coverage section. Add the matrix there, or change this statement to allow prose.🤖 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 `@docs/modules/README.md` around lines 39 - 41, Align the “Audit-coverage matrices” statement with the module documentation by updating docs/modules/entitlements.md’s “Audit coverage” section to use the same MUST/SHOULD/MAY matrix format, or revise the statement to explicitly permit prose; keep the documented audit requirements consistent with the existing entitlements content.
🧹 Nitpick comments (4)
docs/roadmap/hub-marketplace.md (2)
3-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLink the migrated LearnStack roadmap file.
This paragraph names a sibling-repository document without a navigable link. Replace the bare filename with the canonical absolute GitHub URL.
Proposed documentation fix
-> `phase-12-hub-marketplace.md`, which is now a pointer at this file. +> [`phase-12-hub-marketplace.md`](https://github.com/HodeTech/LearnStack/blob/main/docs/roadmap/phase-12-hub-marketplace.md), which is now a pointer at this file.As per coding guidelines, cross-repo references to LearnStack documentation must use absolute GitHub URLs; sibling-relative paths are reserved for shell commands and filesystem paths.
🤖 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 `@docs/roadmap/hub-marketplace.md` around lines 3 - 6, Update the introductory paragraph near “This is the authoritative plan” to replace the bare LearnStack filename phase-12-hub-marketplace.md with its canonical absolute GitHub URL, preserving the surrounding wording and formatting.Source: Coding guidelines
68-90: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssign the collision-resolution ADR to the cross-repository decision owner.
This ADR changes ADR-0034's Hub/LearnStack invariant. The roadmap says an ADR is required but does not identify
LearnStack/docs/decisions/as the authoritative location. A Hub-only decision would create a second source of truth for a cross-repository boundary. State the owner and require both roadmaps to reference the accepted ADR.As per coding guidelines, cross-cutting architectural decisions belong in
../LearnStack/docs/decisions/; Hub-only decisions use theHUB-NNNNseries.Also applies to: 198-199
🤖 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 `@docs/roadmap/hub-marketplace.md` around lines 68 - 90, The roadmap’s collision-resolution ADR must be owned by the cross-repository decision owner and recorded authoritatively in ../LearnStack/docs/decisions/ rather than as a Hub-only decision. Update the “unresolved collision with ADR-0034” section to state this location and require both Hub and LearnStack roadmaps to reference the accepted ADR; do not use the HUB-NNNN series.Source: Coding guidelines
docs/roadmap/p02c-5-custom-domain-lifecycle.md (1)
199-202: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDocument Hub-first coordination for the paired implementation.
“Same session” does not identify which repository owns the canonical contract. State that the Hub PR opens first, the LearnStack PR references the Hub commit, and both PRs merge in the same session.
Based on learnings, cross-repository changes must coordinate both PRs: the Hub PR opens first with the canonical contract, the LearnStack PR references its commit, and both merge in the same session.
🤖 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 `@docs/roadmap/p02c-5-custom-domain-lifecycle.md` around lines 199 - 202, Update the cross-repository coordination note around the LearnStack-side event consumer, host-mappings handler, and platform_host_to_tenant writes to state that the Hub PR opens first and establishes the canonical contract, the LearnStack PR references the Hub commit, and both PRs merge in the same session.Source: Learnings
docs/roadmap/P02c-1-implementation-prompt.md (1)
137-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRequire one co-author trailer per contributing agent.
If multiple agents materially contribute, this singular rule can omit contributors. Require one
Co-Authored-Bytrailer for each materially contributing agent and use each agent's actual identity.Based on learnings, commit attribution must include one
Co-Authored-Bytrailer per materially contributing agent.Suggested wording
-**English docs; Conventional Commits; AI co-author trailer** (`Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>`). +**English docs; Conventional Commits; one AI co-author trailer per materially contributing agent** (`Co-Authored-By: <agent identity>`).🤖 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 `@docs/roadmap/P02c-1-implementation-prompt.md` at line 137, Update the commit-attribution guidance in the roadmap document to require one Co-Authored-By trailer for every materially contributing agent, using each agent’s actual identity rather than a single fixed identity. Preserve the existing English documentation and Conventional Commits requirements.Source: Learnings
🤖 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 @.claude/skills/add-hub-module/SKILL.md:
- Line 18: Update the module description in the add-hub-module skill to remove
the normative link and dependency on LearnStack's add-backend-module skill.
Describe or reference the stable module pattern using this repository's
.claude/skills/ content or a standard architecture document, while preserving
the Hub-specific exclusions such as tenant isolation.
In `@AGENTS.md`:
- Around line 18-24: Remove the duplicated shared skill-catalogue, workflow,
sibling-path, URL, and entry-point guidance from AGENTS.md. Keep only the
agent-specific trailer differences described by the file, and preserve the
redirect structure; make any shared-guidance updates in CLAUDE.md instead.
In `@backend/src/Core/LearnStack.Hub.Api/Program.cs`:
- Around line 7-9: Update the pipeline documentation comment near the six-step
behavior contract to state that Hub includes the OutboxFlushBehavior shell,
while still excluding TenantContextBehavior. Ensure the wording does not imply
that OutboxFlushBehavior is absent or may be omitted.
In `@CLAUDE.md`:
- Line 58: Replace cross-repository LearnStack documentation references with
absolute GitHub URLs: update CLAUDE.md lines 58-58 to state that skills cite
LearnStack standards and ADRs by absolute GitHub URL, update
.claude/skills/README.md lines 5-5 to use absolute URLs instead of sibling
paths, and update .github/pull_request_template.md lines 5-5 to use an absolute
ADR URL placeholder. Retain sibling-relative paths only for shell commands and
filesystem locations.
In `@docs/glossary.md`:
- Line 17: Align the glossary entry with the defined packet scope: either remove
LicenseKey from the Entitlement Aggregate inputs and recomputation triggers for
the P02c-1 claim, or explicitly state that LicenseKey belongs to a later packet
and update the authoritative entitlement projection and recomputation-contract
documents consistently. Keep the P02c-1 dependency from including the P02c-6
module unless those documents are intentionally revised together.
In `@docs/modules/entitlements.md`:
- Line 50: Update the EntitlementProjection_Shape_IsStable documentation to make
the contract snapshot mandatory in both LearnStack and Hub repositories. Name
the checked-in entitlement-v1.schema.json and corresponding snapshot test in
each repository, and state that both snapshots must be updated together to
preserve the LearnStack↔Hub wire shape.
In `@docs/roadmap/hub-billing.md`:
- Around line 15-18: Update the P02c-1 and P02c-2 completion wording in the
roadmap to match the authoritative status ledger: replace claims that P02c-1
“shipped” or that usage is already handled by P02c-2 with neutral “defines” and
future-oriented “will be handled by P02c-2” wording, including the corresponding
passage at the other referenced section.
- Around line 135-143: Update the Naming section to place Stripe SDK adapters in
LearnStack.Hub.Infrastructure.Stripe and Iyzico SDK adapters in
LearnStack.Hub.Infrastructure.Iyzico, with the manual adapter documented
separately under its own project. Preserve the distinction from LearnStack
core’s IPaymentProvider and ensure the documented project names comply with the
SDK import boundary.
In `@docs/roadmap/p02c-2-internal-api-and-contract.md`:
- Around line 31-37: The authentication section should treat ADR-0034 as the
current authority instead of describing the layers as unchanged by ADR-0034 or
relying on ADR-0019. Update the citation and wording around the mTLS, RS256 JWT,
and HMAC requirements while preserving that three-layer chain and its stated
secret-handling and rejection behavior.
- Around line 51-56: Clarify the durable dispatch contract in the projection
recomputation section and the requirement at line 89: specify whether a single
outbox record coordinates both the HTTP projection push and the
learnstack.hub.entitlement event, or whether each operation has its own outbox
record and handler. Define independent idempotency and retry behavior for both
deliveries, ensuring neither can be omitted and that partial failures converge
safely.
In `@docs/roadmap/p02c-4-operator-portal.md`:
- Around line 38-43: Align the MFA documentation and integration-test references
with the configuration model and authoritative realm export defined in
infra/keycloak/README.md: consistently state whether CONFIGURE_TOTP applies to
every user or an operator realm role, identify the single owning export file,
and reference one integration-test path that validates that export and enforced
OTP behavior.
In `@docs/roadmap/p02c-5-custom-domain-lifecycle.md`:
- Around line 35-43: The custom-domain lifecycle state diagram must not
transition from Verifying directly to Active on MarkVerified. Add an explicit
propagation state after certificate verification, require successful
host-mapping push and secret replication acknowledgements before entering
Active, and emit the activated event only on that final transition; update the
corresponding lifecycle descriptions and examples consistently.
- Around line 84-87: Update the recurring verification job description to select
validation by challenge mode: the default DNS-01 flow must poll the
_acme-challenge TXT record, while the CNAME mode must perform CNAME
verification. Specify coverage for both TXT and CNAME paths in the associated
tests.
- Around line 219-223: Update the custom-domain lifecycle documentation around
the inbound POST endpoint to name the approved LearnStack-to-Hub adapter, using
IHubTenantSync if that is the applicable contract, and define its
request/response contract before implementation. Make the Hub client boundary
explicit while preserving the existing Admin Studio proxy flow.
In `@docs/roadmap/p02c-7-exit-gate.md`:
- Around line 45-50: Update the LearnStack projection acceptance logic described
in the PUT /api/internal/tenants/{id}/entitlements flow: accept higher
generations, accept equal-generation payloads only when they are identical to
the cached projection, and return a conflict for divergent equal-generation
payloads. Add coverage for a divergent equal-generation replay while preserving
idempotent duplicate delivery.
- Around line 76-94: Clarify Gate 4’s test-count wording so “three” explicitly
refers to the three authentication legs, not three total test cases. Require
separate negative tests for every listed JWT variant (absent, expired, wrong
audience, replayed jti) and HMAC variant (absent, tampered body), across both
internal surfaces and directions.
In `@docs/roadmap/README.md`:
- Line 9: Update the roadmap bullet describing the P02c-1 unfreeze condition to
include both requirements: the ADR-0035 billing or plan-gating trigger and
reconciliation with ADR-0033 and ADR-0034 before frozen P02c-1 code can merge.
Do not narrow the gate to ADR-0035 alone.
---
Minor comments:
In @.claude/skills/commit-and-pr/SKILL.md:
- Around line 43-51: Update the commit trailer guidance in the commit workflow
example to require the actual materially contributing agent’s identity rather
than hard-coding Claude Opus 5 or only mentioning Codex. Preserve the HEREDOC
format and instruct contributors to add one Co-Authored-By trailer for each
agent that materially contributed.
In @.github/workflows/ci.yml:
- Around line 174-178: Update the link-audit comment near the link validation
logic to state that ../LearnStack/ documentation paths are rejected, not
skipped. Clarify that documentation links must use absolute GitHub URLs and that
sibling-relative paths are reserved for filesystem or shell usage; keep the
audit behavior in the relevant validation block unchanged.
In `@docs/architecture/entitlement-projection.md`:
- Around line 142-149: Remove the superseded “Past `grace_until` → read-only
mode” sentence from the `HubEntitlementProvider` unreachable read-path
documentation, while preserving the current per-feature-key
fail-open/fail-closed behavior and no-throw contract.
In `@docs/architecture/repository-layout.md`:
- Around line 3-6: Update the cross-repository link guidance in the
repository-layout documentation to require absolute GitHub URLs instead of
sibling-relative ../../../LearnStack/... links. Preserve relative paths only for
filesystem paths and shell commands, and retain the documented repository
capitalization.
- Line 12: Update the code fence in the repository layout documentation to
specify the text language identifier on its opening fence, resolving the MD040
markdownlint violation.
- Around line 119-122: Update the Plans entry in the repository layout so the
aggregate column lists only Plan, removing PlanTier from the aggregate
classification. Preserve the existing module and specification status
information.
In `@docs/modules/README.md`:
- Around line 39-41: Align the “Audit-coverage matrices” statement with the
module documentation by updating docs/modules/entitlements.md’s “Audit coverage”
section to use the same MUST/SHOULD/MAY matrix format, or revise the statement
to explicitly permit prose; keep the documented audit requirements consistent
with the existing entitlements content.
In `@docs/roadmap/p02c-0-repository-bootstrap.md`:
- Line 50: Reconcile the test inventory in the roadmap document: align the
Deliverables count with the three tests listed in the Scope, or explicitly name
the missing fourth architecture test. Ensure the Scope and Deliverables sections
consistently describe the same architecture-test and smoke-test totals.
In `@docs/roadmap/p02c-1-hub-domain-core.md`:
- Line 28: Update the MediatR pipeline description to say “six MediatR
behaviors, followed by the Handler,” while preserving the listed behavior order
and existing status details.
In `@docs/roadmap/P02c-1-implementation-prompt.md`:
- Line 6: Update the working-repository path reference in the prompt to use the
exact case-sensitive name LearnStack-Hub/ instead of learnstack-hub, while
preserving the existing sibling LearnStack/ path.
In `@docs/roadmap/p02c-2-internal-api-and-contract.md`:
- Line 37: Update the documentation statement around the three secrets to
qualify that production secret material must not appear in configuration files,
logs, error messages, or traces. Explicitly identify the .env.example
HUB_INTERNAL_API_HMAC_KEY placeholder as non-secret, development-only data.
In `@docs/roadmap/p02c-4-operator-portal.md`:
- Around line 122-131: Standardize all repository-root references to the
operator portal on frontend/apps/operator-portal, including the later
apps/operator-portal mention in this section. If retaining apps/operator-portal,
explicitly identify it as frontend-workspace-relative.
In `@README.md`:
- Around line 30-34: Add the text language identifier to the directory-layout
fenced block in README.md by changing its opening fence to use ```text, while
leaving the directory tree content unchanged.
---
Nitpick comments:
In `@docs/roadmap/hub-marketplace.md`:
- Around line 3-6: Update the introductory paragraph near “This is the
authoritative plan” to replace the bare LearnStack filename
phase-12-hub-marketplace.md with its canonical absolute GitHub URL, preserving
the surrounding wording and formatting.
- Around line 68-90: The roadmap’s collision-resolution ADR must be owned by the
cross-repository decision owner and recorded authoritatively in
../LearnStack/docs/decisions/ rather than as a Hub-only decision. Update the
“unresolved collision with ADR-0034” section to state this location and require
both Hub and LearnStack roadmaps to reference the accepted ADR; do not use the
HUB-NNNN series.
In `@docs/roadmap/P02c-1-implementation-prompt.md`:
- Line 137: Update the commit-attribution guidance in the roadmap document to
require one Co-Authored-By trailer for every materially contributing agent,
using each agent’s actual identity rather than a single fixed identity. Preserve
the existing English documentation and Conventional Commits requirements.
In `@docs/roadmap/p02c-5-custom-domain-lifecycle.md`:
- Around line 199-202: Update the cross-repository coordination note around the
LearnStack-side event consumer, host-mappings handler, and
platform_host_to_tenant writes to state that the Hub PR opens first and
establishes the canonical contract, the LearnStack PR references the Hub commit,
and both PRs merge in the same session.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 07afafc8-31c7-476e-bba4-7acfcb481fbd
📒 Files selected for processing (57)
.claude/skills/README.md.claude/skills/add-architecture-test/SKILL.md.claude/skills/add-ef-migration/SKILL.md.claude/skills/add-feature-key/SKILL.md.claude/skills/add-hub-module/SKILL.md.claude/skills/add-integration-event/SKILL.md.claude/skills/code-review/SKILL.md.claude/skills/commit-and-pr/SKILL.md.claude/skills/implement-task/SKILL.md.claude/skills/local-dev-setup/SKILL.md.claude/skills/standards-check/SKILL.md.claude/skills/start-task/SKILL.md.claude/skills/update-glossary/SKILL.md.claude/skills/wire-cross-cutting-foundation/SKILL.md.claude/skills/write-adr/SKILL.md.env.example.github/pull_request_template.md.github/workflows/ci.ymlAGENTS.mdCLAUDE.mdCONTRIBUTING.mdREADME.mdbackend/src/Core/LearnStack.Hub.Api/Program.csbackend/src/Modules/README.mdbackend/tests/LearnStack.Hub.Tests.Architecture/HubBoundaryTests.csbackend/tests/LearnStack.Hub.Tests.Architecture/RepositoryLayoutTests.csdocs/README.mddocs/architecture/README.mddocs/architecture/contract-with-learnstack.mddocs/architecture/cross-cutting-foundation.mddocs/architecture/entitlement-projection.mddocs/architecture/module-topology.mddocs/architecture/repository-layout.mddocs/decisions/README.mddocs/glossary.mddocs/modules/README.mddocs/modules/entitlements.mddocs/modules/plans.mddocs/modules/subscriptions.mddocs/modules/tenant-lifecycle.mddocs/operations/README.mddocs/roadmap/P02c-1-implementation-prompt.mddocs/roadmap/README.mddocs/roadmap/hub-billing.mddocs/roadmap/hub-marketplace.mddocs/roadmap/p02c-0-repository-bootstrap.mddocs/roadmap/p02c-1-hub-domain-core.mddocs/roadmap/p02c-2-internal-api-and-contract.mddocs/roadmap/p02c-3-learnstack-integration.mddocs/roadmap/p02c-4-operator-portal.mddocs/roadmap/p02c-5-custom-domain-lifecycle.mddocs/roadmap/p02c-6-license-key.mddocs/roadmap/p02c-7-exit-gate.mdinfra/compose/README.mdinfra/compose/dev.ymlinfra/keycloak/README.mdscripts/seed.sh
… P02c-2 P02c-1 merged to `main` while this branch was open, so the branch's central claim — that the Hub domain core "exists and carries the Hub domain core, but it is not merged" — stopped being true. Rather than pick a winner between the two, this reconciles them on the distinction that actually matters: **the freeze is on the track's forward motion, not on the merged artefact.** That reframe came out of a review of PR #1's 221 files against the three decisions the restructure moved. It implements none of them: - ADR-0034 (endpoint set, auth chain) — no LearnStackApiClient, no hosted /api/internal/*, both deferred to P02c-2. Nothing to be stale. - ADR-0033 (audit durability) — AuditLogBehavior is an explicit shell, and the Hub has no IAuditStore and no audit_log table at all. - ADR-0035 (demand-gating) — a scheduling decision, not code. And the one cross-repo artefact it does ship was already correct: entitlement-v1.schema.json carries `grace_until` and `generation` with the wire names `tier` and `expires_at`, and carries no certificate, key, host or domain field. That is the same shape LearnStack's side was aligned to, so the snapshot test the freeze was written to protect is the thing that already matched. Freezing the merged artefact would therefore have bought nothing and cost the SharedKernel reconciliation against LearnStack Packet 3b — which, unlike LearnStack's, already has four modules of consumers and grows with every packet built on top. Four conflicts, resolved: - CLAUDE.md and docs/roadmap/README.md — rewritten to the reframe rather than to either side. P02c-1 ✅ Shipped; P02c-2 onward ⏸ Frozen on the ADR-0035 trigger. main's "Deferred from P02c-1" follow-ups are preserved — they are real tracked work, not status prose. - Program.cs — main's real implementation kept, its TODO replaced. The old one said "wire the four-endpoint contract surface" and named four of ten crossings, omitting host-mappings, which is the endpoint that exists so certificate material stops riding the entitlement payload. An implementer following it would have rebuilt the defect ADR-0034 removed. - scripts/seed.sh — main's relaxed pre-flight kept; it is correct now that the in-process seeder landed and needs only Postgres. One defect merged silently, with no conflict marker, and is fixed here: ci.yml's header said `backend-integration` is `if: false` until P02c-2 while the job below it ran ungated. P02c-1 shipped the first Testcontainers tests and activating it there is the right call, so the five carriers that said P02c-2 owns the activation are corrected — except P02c-0's delivery record, which was true when written. Also corrected: three source comments claiming Hub drops "the two tenant-isolation steps" from "core's 8-step pipeline". Hub drops exactly one behavior, TenantContextBehavior. Seven behaviors against six, with the convention named so the arithmetic stops going wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/Core/LearnStack.Hub.Api/Program.cs (1)
31-38: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFail closed for an invalid deployment mode.
When
Hub:DeploymentModeis missing or invalid, this code selectsDeploymentMode.Developmentand can registerNoOpErrorTrackerin production.Enum.TryParsealso accepts undefined numeric values, which bypass the intended validation. Allow the missing value only whenbuilder.Environment.IsDevelopment(); otherwise require a definedDeploymentModeand throw on invalid input.🤖 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 `@backend/src/Core/LearnStack.Hub.Api/Program.cs` around lines 31 - 38, Update the deploymentMode initialization at the composition root to fail closed: allow a missing Hub:DeploymentMode only when builder.Environment.IsDevelopment(), otherwise throw. Parse configured values case-insensitively and require Enum.IsDefined for the parsed DeploymentMode so undefined numeric values are rejected; do not fall back to Development for invalid or missing production configuration.
🧹 Nitpick comments (1)
backend/tests/LearnStack.Hub.Tests.Architecture/PipelineOrderTests.cs (1)
8-28: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winVerify the actual MediatR registration order.
The current test checks only
CanonicalBehaviorOrder. It does not callAddHubMediatRPipelineor execute a request, so the registered behavior order can diverge while the test and XML documentation continue to pass.
backend/tests/LearnStack.Hub.Tests.Architecture/PipelineOrderTests.cs#L8-L28: add a service-provider or probe-request assertion for the resolved behavior order.backend/src/Core/LearnStack.Hub.Application/Pipeline/MediatRPipelineRegistration.cs#L12-L14: narrow the XML claim until the DI/execution assertion exists.🤖 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 `@backend/tests/LearnStack.Hub.Tests.Architecture/PipelineOrderTests.cs` around lines 8 - 28, The test at backend/tests/LearnStack.Hub.Tests.Architecture/PipelineOrderTests.cs:8-28 must verify the actual order produced by AddHubMediatRPipeline, using a service provider or probe request to assert resolved/executed behaviors match the canonical sequence; checking only MediatRPipelineRegistration.CanonicalBehaviorOrder is insufficient. Until that runtime assertion exists, narrow the XML documentation claim in backend/src/Core/LearnStack.Hub.Application/Pipeline/MediatRPipelineRegistration.cs:12-14 so it describes the declared canonical order without claiming the registered pipeline is verified.
🤖 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 `@docs/roadmap/p02c-1-hub-domain-core.md`:
- Around line 123-132: Update the closing P02c-2 gate statement to require both
conditions before unfreezing: the ADR-0035 billing or plan-gating trigger must
fire and the contract surface must comply with ADR-0034. Keep P02c-2 and
downstream work explicitly frozen until both conditions are satisfied.
In `@docs/roadmap/README.md`:
- Around line 19-21: Reconcile P02c-1 to one authoritative merge state across
all records: update the packet table at docs/roadmap/README.md:19-21, the
closure record and ledger date at docs/roadmap/README.md:171-171, and the
status/merge record at docs/roadmap/p02c-1-hub-domain-core.md:3-5 so they
consistently reflect whether the packet is merged or remains branch-held and
frozen. Do not leave conflicting shipped and unmerged claims.
---
Outside diff comments:
In `@backend/src/Core/LearnStack.Hub.Api/Program.cs`:
- Around line 31-38: Update the deploymentMode initialization at the composition
root to fail closed: allow a missing Hub:DeploymentMode only when
builder.Environment.IsDevelopment(), otherwise throw. Parse configured values
case-insensitively and require Enum.IsDefined for the parsed DeploymentMode so
undefined numeric values are rejected; do not fall back to Development for
invalid or missing production configuration.
---
Nitpick comments:
In `@backend/tests/LearnStack.Hub.Tests.Architecture/PipelineOrderTests.cs`:
- Around line 8-28: The test at
backend/tests/LearnStack.Hub.Tests.Architecture/PipelineOrderTests.cs:8-28 must
verify the actual order produced by AddHubMediatRPipeline, using a service
provider or probe request to assert resolved/executed behaviors match the
canonical sequence; checking only
MediatRPipelineRegistration.CanonicalBehaviorOrder is insufficient. Until that
runtime assertion exists, narrow the XML documentation claim in
backend/src/Core/LearnStack.Hub.Application/Pipeline/MediatRPipelineRegistration.cs:12-14
so it describes the declared canonical order without claiming the registered
pipeline is verified.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d628f863-5891-41a4-8fad-f6cbb209047d
📒 Files selected for processing (13)
.github/workflows/ci.ymlCLAUDE.mdCONTRIBUTING.mdREADME.mdbackend/src/Core/LearnStack.Hub.Api/Program.csbackend/src/Core/LearnStack.Hub.Application/LearnStack.Hub.Application.csprojbackend/src/Core/LearnStack.Hub.Application/Pipeline/MediatRPipelineRegistration.csbackend/tests/LearnStack.Hub.Tests.Architecture/HubBoundaryTests.csbackend/tests/LearnStack.Hub.Tests.Architecture/PipelineOrderTests.csdocs/architecture/repository-layout.mddocs/roadmap/README.mddocs/roadmap/p02c-1-hub-domain-core.mddocs/roadmap/p02c-2-internal-api-and-contract.md
🚧 Files skipped from review as they are similar to previous changes (7)
- backend/tests/LearnStack.Hub.Tests.Architecture/HubBoundaryTests.cs
- CONTRIBUTING.md
- .github/workflows/ci.yml
- README.md
- docs/roadmap/p02c-2-internal-api-and-contract.md
- docs/architecture/repository-layout.md
- CLAUDE.md
Standards 21's "Awaiting backfill" section now reads "Every identifier previously parked in this section has been folded into the catalogue above under its canonical name", and the Hub-side tests carry full entries there. This pointer file still told a Hub-side reader they were pending. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Caught by CodeRabbit on the PR, and it is the exact class of defect this restructure exists to eliminate: one file stating two opposite things about the same packet. `docs/roadmap/README.md` line 19 says "P02c-1 — ✅ Shipped", line 29 says "P02c-1 is **merged**", and line 165 — the artefact status ledger further down the same file — said "On branch, **frozen** … Not merged." The earlier sweep matched on "P02c-1 is frozen" and "❄️" and never saw "On branch, **frozen**". Four more carriers of the same stale state, all missed for the same reason: - The ledger row for everything after P02c-1 said "Not started" where the distinction that matters is *frozen* — not started is a schedule, frozen is a decision. - "P02c-0, P02c-1, P02c-2 and P02c-4 are unblocked by LearnStack" was written before the merge and still listed two shipped packets among the work that can proceed. - `repository-layout.md` said `backend/src/Modules/` "is still empty of module subdirectories" and that the branch filling it is frozen. Four modules are on `main`. Its tree comment and its "NOT YET ON DISK" heading said the same. Also, the governance bullet said the ADR-0035 trigger "is what governs when this plan resumes" — one condition — while CLAUDE.md states two, the second being that the contract surface is built to ADR-0034's invariants rather than an endpoint count. The bullet now names both and points at the same place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CLAUDE.md and the roadmap README both say P02c-2 unfreezes when two conditions hold; this packet's exit decision named only the ADR-0035 trigger. A reader following it would start P02c-2 with the trigger fired and build the contract surface to an endpoint count — the thing ADR-0034 exists to stop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/architecture/repository-layout.md (1)
3-6: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse absolute GitHub URLs for cross-repository documentation links.
Line 6 describes
../../../LearnStack/...as a cross-repository link. Use that path only for local filesystem or shell examples. Use an absolute GitHub URL for Markdown documentation links.Suggested wording
- with those exact capitalisations; cross-repo links from this file are written - `../../../LearnStack/...`. + with those exact capitalisations; cross-repository Markdown links use absolute + GitHub URLs. Sibling paths such as `../../../LearnStack/...` are for local + filesystem and shell examples only.As per coding guidelines,
**/*.mdfiles must use absolute GitHub URLs for cross-repository LearnStack documentation links; sibling-relative paths are allowed only for local shell commands and filesystem paths.🤖 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 `@docs/architecture/repository-layout.md` around lines 3 - 6, Update the cross-repository Markdown link guidance in repository-layout.md to use an absolute GitHub URL for LearnStack documentation references, while retaining ../../../LearnStack/... only in local filesystem or shell examples.Source: Coding guidelines
🤖 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 `@docs/architecture/repository-layout.md`:
- Around line 8-11: Reconcile the P02c-1 documentation with the implemented
state of TenantLifecycle, Plans, Subscriptions, and Entitlements: remove or
update stale “only file on disk,” “unfreezes,” “ahead of implementation,” and
“spec written” claims in docs/architecture/repository-layout.md at lines 8-11,
29-30, 78-79, 107-108, 114-123, and 144, and align docs/modules/README.md with
the four implemented module directories.
---
Outside diff comments:
In `@docs/architecture/repository-layout.md`:
- Around line 3-6: Update the cross-repository Markdown link guidance in
repository-layout.md to use an absolute GitHub URL for LearnStack documentation
references, while retaining ../../../LearnStack/... only in local filesystem or
shell examples.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6c8d0f83-5fbf-4c0e-876e-d1e62dd7989e
📒 Files selected for processing (2)
docs/architecture/repository-layout.mddocs/roadmap/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/roadmap/README.md
This was referenced Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Hub half of a two-repository roadmap restructure. Opens first per
CLAUDE.md § Cross-repo coordination — this side carries the
canonical contract shape, and the LearnStack PR references it.
Paired PR: HodeTech/LearnStack#9 — both merge in the same session.
Either side alone leaves the contract dangling.
Documentation only. No source, no schema, no CI behaviour change except
one deliberate link-audit tightening (below).
What this is
Two things, in sequence. First the Hub roadmap moved into this repository,
which owns it. Then a four-report audit of the corpus produced 151 findings
across both repos, and this PR closes the Hub's share of them.
The audit's own recurring failure mode is worth stating up front, because it
shaped how the fixes were applied: the corpus fixed the documents it touched
and left the untouched carriers of the same fact stale. Every fix here was
applied by sweeping for every carrier of a claim, not the one the finding
happened to cite. Three findings under-counted their own carriers by half or
more.
The Hub-side changes that matter
The CI link audit was broken and nobody could see it. The workflow skips
the file-existence check for cross-repo links, because CI cannot reach a
sibling checkout — but the skip pattern matched
learnstackin lowercaseonly, and bash
=~is case-sensitive with nonocasematchset. A priorcommit had recapitalised all 215 cross-repo links to
../LearnStack/..., soevery one of them fell through to a check that could not pass. Fixed, then
made moot: the links are now absolute URLs and the exemption is removed
entirely, so a relative cross-repo link fails the build instead of reaching
a reader.
222 cross-repo links became absolute URLs. LearnStack's Documentation
Standards § Layout says cross-repo references use absolute URLs; this
repository used sibling-relative paths at four different
../depths. Everyone 404s on github.com, and moving a document one directory deeper silently
broke all of its cross-repo links. Link targets only —
cd ../LearnStackandfilesystem references in prose keep the relative path, because those are paths,
not links. An earlier, broader sweep rewrote both and produced
cd https://github.com/...; it was discarded rather than patched.The contract pointer file was pointing at the wrong authority.
docs/architecture/contract-with-learnstack.mdis pointer-only and is what aHub-side agent reads instead of the LearnStack corpus — so a stale pointer
here is worse than a stale statement anywhere else. It named ADR-0019 as the
source for "the endpoint set + auth chain", one row for two topics that
ADR-0034 splits. It also said "the two LearnStack → Hub endpoints" and "the two
Hub → LearnStack endpoints" — a count that was wrong under ADR-0019 (six paths)
and wrong under ADR-0034 (nine). Counts removed rather than corrected: a
pointer file that carries a number acquires the maintenance obligation it
exists to avoid.
P02c-5 was blocked on the wrong phase. The blocking table pointed it at
LearnStack Phase 11 for the host-mapping handler; LearnStack's own
27-custom-domain-tls.md § 11puts that handler in Phase 02c and only the ACMEautomation in Phase 11. Split into two rows, and the Phase 11 row carries an
explicit "P02c-5 does not wait on it" — without that clause the split
silently re-gates the packet by a different route.
P02c-6 gained the phone-home client certificate. ADR-0034 replaces the
per-instance API key on the LearnStack → Hub direction with the same
mTLS + JWT + HMAC chain the other direction carries. That leaves a
SelfHostedOnlineinstance unable to phone home at all unless something handsit a certificate the Hub's CA validates — and P02c-6 is the only packet that
hands a customer-run instance anything. It rides with the
.licfile: samelicense_id, same re-issue command, same revocation, so a revoked instanceloses the transport before it loses the entitlement.
Three packets cross repositories, not one. P02c-3 called itself "the only
packet that changes both repositories" while P02c-5 carries the LearnStack-side
host-mapping handler and P02c-6 the
SignedLicenseKeyEntitlementProviderskeleton. Both missing blocking-table rows added, including P02c-3's dependency
on LearnStack Phase 02b — the outbox processor and inbox guard, without
which
IUsageReportercannot dispatch through the outbox.Hub production deployment had no owner. The P02c-7 exit gate pointed at
LearnStack Phase 11, which scopes itself to LearnStack and does not cover the
Hub's own database, realm or certificate chain. A "Hub Operations" track joins
the post-MVP table — deliberately unlinked, because no plan document exists and
a dead link is worse than an unlinked row.
The pipeline arithmetic was off by one.
cross-cutting-foundation.mdsaid"LearnStack runs an 8-step pipeline … Hub runs 6-step. The two steps that drop
out are…" and then listed one. LearnStack's eight counts the Handler; Hub's six
does not. Both sides now count behaviors — seven and six — and the document
says which convention it uses.
Plus the glossary corrections (
generation's acceptance inequality wasinverted;
LicenseKeynamed a third spelling of the timestamps P02c-6 existsto reconcile), the plan editor's owner (Hub Billing, not P02c-4), soft-limit
alert provenance (produced by LearnStack, ingested by P02c-2), and Status
banners on P02c-4 through P02c-7.
Verification
A nine-check script ran after every wave against both repositories and returned
zero: relative links resolved case-sensitively (macOS APFS hides the casing bug
that breaks Linux CI), frozen delivery records byte-identical, phase-doc section
conformance, mermaid well-formedness, Markdown table column counts, no committed
file citing the gitignored
docs/analysis/, retired claims not resurfacing,sibling-path casing, and balanced code fences.
Three of those checks were added during execution, each because something got
through: a dropped closing fence passed all eight existing checks (the table
check reads an unclosed fence as "still in code" and stops inspecting); a
retired claim's carriers were under-counted for the fourth time; and the
frozen-record check compared line numbers against a hard-coded boundary that a
structural move erased — it now extracts the record by content and requires
byte-identity.
Review notes
cemililik/leakwatch(Go module path — itscanonical form is fixed by the module's own
go.mod, so rewriting the installcommand can break
go installrather than redirect it), the Homebrew tap, and@cemililikin CODEOWNERS, which names a person rather than a repository.ICacheService.RemoveByPrefixAsyncleaves the port or becomes ageneration-key pattern. ADR-0035 permits both, every downstream document
survives either, and Phase 02a Packet 5 owns it.
🤖 Generated with Claude Code
Summary by Sourcery
Own the Hub roadmap and contract documentation in this repository, align cross-repo contracts and invariants with updated LearnStack ADRs, and tighten link/audit and sibling-layout rules while clarifying the Hub’s structural deltas from LearnStack core.
Enhancements:
CI:
Documentation:
Summary by CodeRabbit
Documentation
CI and Tooling