Define the shared GraphIR, source identity, and navigation contracts (MOO-68) - #68
Closed
OwenTanzer wants to merge 26 commits into
Conversation
Document the pinned starting revision, startup commands, and known nondeterministic fields, and extend the existing repo-smoke script with committed structural snapshots for all four fixtures so later modularization commits can prove they preserved analyzer behavior rather than assert it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Stand up dev/build/preview commands over the existing index.html unchanged, plus a minimal static server entry point serving the production build. Analyzer marker extraction and the app's inline script are untouched — this only proves the tooling works, so the existing single-file index.html stays available as a rollback path until module extraction lands in Commit 3. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… Commit 3) Move Parser, buildAnalysisData, calcBlast, calcHealth, runAnalysisData, createAnalysisWorkerSource, GitHub, and the rest of the analyzer's top-level names out of index.html's inline script into src/analyzer.js, a real ES module. index.html bridges its exports onto window for the still-classic app script; card/lib/analyzer.js and every Node test now import it directly instead of VM-extracting a marker-delimited block from index.html text. The analysis Web Worker's bootstrap (createAnalysisWorkerSource) used to fetch the page's own HTML and slice out the analyzer by string marker; it now fetches its own module URL via import.meta.url instead, with new internal CODEFLOW_CORE_START/END markers excluding runAnalysisData/createAnalysisWorkerSource themselves from what gets embedded into the worker (their source contains import.meta, which is a syntax error in the classic-script worker context). Verified end-to-end via headless Chromium against both the dev server and a production build. Also folds two previously-duplicated stub functions (getSecurityScanContent, isSanitizedPreviewRenderer) into the real module, closing a latent behavioral gap between what the browser ran and what Node-side tests exercised. Known regression, documented in docs/baseline.md rather than silently carried: opening index.html directly via file:// now crashes, since Chromium blocks the analyzer module's import under CORS for that origin. The app still works correctly served over npm run dev or npm run build + npm start. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
card/lib/analyzer.js's synchronous require(esm) of src/analyzer.js needs unflagged require(esm) support, which shipped in Node 20.19.0 and 22.12.0 specifically -- not all of card/'s previously-declared ">=20" range, and not 21.x (non-LTS, EOL before the backport). Narrow both package.json engines fields to "^20.19.0 || >=22.12.0" (matching Vite 8's own declared constraint) and add engine-strict=true via .npmrc in both locations so an incompatible local Node fails loudly at install time instead of a later ERR_REQUIRE_ESM. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Product decision: accept file:// support's removal (flagged in Commit 3) rather than restore it. MOO-67 is building toward a server-backed Railway application with server-held credentials; preserving double-click local-file execution would complicate the modular architecture for little future value. docs/baseline.md now records this as resolved rather than open. README.md's Quick Start, Architecture, Contributing, and FAQ sections no longer advertise the zero-install "just open index.html" workflow -- self-hosting now documents npm install + npm run dev (or npm run build && npm start) as the supported path. Also fixed two other now-stale README claims caught while editing this section: the Architecture diagram's "Single File" label, and the Contributing section's test command (node --test tests/ doesn't discover this repo's flat tests/*.test.mjs layout; needs the glob). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add tests/ui-smoke.mjs: six deterministic Playwright checks covering the repository-view behaviors Commit 4B-4E are about to touch -- local-folder load + graph render, node-click detail-panel update, visualization-type switching, ?repo= URL prefill without a network call, browser back/forward, and zero real console errors. Verified passing against both the dev server and a production build. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Move the ~150-line D3 force-graph build/update lifecycle out of App()'s inline useEffect into renderRepositoryGraph() (src/render/repositoryGraph.js), a real ES module bridged onto window the same way src/analyzer.js is. Mechanical extraction (sed-sliced, not retyped): svgRef.current -> svgEl, setTooltip -> onHover, setSelected/setBlastRadius on background click -> onBackgroundClick, everything else (data, colorMap, colorMode, theme, folderFilter, graphConfig, COLORS, LAYER_COLORS) became explicit parameters. zoomRef/simRef/linksRef/nodesRef/selectFileRef are passed through as the same ref objects App() already holds -- several other call sites (zoom controls, blast-radius reset, PDF export, "Back to Issues") read these directly and needed no changes as a result. Verified via tests/ui-smoke.mjs (6/6 against both dev and production builds) plus an ad hoc Playwright probe specifically exercising zoom in/out/reset, hover tooltip, and "Back to Issues" -- zero console errors, all three still worked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Move selected, blastRadius, rightTab, and drillDown -- the state the repository view's selection and detail panel need -- from four separate useState calls in App() into useRepositorySelection() (src/state/selection.js), bridged onto window the same way as Commits 3 and 4B. Every variable name stayed identical, so no call site elsewhere in App() needed to change. Deliberately left alone: showGraphConfig, rightPanelWidth, folderFilter, data/loading/error, theme, and all architecture/security-specific state, plus expandedPaths/ expandedCards (unrelated tree-expansion state that happened to sit textually nearby). Verified via tests/ui-smoke.mjs (6/6) plus an ad hoc Playwright probe cycling all four panel tabs -- zero console errors. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Consolidate buildAppUrl and the scattered query-param read/write logic (a mount-time useEffect reading ?repo=/?run=1, and two window.history.replaceState call sites) into src/state/route.js: buildRepoUrl, readRouteRepo, writeRepoRoute, clearRoute. Bridged onto window the same way as Commits 3, 4B, and 4C. Scoped to repository identity only, per the checklist -- not active view/panel/selection restoration, and not canonical source coordinates or breadcrumb payloads, both reserved for MOO-68. buildRepoUrl/readRouteRepo accept an optional baseHref/search parameter (defaulting to the real window.location), making this the first of the four extractions genuinely unit-testable without a DOM -- tests/route-state.test.mjs adds 8 tests covering URL construction, the run=1 gate, and the three validation guards the original inline code already had. Verified via tests/ui-smoke.mjs (6/6, its route/hash-restoration check exercises readRouteRepo directly) plus an ad hoc Playwright probe calling writeRepoRoute/clearRoute directly, since the write path only triggers on a real GitHub load the smoke suite deliberately avoids. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… 4E) Add a dblclick handler on graph nodes in repositoryGraph.js alongside the existing click handler, using the same node identity (d.id) the click handler already uses. Wired to a new activateFileRef (same pattern as selectFileRef), which App() initializes to a no-op and does not wire to anything else -- node-activate may remain unused or resolve to a no-op in MOO-67 per the checklist, with real drill-down semantics reserved for MOO-68. This is the last piece of Commit 4 -- 4A (UI smoke suite), 4B (repository graph renderer), 4C (selection/panel state), 4D (route persistence), and now 4E (interaction seam) are all landed and independently verified. Verified via tests/ui-smoke.mjs (6/6) plus an ad hoc Playwright probe double-clicking a node (no crash) then single-clicking it again (still selects correctly, proving no event-wiring corruption). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Restructure server/index.js from the Commit 2 static-file placeholder into a real server shell: config validated fail-fast at startup (server/lib/config.js), a request-scoped workspace abstraction with a single controlled root and cleanup (server/lib/workspace.js), structured per-request JSON logging with sanitized secrets (server/lib/logger.js), distinct liveness/readiness endpoints (server/lib/health.js), and a bounded POST /api/analyze endpoint (server/routes/analyze.js) that only reaches paths already on the server's own filesystem -- no GitHub credential or auth gate exists yet, that's Commit 6. analyzer-bridge.js reuses card/lib/collect.js's buildAnalyzed() for file collection rather than writing a fourth copy of that logic. Added durable automated coverage: 9 new unit tests (server-config.test.mjs, server-workspace.test.mjs) plus tests/server-smoke.mjs, which spawns the real server process and confirms /api/analyze against golden-world matches the exact files:6/functions:7/connections:6 baseline from Commit 1, path- traversal and missing-path requests are rejected, and the workspace root is completely empty after all requests (cleanup verified, not just callable). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
GitHub.scanTree/scanRecursive call shouldExcludeFile/ shouldIgnoreDirectory, which were left behind in index.html during the Commit 3 extraction -- they only worked in the browser by accident (window-fallthrough), and threw "shouldExcludeFile is not defined" the first time anything called GitHub.scan() server-side, ahead of building Commit 6's GitHub-backed endpoint around it. Move IGNORE, normalizeExcludePath, matchesExcludePattern, shouldIgnoreDirectory, and shouldExcludeFile into src/analyzer.js for real, exported and re-bridged onto window so index.html's own local-folder-reading code (which also calls these) keeps working unchanged. Verified GitHub.scan/getFile against a real public repo from Node. Added tests/analyzer-module.test.mjs (4 tests, no network) so this gap can't silently reappear. Full suite 83/83, clean build, and tests/ui-smoke.mjs (6/6) all pass -- pure addition/relocation, no behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Three independent gates now sit in front of every /api/* route: - server/lib/auth.js -- a shared-secret Authorization: Bearer check, timing-safe compared. /healthz/readyz/static stay public for Railway's own health monitoring. - server/lib/rate-limit.js -- in-memory per-client-IP fixed-window limiter (RATE_LIMIT_PER_MINUTE). - server/lib/validate-repo-request.js + server/lib/allowlist.js -- owner/repo/ref/PR format validation, then an allowlist check, both before any GitHub call. AUTH_TOKEN, GITHUB_TOKEN, and at least one of ALLOWED_REPOS/ ALLOWED_OWNERS are now required, validated fail-fast at startup like everything else in config.js. New POST /api/analyze-repo (server/routes/analyze-repo.js + server/lib/github-analyzer-bridge.js) fetches a repo at a resolved ref (default branch, explicit branch/commit, or a PR's head SHA) via the GitHub REST API using the server-held token, then runs it through the same analyzer everything else uses. Reuses GitHub/Parser/ shouldExcludeFile/buildAnalysisData from src/analyzer.js rather than writing a second GitHub client -- but not GitHub.scanTree/getFile as-is, since both are hardcoded to the default branch with no ref parameter, which is exactly the gap this commit needs closed. Found and fixed two real bugs while verifying against real GitHub data (not fixtures): a PR's head commit usually lives in a fork, not the base repo, so tree/blob fetches need to follow head.repo, not the originally-requested owner/repo; and GitHub.request()'s errorMap errors are plain Errors, not GithubFetchError, so a genuinely-expected failure (a deleted PR fork) was surfacing as a generic 500 instead of a clean 502 with GitHub's own message. Added tests/server-auth.test.mjs (16 unit tests), expanded server-config.test.mjs (10 tests) and server-smoke.mjs (17 steps, including both bugs above verified against real repos/PRs via a real GitHub credential from `gh auth token`). Full suite 105/105, clean build, tests/ui-smoke.mjs still 6/6. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add railway.json pinning the build/start commands and wiring Railway's own rollout health gate to /readyz (not just "process is up") -- confirmed applied by re-checking deployment metadata before and after. Railway's Railpack builder had already auto-detected and deployed this successfully with zero config, but explicit configuration is a real checklist item, not just a nice-to-have. Created and linked the codeviz Railway project/service, set AUTH_TOKEN (generated, not committed anywhere), GITHUB_TOKEN (the same PAT already used throughout Commits 5-6), ALLOWED_OWNERS (OwenTanzer), and NODE_ENV=production via `railway variable set`. Verified against the live deployment, not just locally: healthz/ readyz, static serving, auth rejection (401 anonymous and wrong- token), the local-path analyze endpoint matching the same golden-world baseline every other environment produces, allowlist rejection (403) for a non-allowlisted owner, and a real GitHub-backed analysis (OwenTanzer/CodeVisualizer) succeeding end-to-end through the deployed instance. Documented both real rollback paths (dashboard redeploy of a specific past deployment; CLI redeploy from an earlier git commit -- confirmed there's no CLI command to target a non-latest deployment directly, rather than assuming one exists) and the exact DNS/Moopertonic Hub cutover steps for MOO-72, without performing that cutover now. MOO-67 complete: all seven commits landed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Requested after initial deployment: analyze other users' repos, not just the operator's own. server/lib/allowlist.js now recognizes a literal "*" in ALLOWED_OWNERS as an explicit "any owner" opt-in, rather than requiring individual owner names to be piled on. The auth token remains the actual gate on who can reach /api/analyze-repo; this only changes which repos a valid caller can point it at. Set ALLOWED_OWNERS=* on the live codeviz Railway deployment and redeployed. Verified octocat/Hello-World (previously blocked) now analyzes successfully, and OwenTanzer/CodeVisualizer (the original allowlist entry) still works. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Three findings from PR #1 review, all fixed: 1. MAX_REPO_FILES capped file count but not byte size -- the GitHub-backed path fetched every accepted blob into memory before analysis, and the wildcard allowlist follow-up means any authenticated caller can now point the server at any public repo. Add MAX_FILE_BYTES/MAX_REPO_BYTES; GitHub's tree API already reports each blob's size, so oversized files are rejected before any content is fetched/decoded. Individually-oversized files are skipped (like an ignored directory); the aggregate is a hard cap. Refactored the selection logic into a pure, exported selectAnalyzableFiles() specifically so it's unit-testable against synthetic tree data instead of only reachable through a real GitHub round-trip (7 new tests). 2. resolveWithinRepo()'s comment claimed it rejected symlinks; it only did lexical path-traversal checking and never called realpath(), so a symlink sitting lexically inside the repo could still point elsewhere. Fixed by resolving both the repo root and the requested target through realpath() and checking containment on the resolved paths -- what cp() actually reads from. Verified with real filesystem junctions (5 new tests) rather than just asserting the fix works: one junction escaping the repo root is rejected, one pointing elsewhere inside the root is accepted. 3. /api/analyze-repo bounded its body via MAX_REQUEST_BODY_BYTES; /api/analyze buffered the whole request unbounded, despite both being publicly addressable behind the same bearer-token gate. Extracted one shared readJsonBody() (server/lib/http-body.js, 5 new tests) both routes now use. Full suite 126/126 (up from 107). tests/server-smoke.mjs re-verified against real GitHub data (17/17) to confirm none of this broke the existing happy paths. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Flagged as non-blocking by review but easy to close out now: add .github/workflows/test.yml running npm ci && npm run build && node --test tests/*.test.mjs on push/PR to main. Scoped to the credential-free, no-real-network unit/integration suite -- tests/ui-smoke.mjs (needs a Playwright browser) and tests/server-smoke.mjs (needs a real GitHub token) stay manual-precondition scripts for now; wiring those into CI too is a reasonable separate follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 new alerts (2 high, 1 medium) surfaced once the diff was large enough for CodeQL to re-evaluate index.html/server/ -- pre-existing gaps, not regressions, per GitHub's own note that alerts not introduced by the PR can still appear when changes are large. Fixed regardless: - server/lib/static.js (js/path-injection, high x2): the resolved.startsWith(distDir + sep) check is a raw string-prefix comparison, not a pattern CodeQL recognizes as a proven sanitizer. Rewrote to use path.relative() + a ".."/absolute check, the same idiom server/routes/analyze.js's resolveWithinRepo already uses (which CodeQL did not flag). Deliberately did not add realpath(): dist/ is build output the operator controls, and realpath() throws for anything not on disk, which would have broken the existing SPA-style fallback for unmatched client-side routes. Added tests/server-static.test.mjs (6 tests) covering traversal rejection, a percent-encoded attempt, and the SPA-fallback case specifically. - index.html (js/functionality-from-untrusted-source, medium): every other CDN script tag already had an integrity="sha512-..." attribute; only mermaid.min.js was missing one. Computed the hash locally and cross-checked it against cdnjs's own published SRI metadata before trusting it -- they matched. Full suite 132/132 (up from 126). Clean build. tests/server-smoke.mjs re-verified (17/17). Manually re-confirmed live: traversal still rejected (400), SPA fallback for an unmatched route still works (200), a real asset still serves (200). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…deflow-and-establish-the-railway-application MOO-67: Modularize CodeFlow and establish the Railway application shell
Structured, versioned identity for any point in a specific revision of a specific repository: repository identity, resolved revision, path, scope-chain (symbolPath), symbol kind, and optional range. Serializes canonically (stable key order) and encodes to an opaque base64url token for routes/cache keys, avoiding fragile delimiter-based string splitting.
Normalizes repository/branch/commit/PR requests into one canonical AnalysisContext (owner, repo, mode, resolvedSha, optional base/head SHA, PR number), rejecting mixed or contradictory revision fields. assertContextPropagation enforces that a file/function drill-down request can never silently switch revisions relative to its parent graph.
One schema (schemaVersion, layer, context, nodes, edges, groups, analyzer provenance, confidence, warnings, rendering hints) shared by repository/file/function graphs. Layers keep distinct kind/hint vocabularies (shape, semantic colorRole, layout preference) while validation rejects cross-layer node/edge mismatches and dangling edge references with clear messages, and safely ignores unknown extra fields anywhere in the tree.
One AdapterResult envelope (graph, warnings, diagnostics, provenance, timing, cache metadata, partial flag) for every layer adapter, plus a fixed set of stable error categories (github_access, unsupported_input, parser_failure, subprocess_failure, malformed_analyzer_output, timeout, renderer_failure, internal_error). sanitizeDiagnostic strips stack traces and redacts secret-shaped keys at any depth, applied unconditionally by buildAdapterResult so a producer can't forget it. Partial-success results may omit the graph entirely, but never carry a schema-invalid one.
Shared interaction contract: single click emits selection/focus only (createSelectionEvent); double click emits drill-down intent (createDrillDownEvent) carrying the source coordinate and target layer, but only when isDrillDownEligible — an ambiguous or under-resolved coordinate throws NavigationError rather than dispatching an incorrect drill-down. createOpenSourceEvent covers the separate "view raw source" action. NavigationHistory is a back/forward breadcrumb stack keyed by graph cache key + selection + coordinate, truncating the forward branch on push like standard browser history. This is the real seam MOO-67 Commit 4E's node-activate no-op was left for.
buildCacheKey derives a stable sha256-based key from normalized revision context, analyzer name/version, GraphIR schema version, requested coordinate, and depth/options — canonicalized (deep key-sorted) so equivalent normalized requests collapse to the same key while any real difference (revision, analyzer version, schema version, depth, requested coordinate) always changes it. isCacheStale treats a schema-version mismatch as always stale and supports an optional TTL. buildProvenanceSummary reports visible provenance plus resolved/unresolved adapter-match counts.
- src/graph-ir/index.js: single barrel import surface for the whole
contract (no name collisions across the six modules).
- tests/fixtures/graph-ir/{repository,file-pyan,function-codevisualizer}.json:
representative GraphIR fixtures for all three layers, regenerable
via scripts/gen-graph-ir-fixtures.mjs, validated in CI.
- examples/minimal-graphir-adapter.mjs: a runnable example adapter
proving GraphIR can be produced and consumed (selection + drill-down
events, provenance summary) using only src/graph-ir/index.js, no
application code.
- docs/graph-ir-contract.md: the repository -> file -> function
identity flow, module-by-module ownership, and extension rules for
future languages/analyzers/renderers/LLM annotation.
- docs/baseline.md: MOO-68 completion summary alongside the existing
MOO-67 regression baseline.
Full suite: 207/207 (132 pre-existing + 75 new across all seven
commits). This closes MOO-68 -- MOO-69 can now adapt the repository
layer against these contracts instead of the MOO-67 neutral seams
directly.
|
@OwenTanzer is attempting to deploy a commit to the braedonsaunders' projects Team on Vercel. A member of the Team first needs to authorize it. |
Author
|
Opened against the wrong repo by mistake (upstream fork parent instead of OwenTanzer/codeflow). Correct PR: see OwenTanzer/codeflow. |
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.
Summary
Implements MOO-68's full 7-commit checklist: the versioned contract layer (
src/graph-ir/) that MOO-69 (repository adapter), MOO-70 (pyan3 file layer), and MOO-71 (CodeVisualizer function layer) will all build against. Pure library modules + fixtures + docs — no application wiring, no pyan3/CodeVisualizer adapters (both explicitly out of scope for this issue).sourceCoordinate.js: canonicalSourceCoordinate(repository identity, resolved revision, path, symbol scope chain, kind, range, ambiguity flag), structured JSON serialization, base64url route/cache tokens. Cross-platform path normalization.githubContext.js: normalizes repository/branch/commit/PR requests into oneAnalysisContextalways pinned to a resolved SHA.assertContextPropagationblocks a drill-down request from silently switching revisions relative to its parent graph.graphIR.js: the shared, versionedGraphIRenvelope (nodes/edges/groups, analyzer provenance, confidence, warnings, rendering hints) all three layers validate against, while keeping distinctkind/hint vocabularies per layer. Rejects cross-layer node/edge mismatches and dangling edges with specific messages; ignores unknown extra fields anywhere in the tree.adapterResult.js:AdapterResultenvelope + fixed 8-categoryErrorCategoryset.sanitizeDiagnosticstrips stack traces and redacts secret-shaped keys at any depth, applied unconditionally.navigation.js:createSelectionEvent/createDrillDownEvent/createOpenSourceEvent, gated byisDrillDownEligibleso an ambiguous/unresolved coordinate can never dispatch an incorrect drill-down.NavigationHistoryback/forward breadcrumb stack. This is the real dispatch MOO-67 Commit 4E'snode-activateno-op seam was left for.cacheKey.js:buildCacheKey(stable sha256 over normalized context/analyzer/schema/coordinate/depth/options — equivalent requests collapse to one key, any real difference never collides),isCacheStale,buildProvenanceSummary.src/graph-ir/index.jsbarrel, representative fixtures for all three layers (tests/fixtures/graph-ir/*.json, regenerable viascripts/gen-graph-ir-fixtures.mjs), a runnable example adapter (examples/minimal-graphir-adapter.mjs) proving GraphIR can be produced/consumed with zero application-code imports, anddocs/graph-ir-contract.mddocumenting the full repository → file → function identity flow and extension rules.Test plan
node --test tests/*.test.mjs— 207/207 (132 pre-existing + 75 new)npm run build— cleannode examples/minimal-graphir-adapter.mjsruns end-to-end with no application-code importsvalidateGraphIR🤖 Generated with Claude Code