feat(graph): phase 3 relationship graph view — Slice 1 (graph builder) - #581
Conversation
40-task plan executing the 2026-04-24 graph view design spec, organised as 6 slices that each end green and independently revertable: - Pre-flight: verify the 5 open assumptions from spec §13 before starting. - Slice 1 (9 tasks): graph builder in internal/ldap_cache/graph.go — types, per-focus BFS walks (user / group / computer / OU), per-ring + total caps, concentric layout math, cycle safety. - Slice 2 (5 tasks): /api/graph.json handler — ETag via body hash mirroring /api/search-index.json, validation, integration test. - Slice 3 (8 tasks): /graph HTML template — SSR SVG canvas, always- visible edge table, depth slider with no-JS fallback, graph CSS tokens with AAA-verified contrast. - Slice 4 (7 tasks): v2-graph.js — pan/zoom, keyboard nav, click-to- pivot, click-to-expand with aria-live announce, reduced-motion respect. - Slice 5 (5 tasks): list-page Graph mode for /users, /groups, /computers with a segmented List | Graph control. - Slice 6 (6 tasks): drawer pivots, axe-core + tab-order ratchet, README conformance statement. Each task gives exact file paths, runnable commands, actual test + implementation code (no placeholders), and an atomic signed commit with conventional-commit prefix. Self-review at the end cross-checks every spec section against a concrete task. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Pre-existing issue that Task 2's reflection-based DN helpers made more
visible: test_helpers.go was compiled into the production binary of
internal/ldap_cache because Go only excludes *_test.go files from
non-test builds, not files named test_helpers.go. That meant:
- mockLDAPClient, NewMockUser/Group/Computer, and now
newUserWithDN/newGroupWithDN/newComputerWithDN all shipped to
production.
- The reflection+unsafe writes to simple-ldap-go's unexported
Object.dn/cn fields (test-only necessity) were reachable from any
runtime code that imported the package.
Rename to test_helpers_test.go so the file is compiled only for test
builds. Verified no production code references these helpers before
the rename — grep across **/*.go minus _test.go returns no hits.
Tests still pass; build clean.
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Addresses M-4 from Task 2's code quality review. Two low-cost
assertions that catch regression classes the existing node-count /
edge-count asserts don't:
- Every edge from bob (the focus) must be EdgeMemberOf, and every
edge from the OU must be EdgeContains. Catches a bug where
direction is right but kind is swapped.
- No edge has Source == Target. Catches self-loops, which the BFS
walker could emit if the seen-map and edge-dedup interact
wrongly.
Both are trivial and don't duplicate the determinism / cycle-safety
tests planned for Tasks 7/8.
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #581 +/- ##
==========================================
+ Coverage 67.42% 68.07% +0.65%
==========================================
Files 29 29
Lines 2864 3092 +228
==========================================
+ Hits 1931 2105 +174
- Misses 799 836 +37
- Partials 134 151 +17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
…loops Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
…check Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
There was a problem hiding this comment.
Pull request overview
This PR introduces Slice 1 of the Phase 3 relationship graph view by adding an in-memory graph builder to internal/ldap_cache/, along with unit tests and supporting docs updates.
Changes:
- Add
BuildGraph(focusDN, depth)and supporting graph types (Node,Edge,GraphData, caps, layout). - Add a comprehensive unit test suite for graph building behavior, caps, cycles, and deterministic layout.
- Update graph-view spec text (ETag hashing) and add the full implementation plan document.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/ldap_cache/graph.go | New in-memory relationship graph builder, OU enumeration, caps, deterministic concentric layout. |
| internal/ldap_cache/graph_test.go | Unit tests for all focus types, caps behavior, cycle safety, and layout determinism. |
| internal/ldap_cache/test_helpers_test.go | Adds test-only helpers using reflect/unsafe to seed DNs/CNs into simple-ldap-go objects. |
| docs/superpowers/specs/2026-04-24-phase-3-graph-view-design.md | Updates ETag guidance to hash marshalled JSON body. |
| docs/superpowers/plans/2026-04-24-phase-3-graph-view.md | Adds the detailed multi-slice implementation plan. |
Comments suppressed due to low confidence (1)
internal/ldap_cache/test_helpers_test.go:143
setObjectFieldsassumes theObjectfield exists and that its unexporteddn/cnfields are present and addressable. Ifsimple-ldap-gochanges its internal struct layout, this will panic with an opaquereflect/unsafeerror.
Since this helper is intended to support many tests, it would be more maintainable to add validation (field exists, kind is string, CanAddr) and fail with a clear test error/panic message when the assumptions don’t hold.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- addOUChildren: parse DNs and compare RDN components instead of string-matching. Fixes false-negative on immediate children whose CN contains an escaped comma (e.g. "cn=Last\, First,ou=..."). - immediateOUFromDN: use go-ldap's DN serializer so escaped/multi- valued RDNs round-trip correctly instead of manual concat. - Regression test TestBuildGraph_OUFocus_EscapedCommaChild pins the escaped-comma case. Mutation-tested: temporarily reverting the DN-parsing check makes the test fail as expected. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
## Summary Slice 2 of 6 for the Phase 3 relationship graph view. Adds the JSON endpoint that the upcoming HTML/SVG slices will consume. Plan: `docs/superpowers/plans/2026-04-24-phase-3-graph-view.md` Tasks 10-14. Slice 1 (the in-memory graph builder) is already on main as #581. ## What ships - **`GET /api/graph.json?entity=<dn>&depth=<N>`** — registered in `protected` group of `internal/web/server.go`. - **Validation:** missing entity (400), invalid DN (400), non-numeric depth (400), unknown DN (404). - **Caching:** sha256-based ETag (matches `/api/search-index.json` pattern); `If-None-Match` short-circuits to 304; `Cache-Control: private, must-revalidate`. - **Default depth:** 2. Out-of-range integer values (-5, 0, 99) clamp to [1, 3] inside `BuildGraph`. - **Error hygiene:** 500 returns generic `"internal error"`; underlying error logged via zerolog with `entity` + `depth` context (no DN/internals leakage). ## Test infrastructure changes To let `internal/web/` tests seed `internal/ldap_cache.Manager` from outside the package: - **NEW `internal/ldap_cache/cachetest/`** sub-package — exports `NewUserWithDN`, `NewGroupWithDN`, `NewComputerWithDN`, `Seed`. Uses reflection+unsafe to populate the unexported `Object.dn`/`cn` fields on `simple-ldap-go` types. Test-only by convention; not imported by any production code. - **NEW `internal/ldap_cache/seed_for_tests.go`** — adds `Manager.SetUsersForTesting / SetGroupsForTesting / SetComputersForTesting` exported methods. Thin wrappers around the unexported `setAll`. `ForTesting` suffix per Go convention. ## Tests (7 total, all green) - `TestHandleGraphJSON_MissingEntity` — 400 on missing `?entity=`. - `TestHandleGraphJSON_InvalidDN` — 400 on garbage DN. - `TestHandleGraphJSON_UnknownDN` — 404 when DN not in cache. - `TestHandleGraphJSON_InvalidDepth` — 400 on non-numeric depth. - `TestHandleGraphJSON_ETagStable` — first call returns 200+ETag, second with `If-None-Match` returns 304. - `TestHandleGraphJSON_DepthClamping` — depth=0/99/-5 all return 200 with `Depth ∈ [1,3]`. - `TestGraphJSON_IntegrationUserFocus` — exercises the full stack against real OpenLDAP (skipped if no container at `127.0.0.1:1389`). ## Test plan - [x] `go test ./internal/web/ -run TestHandleGraphJSON -count=1 -v` → 6 unit tests PASS - [x] Integration test PASS against `osixia/openldap:1.5.0` with `dc=test,dc=local` - [x] `go test ./internal/ldap_cache/... -count=1` → no Slice 1 regression - [x] `golangci-lint run ./internal/web/... ./internal/ldap_cache/...` → 0 issues - [x] `go build ./...` clean - [ ] CI verification on push ## What's next Slice 3: HTML template + SSR-rendered SVG (no-JS fallback path). Slice 4: interactive canvas. Slices 5-6: list-page mode + drawer pivots + AAA parallel list view + docs.
…+ CSS) (#584) ## Summary Slice 3 of 6 for the Phase 3 relationship graph view. Adds the server-side-rendered HTML page that consumes the JSON endpoint from Slice 2. Plan: `docs/superpowers/plans/2026-04-24-phase-3-graph-view.md` Tasks 15-22. Slice 1 (in-memory builder) is on main as #581; Slice 2 (JSON endpoint) is on main as #582. ## What ships - **`GET /graph?entity=<dn>&depth=<N>`** — HTML page rendering the same graph data as `/api/graph.json` via the templ `GraphPageV2` template. - **No-JS fallback path** is the primary deliverable: depth slider has `<noscript>` Apply button, SVG nodes render at SSR-computed positions, edge table is fully functional. Slice 4 will add interactivity on top. - **WCAG 2.2 compliance baseline:** - SSR edge table mirrors the visual graph (AAA-mandated text alternative). - SVG `role="img"` + `aria-labelledby`/`aria-describedby`; nodes get `role="button"` + `tabindex="0"` + descriptive `aria-label`. - `prefers-reduced-motion: reduce` honored. - All graph color tokens hit AAA (7:1) for text and AA (3:1) for non-text per §1.4.11. ## Files - `internal/web/templates/graph_v2.templ` — full SSR template + helpers - `internal/web/graph_v2_handler.go` — `handleGraphV2` HTML handler + focus label/type helpers - `internal/web/server.go` — `/graph` route in protected group - `internal/web/static/app.css` — graph styles + light/dark tokens + motion-reduce override - `internal/web/contrast_test.go` — 6 new contrast pair assertions, fixed latent dark-block parser bug ## Tests - `TestHandleGraphV2_RendersHTML` — verifies status 200 + 4 HTML markers (`graph-canvas`, `graph-data`, `graph-table`, `Relationships: bob`). - `TestAppCSSContrastAAA` — extended with 6 graph pairs (3 light, 3 dark) covering edge/edge-focus/node-border. Latent parser bug fixed (`FindStringSubmatch` → `FindAllStringSubmatch` for dark blocks). - All Slice 1+2 tests still pass (`TestHandleGraphJSON_*`, `TestBuildGraph_*`). ## Notable design decisions - **Inline JSON via `@templ.Raw(graphInlineScript(data))`** — templ v0.3 treats `<script>` contents as opaque text, so the script element is built in Go and emitted as raw HTML outside any script block. JSON is `<`/`>`/`&`-escaped to Unicode by `json.Marshal`, so DN values cannot break out of the script tag. - **Concentric ring multiplier 150** (was 180 in plan) — keeps ring-3 nodes inside the SVG viewBox. - **`GraphPageVM.Sort*` and `BackHref` fields reserved** for Slice 5 list-page mode wiring. - **`/graph` 400/404 returns plain text** — consistent with `/api/graph.json`. Slice 5 may add a styled HTML error page. ## Test plan - [x] `go test ./internal/web/ -count=1` → ok - [x] `go test ./internal/ldap_cache/...` → no regressions - [x] `golangci-lint run ./...` → 0 issues - [x] `go build ./...` → clean - [x] `templ generate` → produces `graph_v2_templ.go` (45 KB), gitignored - [ ] CI verification on push - [ ] Visual smoke (after merge): `docker compose --profile dev up -d` then browse to `/graph?entity=<some-DN>` to confirm SSR-only rendering looks reasonable ## What's next - **Slice 4:** `static/js/v2-graph.js` — pan/zoom, keyboard navigation, click-to-pivot, depth-slider on input, edge highlighting, ARIA live announcements. - **Slice 5+6:** list-page Graph mode, drawer pivots, AAA parallel-list verification, docs.
## Summary Slice 4 of 6 for the Phase 3 relationship graph view. Adds the client-side JS layer that turns Slice 3's SSR view into an interactive canvas (pan/zoom/keyboard nav/click-to-expand) without breaking the no-JS fallback. Plan: `docs/superpowers/plans/2026-04-24-phase-3-graph-view.md` Tasks 23-29. Slices 1-3 already on main as #581 / #582 / #584. ## What ships - **`internal/web/static/js/v2-graph.js`** (~290 lines, IIFE, plain JS, CSP-safe) — pan/zoom (mouse drag, wheel+ctrl/meta, arrow keys, +/-), keyboard nav (Tab/Shift-Tab cycles nodes), click-to-pivot (node body → drawer route), click-to-expand (`+` badge → fetch `/api/graph.json?depth=1`, merge nodes/edges, announce via aria-live), depth slider auto-submit on change. - **Activates SVG nodes for interactivity** at runtime (re-adds `tabindex`, `role="button"`, "Press Enter to open" suffix that Slice 3 deliberately dropped because there was no JS handler then). - **Reads inline graph data from Slice 3's `<template id="graph-data">`** via `template.content.textContent` — works under strict `script-src 'self'` CSP. - **Renders dynamically added expandable nodes with their `+` badge** so they're discoverable with the mouse, not just the keyboard. - **Handles fetch errors gracefully** — bad responses get logged + announced "Expand failed." rather than silently no-op. - Script wired in `base_v2.templ` head with `defer`; bails immediately on pages without `#graph-data`. - **`internal/e2e/graph_test.go`** — Playwright happy-path: login → /users → grab DN → /graph?entity=…&depth=2 → assert SVG + table + parseable inline data → click expandable badge → assert aria-live announces → reduced-motion repeat. ## Notable design decisions - **No build step, no transpilation, no module system** — matches `v2-bulk.js` / `v2-drawer.js` precedent. IIFE wrapper, `'use strict'`, `var` (not `let`/`const`), `function ()` callbacks (no arrows). - **Ring multiplier 150** in `renderNode`/`renderEdge` matches Slice 3's SSR `concentricXY` so dynamically-added nodes line up with SSR-positioned ones. - **`CSS.escape(dn)` before selector use** — DNs contain commas/equals that would break attribute selectors. - **`fetch` with `credentials: 'same-origin'`** — required for the session-authed endpoint. - **Reduced-motion: nothing extra in JS** — the CSS already disables transitions; JS doesn't introduce new ones. ## Tests - Existing Slice 1-3 unit tests still pass. - `TestGraphHappyPath` (Playwright, behind `//go:build e2e`) — runs in CI. ## Test plan - [x] `go build ./...` — clean - [x] `go build -tags e2e ./internal/e2e/...` — clean - [x] `go test ./internal/web/ ./internal/ldap_cache/...` — pass, no regressions - [x] `golangci-lint run ./...` — 0 issues - [x] `node -c internal/web/static/js/v2-graph.js` — clean syntax - [ ] CI verification on push (unit + e2e) - [ ] Visual smoke (after merge): browse to `/graph?entity=<DN>` and try drag, ctrl+wheel zoom, Tab between nodes, click `+` badge to expand ## What's next - **Slice 5:** list-page Graph mode (`/users?view=graph`, `/groups?view=graph`, `/computers?view=graph`) — Tasks 30-34. - **Slice 6:** drawer "View relationships" pivot, axe-core E2E ratchet, README conformance statement — Tasks 35-40.
…t + docs) (#587) ## Summary Final slice of the Phase 3 relationship graph view. Adds the drawer entry point, the axe-core a11y ratchet, and the README conformance statement. Plan: `docs/superpowers/plans/2026-04-24-phase-3-graph-view.md` Tasks 35-40. Slices 1-5 already on main (#581 / #582 / #584 / #585 / #586). ## What ships - **"View relationships" drawer pivot** on user, group, and computer drawers (Tasks 35-36). Inserted as the second pivot after "Open full page", with a new `iconGraph` SVG (4 nodes connected as a square). Links to `/graph?entity=<DN>`. - **`internal/e2e/axe_graph_test.go`** (Task 37) — runs axe-core against `/graph?entity=<seeded-DN>&depth=2`, `/users?view=graph`, `/groups?view=graph`, `/computers?view=graph`. Uses the WCAG 2.2 AA ruleset (graph view's stated conformance level — the AAA login ratchet stays in place separately). Each subtest waits for `svg#graph-canvas` so axe sees the JS-mutated DOM. - **README accessibility section updated** (Task 39) — adds a sentence about the graph view's WCAG 2.2 AA conformance and the AAA-equivalent flat edge table fallback. ## Deferred - **Task 38 (tab-order snapshot):** plan called for adding graph pages to a "tab-order snapshot suite" — but no such suite exists in the codebase. Building one from scratch for just three pages is a separate a11y-tooling effort. Deferred — file as a future enhancement. ## Tests - All Slice 1-5 tests still pass. - New axe-core ratchet runs in CI under the `e2e` build tag. ## Test plan - [x] `go build ./...` — clean - [x] `go build -tags e2e ./internal/e2e/...` — clean - [x] `go test ./... -count=1` — all packages pass - [x] `golangci-lint run ./...` — 0 issues - [ ] CI verification on push (unit + integration + e2e) - [ ] Manual smoke (after merge): open a user drawer → click "View relationships" → confirm graph renders → use the `List | Graph` toggle to switch back ## Phase 3 complete This PR closes out the Phase 3 relationship graph view per spec `docs/superpowers/specs/2026-04-24-phase-3-graph-view-design.md`. With this merged: - `BuildGraph(focusDN, depth)` + `BuildListGraph(users, computers)` in-memory builders (Slice 1, Slice 5) - `/api/graph.json?entity=…&depth=…` JSON endpoint with ETag/304/RFC 7232 (Slice 2) - `/graph` HTML page with SSR SVG + edge table (Slice 3) - `v2-graph.js` interactive client: pan/zoom, keyboard nav, click-to-pivot, click-to-expand, depth slider auto-submit, ARIA live announce (Slice 4) - `?view=graph` list-page mode for users/groups/computers with `List | Graph` segmented toggle (Slice 5) - "View relationships" drawer pivot + axe-core ratchet + README conformance (this PR)
Summary
Slice 1 of the Phase 3 relationship graph view: the in-memory graph builder in
internal/ldap_cache/.Per plan
docs/superpowers/plans/2026-04-24-phase-3-graph-view.md, this PR implements Tasks 1-9 of 40 (Slices 2-6 — JSON endpoint, HTML template, SVG canvas, list-page mode, drawer pivots + AAA tests + docs — will land in follow-up PRs).What this ships
BuildGraph(focusDN, depth)— public entry point, dispatches to the right builder (user → group → computer → OU) or returnsErrGraphNotFound.addOUChildren— OU children enumerator (users + computers), with correct immediate-child filter.applyCaps— enforces per-ring cap (60) and total cap (200), sorted deterministically before truncation, dangling edges pruned.assignConcentric— hybrid layout: server computes(ring, angle)per node, client will scale to viewport (Slice 4).Node,Edge,GraphData,Overflow,NodeType,EdgeKind— JSON-marshal-ready with lowercase-camelCase tags.Tests (9 total, all green)
TestBuildGraph_UserFocus_Depth1— node + edge counts, edge kinds, no self-loops.TestBuildGraph_GroupFocus_Depth1— members + parent groups on ring 1.TestBuildGraph_ComputerFocus_Depth1— groups + OU on ring 1.TestBuildGraph_OUFocus_Depth1— immediate-child filter (nesteddeepuser is filtered out).TestBuildGraph_OUFocus_ComputersBranch— covers the computers branch ofaddOUChildren.TestBuildGraph_PerRingCap— 100 users → 60 survive,Overflow.Truncated == true, edges pruned.TestBuildGraph_CycleSafe— A↔B cycle terminates under 500ms.TestAssignConcentric_Deterministic— (ring, angle) stable across calls.TestAssignConcentric_EvenDistribution— gaps equal2π/Nwithin1e-9.Pure in-memory; no LDAP round-trips.
What's next
GET /graph?entity=<dn>&depth=<N>endpoint (JSON, ETag via body hash).Test plan
go test ./internal/ldap_cache/ -count=1→ okgolangci-lint run ./internal/ldap_cache/...→ 0 issues-rungo test -racepasses on the cycle-safety test