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: 4ff852de28
ℹ️ 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".
| const record = investigationRecordSchema.safeParse(envelope.data.data); | ||
| if (!record.success) throw new Error('Invalid investigation response'); |
There was a problem hiding this comment.
requestInvestigation parses every successful response as an investigationRecord, but POST /api/investigations/:id/sessions/:sessionId returns the sessionRecord produced by saveInvestigationSession (server/src/routes/investigations.js:65-68). Consequently every authenticated progress write succeeds in the database and then throws Invalid investigation response; ScanPage reports a persistence failure and creates an anonymous fallback, which can lead users to import a duplicate investigation.
Useful? React with 👍 / 👎.
| const snapshot = loadAnonymousInvestigation(); | ||
| if (snapshot) { | ||
| setAnonymousSnapshot(snapshot); | ||
| if (!isAuthenticated) { | ||
| setInvestigatorSession(hydrateSession(snapshot.record.session, snapshot.domain, scanSettings.options)); |
There was a problem hiding this comment.
If the page reloads while an anonymous scan is queued or running, this effect only hydrates that persisted state and never restarts or converts it to a recoverable failure. The restored status keeps the Start scan button disabled at line 349, while retry controls are offered only for terminal capability states, leaving the investigator permanently stuck; the authenticated resume path at lines 97-102 has the same behavior.
Useful? React with 👍 / 👎.
| function IdentityLayer({ result }) { | ||
| const identity = result?.identity; | ||
| const evidence = normalizeEvidence(identity?.evidence, identity); | ||
| const hasValue = typeof identity?.value === 'string' && identity.value.trim().length > 0; | ||
| const status = hasValue ? evidenceStatus(evidence.successful) : 'unavailable'; | ||
| const value = status === 'unavailable' ? 'Unavailable' : identity.value; |
There was a problem hiding this comment.
The production WordPress runner remains scanDomain, whose returned object contains domain, summary, exposure, performance, plugins, and metrics, but no identity or findings; repository-wide search finds producers for these fields only in test fixtures. Therefore every real successful scan reaches this code with identity undefined, renders the Site profile as unavailable, and never renders actionable findings, defeating the new core result hierarchy.
AGENTS.md reference: AGENTS.md:L7-L9
Useful? React with 👍 / 👎.
| {['failed', 'unavailable'].includes(capability.status) && capability.outcome?.error?.retryable ? ( | ||
| <Button type="button" variant="secondary" size="sm" disabled={retryingCapabilityId === id} onClick={() => onRetryCapability(id)}> | ||
| {retryingCapabilityId === id ? `Retrying ${CAPABILITY_LABELS[id] ?? id}…` : `Retry ${CAPABILITY_LABELS[id] ?? id}`} | ||
| </Button> |
There was a problem hiding this comment.
When a capability is unavailable because its runner was absent or a dependency failed, the contract requires its outcome error to have retryable: false, so this condition can never render a retry button for any unavailable state. After the runner becomes available or the failed dependency is successfully retried, users therefore cannot invoke the engine's supported unavailable-capability retry path without starting a new investigation.
Useful? React with 👍 / 👎.
| <ScanSectionContent | ||
| activeSection={visibleSection} | ||
| session={session} | ||
| session={investigatorSession ? bridgeInvestigatorSession(investigatorSession) : session} | ||
| scanSettings={scanSettings} | ||
| onScanSettingsChange={updateScanSettings} | ||
| onRunCapability={runCapability} | ||
| onRetryCapability={retryCapability} | ||
| onRunCapability={investigatorSession ? handleRunInvestigatorCapability : runCapability} | ||
| onRetryCapability={investigatorSession ? handleRetryInvestigatorCapability : retryCapability} |
There was a problem hiding this comment.
Recent-domain cards still invoke the legacy startScan callback, but after an investigator session has been loaded or completed this conditional always renders investigatorSession instead of the newly updated legacy session. An authenticated user clicking Rescan therefore runs network work while the status stack, sidebar, and results continue showing the previous investigation; clear/replace investigator state or route rescans through the new controller.
Useful? React with 👍 / 👎.
| setInvestigatorSession(hydrated); | ||
| void persistInvestigatorSession(hydrated, identity); | ||
| }; | ||
| const result = await runInvestigationSession(hydratedNextSession, getCapabilityRunners(selection.capabilityIds), onChange, token); |
There was a problem hiding this comment.
The new primary form path passes bare runners directly to runInvestigationSession, bypassing useScan.invokeSettledOutcomes. As a result, WordPress's registered onWordpressSettled hook never persists newly detected unsupported namespaces, invalidates related queries, or emits scan.complete/scan.error, so new investigator scans stop populating the existing unsupported-plugin and scan-history workflows.
AGENTS.md reference: AGENTS.md:L82-L84
Useful? React with 👍 / 👎.
| export function saveAnonymousInvestigation(snapshot) { | ||
| const current = loadAnonymousInvestigation(); | ||
| const next = normalizeSnapshot(snapshot); | ||
| if (!next) return; | ||
| if (current && Date.parse(current.record.persistedAt) >= Date.parse(next.record.persistedAt)) return; |
There was a problem hiding this comment.
ScanPage timestamps every emitted snapshot with millisecond-resolution new Date().toISOString(), while the engine publishes a failed capability state and its finalized failed session synchronously. When those writes share a millisecond, this >= check discards the terminal snapshot and retains the preceding running state; the next reload then restores a scan that can never finish. Preserve ordering with a revision/sequence or allow an equal-timestamp terminal state to supersede progress.
Useful? React with 👍 / 👎.
| function ActionableFindings({ findings }) { | ||
| const supportedFindings = []; | ||
| const unavailableFindings = []; | ||
| (findings ?? []).forEach((finding) => { | ||
| const evidence = normalizeEvidence(finding?.evidence, finding); | ||
| if (evidence.successful.length > 0) { |
There was a problem hiding this comment.
For a contract-valid findingSchema value, finding.evidence contains references shaped as { id, capabilityId, locator }, while confidence is stored in finding.evidenceLevel; none of those references has the status field expected by normalizeEvidence. Thus every canonical finding is classified as having no successful evidence and is moved to the unavailable section. Use evidenceLevel to classify the finding and use the references for provenance rather than requiring the test-only evidence shape.
Useful? React with 👍 / 👎.
| router.post('/', wrapAsync(async (req, res) => { | ||
| const userId = requireUser(req); | ||
| const input = validate(startInvestigationRequestSchema, req.body); | ||
| const record = await createInvestigation(userId, input); |
There was a problem hiding this comment.
For direct API clients, the start route only applies startInvestigationRequestSchema, whose domain fields merely require nonempty strings, and never invokes the existing sanitizeDomain boundary. Requests can therefore persist malformed, private, or noncanonical identities such as localhost, IP literals, or caller-supplied normalized values that disagree with the submitted domain; the claim route has the same gap. Normalize from the submitted value server-side and reject values the repository sanitizer rejects before writing.
Useful? React with 👍 / 👎.
1 participant
Summary
Verification
pnpm typecheckpassed.git diff --checkpassed.Known limitations
AdminPagefailures.ScanPage.jsx,SitemapSection.jsx, andExposurePanel.jsx.Next steps