docs(demo-client): consolidate docs and add call-flow diagrams - #149
Conversation
The demo-client documentation was split across three trees and addressed its integrator audience in commands that audience cannot run. This consolidates it into the module that owns it. - Replace the two agent-facing executor invocations with the ./mvnw form the project's own CI workflow already uses. - Move both doc/-layer documents under demo-client/doc/ with role-descriptive names that resolve their shared-basename collision: doc/user/demo-client.adoc -> demo-client/doc/integration-sample.adoc doc/development/demo-client.adoc -> demo-client/doc/playwright-suite.adoc No stub or redirect is left at either old path. - Draw the three browser-facing call flows the documents previously only described in prose as SVG sequence diagrams: the login round-trip, the userinfo probe and its four disclosure states, and the logout round-trip with the 401 re-probe that proves it. - Collapse the restatement the three-layer split forced into cross-references, giving each duplicated passage a single owning document: the 401-versus-302 path-based split, the no-directory-index rule, the two-session-modes and baseURL property, the npm-run-not-npx rationale, the module-layout table, the gateway-owned response envelope, and the quickstart command trio. All three audiences survive as three documents. demo-client/README.adoc stays at the module root as the front door GitHub renders and still reads standalone as a one-screen quickstart. The contributor document's standing prohibitions survive verbatim with their rationale intact. Every reference site is re-anchored: 69 link and image targets resolve with zero broken, and no file still names either old path. The plan's single test-tree touch is a class-Javadoc path pointer in BffKeycloakLoginFlow.java -- comment text only, with no test method, assertion, fixture, annotation or import touched, so the file's compiled behaviour is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6bFXVCzATZUxR4LzNtB3u
…e diagram No element in demo-client-userinfo-probe.svg carries class="note". The rule was carried over from the sequence-diagram template; the sibling diagrams either omit it (demo-client-login-flow.svg) or declare the variant they actually use (demo-client-logout-flow.svg declares .note-end and uses it). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6bFXVCzATZUxR4LzNtB3u
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cuioss-oliver, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: cuioss/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR relocates demo-client documentation references and clarifies session modes, opt-in execution, generated output, gateway behavior, logout flow, directory-index handling, and Playwright test contracts. ChangesDemo client documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Reviewer Guide 🔍(Review updated until commit 03f785f)
|
|
@coderabbitai review The earlier review was rate-limited with a 20-minute window, which has now elapsed. |
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
demo-client/README.adoc (1)
28-33: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winQualify the teardown claim for failed runs.
demo-client/doc/playwright-suite.adocLines 267-271 states that a failed test preventspost-integration-test, so the containers remain running. The command at Line 33 does not always perform teardown. State that teardown applies to successful runs and referencedemo-client/scripts/stop-dev-environment.shfor failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 73865dfb-a44e-44fd-99c1-98df1b960363
⛔ Files ignored due to path filters (3)
doc/resources/diagrams/demo-client-login-flow.svgis excluded by!**/*.svgdoc/resources/diagrams/demo-client-logout-flow.svgis excluded by!**/*.svgdoc/resources/diagrams/demo-client-userinfo-probe.svgis excluded by!**/*.svg
📒 Files selected for processing (7)
demo-client/README.adocdemo-client/doc/integration-sample.adocdemo-client/doc/playwright-suite.adocdemo-client/pom.xmldoc/development/README.adocdoc/user/README.adocintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffKeycloakLoginFlow.java
Addresses the CodeRabbit review on PR #149. All three claims were verified against the sources before fixing. - Qualify the teardown claim: the e2e-demo profile tears down only on the SUCCESS path. frontend-maven-plugin's npm goal aborts on a non-zero exit, so Maven never reaches post-integration-test. This is a deliberate local behaviour already recorded in demo-client/pom.xml and playwright-suite.adoc; the README simply over-claimed it. - Document the three environment variables a standalone 'npm run test' needs. playwright.config.js hard-requires KEYCLOAK_HOST_URL, PLAYWRIGHT_BASE_URL_SERVER and PLAYWRIGHT_BASE_URL_COOKIE, and only the POM's playwright-test execution supplied them, so the documented iterate loop could not run as written. - Limit the npm opt-in claim to Maven execution: the loop three lines above invokes npm directly, so 'nothing npm-touching runs outside -Pe2e-demo at all' was false as written. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6bFXVCzATZUxR4LzNtB3u
Triage dispositionsIn reply to comment_id:
|
…emo profile Both documents offered "run any clean build" as the remedy for the container stack a failing suite deliberately leaves up. That teardown does not exist outside the profile: demo-client/pom.xml declares no module-level <build>, and the exec-maven-plugin execution carrying stop-dev-environment-pre-clean sits only inside <profile><id>e2e-demo</id>. A plain `./mvnw clean` therefore tears nothing down and leaves keycloak, api-sheriff and api-sheriff-cookie holding ports 1443, 10443 and 10445 while the reader believes the stack is gone. Both sites now name `./mvnw clean -Pe2e-demo -pl demo-client` and state that the teardown lives inside the profile. Activating the profile is sufficient on its own: it sets skipPlaywrightTests=false in its own <properties>, overriding the module default that otherwise skips the execution. The unqualified wording was the more misleading for sitting a few lines from the README's own profile-boundary explanation, and it contradicted the lifecycle table in playwright-suite.adoc, which already scopes pre-clean to the profile. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6bFXVCzATZUxR4LzNtB3u
|
/review |
1 participant
Summary
Consolidates the
demo-clientdocumentation under the module that owns it, replaces the twoagent-facing executor invocations on reader-facing surfaces with the
./mvnwform a downstreamintegrator can actually run, and adds three SVG sequence diagrams for the call flows the docs
previously described only in prose.
Documentation-and-diagram change. No production source and no test logic changed — see the reviewer
note immediately below for the one file under a test tree and exactly what happened to it.
Reviewer note — the single file touched under a test tree
integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffKeycloakLoginFlow.javais the only file in this PR under a test tree, and the change is a class-Javadoc path pointer,
nothing else: inside the class comment, the token
{@code doc/development/demo-client.adoc}became{@code demo-client/doc/playwright-suite.adoc}, following the document move described below. Oneline, inside a comment block.
No test method, no assertion, no fixture, no annotation and no import was touched. The file's
compiled behaviour is unchanged.
This is stated up front rather than left for a reviewer to discover, because the plan carried an
explicit test-tree exclusion and this edit is the carve-out that exclusion obliges the PR to declare
(q-gate finding
2f1906, and the plan spec's "If the documentation cannot be made accurate withoutone, report it." clause). The alternative was leaving the pointer stale — and a dangling
cross-reference inside the very Javadoc that warns "do not read a green IT suite as proof that the
browser flow works" is a worse outcome than a one-token comment fix.
Changes
Documents moved into the owning module
Git records both as renames (~88% similarity):
doc/development/demo-client.adoc→demo-client/doc/playwright-suite.adoc— the contributor /Playwright-suite document
doc/user/demo-client.adoc→demo-client/doc/integration-sample.adoc— the integrator documentThe two documents shared the basename
demo-client.adocand would have collided under onedirectory, so each destination took a name describing what the document actually is.
demo-client/README.adocdeliberately does not move. It stays at the module root as the frontdoor GitHub renders, and is edited in place only for the executor-invocation replacement and for
cross-references into the two moved documents.
Runnable build commands on the reader-facing surface
demo-client/README.adocanddemo-client/doc/playwright-suite.adocnow carry./mvnw verify -Pe2e-demo -pl demo-client— the same goals and profile the project's owndemo-client E2E CI workflow already runs, minus the CI-only
--no-transfer-progresslog-noise flagthat a human integrator should not be told to type.
CLAUDE.md,.claude/**and.plan/**areuntouched: their executor invocations are agent-facing and correct as they stand.
Three SVG sequence diagrams
Added under the existing diagram corpus at
doc/resources/diagrams/:doc/resources/diagrams/demo-client-userinfo-probe.svg— the fetched user-info probedoc/resources/diagrams/demo-client-login-flow.svg— the login flowdoc/resources/diagrams/demo-client-logout-flow.svg— the logout flowEach arrow carries its concrete path; each diagram is theme-neutral, matching the existing corpus;
each was rasterised against both GitHub themes and read back before being committed. The consuming
documents under
demo-client/doc/embed them cross-tree asimage::../../doc/resources/diagrams/{name}.svg[…]. The diagrams are drawn from the SPA sources andthe gateway's reserved-path contract, so they honour the
401-versus-302path-based split and donot depict the info endpoint redirecting.
Inbound references repointed
doc/development/README.adocanddoc/user/README.adoc— the two index entries now referencethe moved documents instead of restating them
demo-client/pom.xml— two prose comments (the module-rationale comment and theteardown-asymmetry rationale) that no build would ever have flagged
integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffKeycloakLoginFlow.java— the Javadoc pointer covered in the reviewer note above
Test Plan
verify -Ppre-commit,then
verify)is an explicit acceptance criterion on this change rather than something the build catches: two
of the references are
pom.xmlprose comments and one is a Javadoc{@code}token, none ofwhich any build validates
Related Issues
No issue reference is recorded for this plan.
Intent
Problem. The demo-client documentation was written for a downstream integrator but did not serve one. Two reader-facing surfaces printed the agent-side executor invocation instead of a command a human can type. The two
doc/-layer documents lived outside the module that owns them, split acrossdoc/development/anddoc/user/, and that split forced each to restate the others. The login, logout and user-info call flows existed only as prose.Approach. Move the two
doc/-layer documents underdemo-client/doc/— the module already owns the SPA sources, the Playwright suite and the npm toolchain, so it should own their docs — renaming both, since both weredemo-client.adoc. Replace restatement with cross-references. Draw the three flows as SVG sequence diagrams in the existingdoc/resources/diagrams/corpus rather than a new module-local asset directory, keeping one corpus and one set of authoring conventions. Repoint every inbound reference by hand, including the ones no build validates.Non-goals. The three audiences stay three documents; collapsing quickstart, integrator and contributor into one page is a different change.
demo-client/README.adocdoes not move — it is the front door GitHub renders. Agent-facing surfaces keep their executor invocations. No production or test behaviour changes; the single test-tree file carries a Javadoc path-pointer fix only.Summary by CodeRabbit