Repository navigation
feat: migrate investigation architecture - #42
Conversation
✅ Deploy Preview for wp-json-discovery ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddae072396
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| </Suspense> | ||
| ); | ||
| return ( | ||
| <InvestigatorShell readModel={investigatorReadModel} commands={{ onSectionChange: handleInvestigatorSectionChange, onRetry: retryInvestigatorCapability }} activeSection={activeInvestigatorSection} contentLandmark="main" contentMode="legacy" headerActions={headerActions}> |
There was a problem hiding this comment.
On the scan route, contentMode="legacy" suppresses InvestigatorShell's canonical section content while ScanPage forwards the selected findings, evidence, assets, history, or tools ID to the legacy ScanSectionContent; that switch handles none of those IDs and returns null. Selecting most of the newly advertised primary sections therefore produces a blank report area. Render the canonical shell content or map/hide these sections until the legacy renderer supports them.
AGENTS.md reference: AGENTS.md:L54-L54
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 41e7a7b. Investigator routes now render canonical section content for all advertised sections instead of sending those IDs to the legacy renderer. Added navigation/content regressions.
| if (snapshot.investigation === undefined) { | ||
| throw new ContractInvalidError('Anonymous investigation is missing validated full state'); | ||
| } |
There was a problem hiding this comment.
Every anonymous snapshot written before this commit lacks the new top-level investigation field—the previous serializer stored only version, revision, domain, and record. Rejecting those otherwise-valid snapshots here means an upgrade makes all existing anonymous investigations impossible to resume or claim, even though their session record contains enough state for the existing recovery mapper to reconstruct the investigation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 41e7a7b. Legacy anonymous snapshots are reconstructed from their validated session record during strict load/claim, preserving upgrade continuity while malformed payloads still raise ContractInvalidError.
| const session = record.latestSession; | ||
| if (session?.investigationState !== undefined) { |
There was a problem hiding this comment.
Authenticated session rows created before this migration legitimately omit the newly optional investigationState, and the server still returns those stored snapshots unchanged. The investigations list can therefore show an old row as resumable, but get() always reaches this branch and fails with contract-invalid; reconstruct the legacy state on read or migrate stored sessions so existing investigations remain usable.
AGENTS.md reference: AGENTS.md:L61-L61
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 41e7a7b. Authenticated records without investigationState are reconstructed from validated legacy session capability data during read/list/resume.
| const selection = normalizeSelection(capabilities); | ||
| const { startInvestigation: startCommand } = await import('../application/investigation/start.ts'); | ||
| const result = await startCommand({ | ||
| domain: { submittedUrl, normalizedUrl: normalize(submittedUrl) }, |
There was a problem hiding this comment.
When a signed-in user submits a value supported by DomainForm, such as https://www.example.com/, this preserves the raw value as submittedUrl and sends it through remoteStart. The server's sanitizeDomain accepts only bare hostnames, so protocol/path inputs are rejected; a bare www.example.com is instead stored with www and then rejected by the client's identity assertion because normalizeDomain removed it. As a result, common inputs that work for anonymous scans fail before authenticated capability execution.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5f44436. Authenticated starts now normalize URL-shaped inputs to hostname identity consistently with the server while preserving submittedUrl and redirectChain. Added protocol/path/query/www regressions.
| const investigation = authenticated && dependencies.remoteStart | ||
| ? await dependencies.remoteStart( |
There was a problem hiding this comment.
For an authenticated start, the initial remote allocation is awaited before createPersistenceContext installs its remote-to-local fallback. If the API or database is unavailable during this POST, the command rejects without allocating, saving, or running a local investigation, so the promised offline/local-only behavior only covers later save failures and not the first persistence operation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c2bd1cb. Initial authenticated allocation falls back locally only for network, retryable, or 5xx failures; validation, auth, conflict, and other 4xx/contract errors propagate.
| startedAt: timestamp, | ||
| completedAt: status === 'running' ? null : timestamp, |
There was a problem hiding this comment.
All terminal sessions are serialized with completedAt equal to the investigation's original createdAt, regardless of how long execution or a later resume took. This records every completed or failed attempt as zero-duration and corrupts persisted timeline data; use the lifecycle completion time or at least the investigation's latest updatedAt for terminal completion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 41e7a7b. Session serialization now uses the investigation latest update/lifecycle completion time for terminal completedAt instead of the original createdAt.
| const hydrated = await Promise.all(summaries.map(async summary => { | ||
| if (!summary.latestSessionId) return summary; | ||
| try { | ||
| const value = await source.get(summary.id); |
There was a problem hiding this comment.
Whenever a summary has latestSessionId, list() issues an additional full-state GET for that investigation even though InvestigationSummary already contains every field consumed by the investigations table. Accounts with substantial history therefore incur an N+1 request pattern and download potentially large evidence payloads; moreover, one failed hydration rejects the entire list instead of leaving that row as its valid summary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 41e7a7b. List returns validated summaries directly without per-item hydration; full state is fetched only through explicit get/resume paths, and summary rows survive individual races/not-found responses.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 37574786 | Triggered | Generic Password | 41e7a7b | server/src/utils/domain.js | View secret |
| 37574786 | Triggered | Generic Password | c2bd1cb | frontend/src/services/investigationSession.js | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Summary
Test Plan
pnpm --filter frontend test(552 passed)pnpm --filter frontend typecheckpnpm --filter frontend buildpnpm --filter frontend build-storybookKnown unrelated issue: server Jest configuration remains documented in
docs/product-roadmap.md.