Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 98 additions & 0 deletions .github/workflows/codex-autofix-review-comments.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,30 @@ jobs:
"chatgpt-codex-connector",
"chatgpt-codex-connector[bot]",
]);
const forceReviewLabel = "codex-review";
const skipReviewLabel = "skip-codex-review";
const complexSourceFileThreshold = 10;
const complexSourceChurnThreshold = 300;
const highRiskPathPatterns = [
/^supabase\//,
/^src\/app\/api\//,
/^src\/(?:lib|app|components)\/.*(?:auth|permission|privacy|security|rag|retriev|rank|search|answer|clinical|citation|source|document|upload|download|billing|payment|quota|job|worker)/i,
/^src\/(?:proxy|instrumentation(?:-client)?)\.ts$/,
/^src\/lib\/(?:env|client-env|security-headers)\.ts$/,
/^scripts\/.*(?:ingest|reindex|migration|governance|production|deploy|drift|supabase)/i,
/^\.github\/workflows\//,
/^(?:package|package-lock)\.json$/,
/^(?:next|playwright|vitest)(?:\..+)?\.config\.[cm]?[jt]s$/,
/^(?:Dockerfile|railway\.json|nixpacks\.toml)$/,
];
const sourcePathPattern = /^(?:src|scripts|supabase|\.github\/(?:actions|workflows))\//;
const sourceExtensionPattern = /\.(?:[cm]?[jt]sx?|sql|css|scss|json|ya?ml)$/i;
const excludedAutomaticRoutePathPatterns = [
/(?:^|\/)(?:__tests__|fixtures|snapshots)\//,
/(?:^|\/)tests?\//,
/\.(?:snap|test|spec)\.[cm]?[jt]sx?$/i,
/(?:^|\/)generated\//,
];
const scopedResolveCommand = "@codex resolve actionable Codex review findings for this pull request and current head";
const resolvedDispositionMarker = "<!-- codex-thread-disposition:resolved -->";

Expand All @@ -70,6 +94,19 @@ jobs:
const owner = context.repo.owner;
const repo = context.repo.repo;
const issue_number = pr.number;
const labels = new Set(
(pr.labels || [])
.map((label) => (typeof label === "string" ? label : label?.name))
.filter(Boolean)
.map((label) => label.toLowerCase()),
);

// An explicit skip always wins, including when the opt-in label is
// also present. This gives maintainers a deterministic kill switch.
if (labels.has(skipReviewLabel)) {
core.notice(`Skipping Codex auto-resolve because the pull request has the '${skipReviewLabel}' label.`);
return;
}

let reviewComments;
try {
Expand All @@ -96,6 +133,66 @@ jobs:
return;
}

const routeReasons = [];
if (labels.has(forceReviewLabel)) {
routeReasons.push(`label:${forceReviewLabel}`);
} else {
let changedFiles;
try {
changedFiles = await github.paginate(github.rest.pulls.listFiles, {
owner,
repo,
pull_number: issue_number,
per_page: 100,
});
} catch (error) {
if (error.status === 403) {
const message = "Codex auto-resolve failed because the trigger token cannot read pull request files.";
core.warning(message);
core.setFailed(message);
return;
}

throw error;
}

const normalizedFiles = changedFiles.map((file) => ({
...file,
filename: (file.filename || "").replaceAll("\\", "/").replace(/^\.\/+/, ""),
}));
const routeableFiles = normalizedFiles.filter(
(file) => !excludedAutomaticRoutePathPatterns.some((pattern) => pattern.test(file.filename)),
);
const highRiskFiles = routeableFiles.filter((file) =>
highRiskPathPatterns.some((pattern) => pattern.test(file.filename)),
);
const changedSourceFiles = routeableFiles.filter(
(file) =>
sourcePathPattern.test(file.filename) && sourceExtensionPattern.test(file.filename),
);
const sourceChurn = changedSourceFiles.reduce(
(total, file) => total + (file.additions || 0) + (file.deletions || 0),
0,
);

if (highRiskFiles.length > 0) {
routeReasons.push("high-risk-path");
}
if (changedSourceFiles.length >= complexSourceFileThreshold) {
routeReasons.push(`complex-files:${changedSourceFiles.length}`);
}
if (sourceChurn >= complexSourceChurnThreshold) {
routeReasons.push(`complex-churn:${sourceChurn}`);
}

if (routeReasons.length === 0) {
core.notice(
`Skipping Codex auto-resolve for a low-risk pull request (${changedSourceFiles.length} source files, ${sourceChurn} changed source lines). Add '${forceReviewLabel}' to opt in.`,
);
return;
}
}

// The request is posted by the trigger token's account, so dedup must
// trust that identity rather than github-actions[bot].
let triggerLogin;
Expand Down Expand Up @@ -154,6 +251,7 @@ jobs:
marker,
`<!-- codex-autoresolve-head:${pr.head.sha} -->`,
`<!-- codex-autoresolve-source-review:${review.id} -->`,
`<!-- codex-autoresolve-route:${routeReasons.join(",")} -->`,
`${scopedResolveCommand} using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with ${resolvedDispositionMarker} as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.`,
].join("\n\n");

Expand Down
5 changes: 4 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -389,10 +389,13 @@ When explicitly asked to fix or resolve review findings:

### Automatic resolve trigger

Automatic Codex review is review-only by default. This repository includes `.github/workflows/codex-autofix-review-comments.yml`, which requests the resolve task automatically after Codex submits a completed PR review that raised findings.
Automatic Codex review is review-only by default. This repository includes `.github/workflows/codex-autofix-review-comments.yml`, which requests the resolve task automatically after Codex submits a completed PR review that raised findings and the pull request passes the repository's risk/complexity router.

- The auto-resolve request must fire only from a Codex-authored `pull_request_review` **submitted** event on an open pull request — never from the first inline comment mid-review. This guarantees the request is posted only after a code review completes; without a review there are no findings and the request is pointless.
- The request job must skip reviews with no actionable findings: skip `approved`/`dismissed` reviews, and skip when the submitted review carries zero inline comments.
- Route automatic repair only when at least one changed path is high-risk, when the pull request changes at least 10 non-test source files or 300 non-test source lines, or when the `codex-review` label explicitly opts in. Treat `skip-codex-review` as an unconditional opt-out that wins if both labels are present.
- High-risk paths include migrations/RLS, application API routes, auth/permissions/privacy/security, clinical/RAG/retrieval/search/source/document behavior, provider or production configuration, dependencies, and CI/release workflows. Do not route docs-only, test-only, generated-only, or small low-risk UI/copy changes unless explicitly opted in.
- Read changed-file metadata through the GitHub API only; never check out or execute pull-request code in the routing job. Record the selected route in a hidden `codex-autoresolve-route` marker for auditability.
- Match the trusted Codex connector bot by exact login and bot type; do not use substring login checks.
- Keep per-pull-request concurrency on the authorized job, not the whole workflow, so unrelated events cannot displace a pending Codex request.
- Pin the supported Node 24-based `actions/github-script` release to its reviewed immutable commit SHA.
Expand Down
63 changes: 63 additions & 0 deletions design-qa.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# Design QA — 2026-07-15

## Scope and source of truth

- Implementation: Clinical KB at `http://localhost:4392` from `codex/design-polish-pass` (`0c56f27a37af88a073d2bb695d2cf4c05067ff4f` before edits).
- Source visual truth: **unavailable**. No Figma file, approved screenshots, mockup package, or other independent target was supplied. The existing product design system was treated as the consistency baseline, not as proof of target fidelity.
- Runtime safety: local Next.js server only. OpenAI variables were cleared, Supabase variables pointed at a non-listening local placeholder, and provider mode was set to offline. No provider-backed workflow was run.

## Viewport and route evidence

- Desktop sweep at 1440 × 1000: 30 routes covering Answer, Documents, Services, Forms, Favourites, Differentials, DSM, Specifiers redirects, Formulation, Prescribing/Medication, Tools/Application redirects, Privacy, and colour coding.
- Phone sweep at 390 × 844: 21 representative routes across the same production surfaces.
- Focused responsive proof after remediation: 320 × 700, 390 × 844, 639 × 900, 768 × 1024, 1440 × 1000, and 1920 × 1080.
- Desktop route metrics: `artifacts/design-audit-2026-07-15/desktop-route-metrics.json`.
- Phone route metrics: `artifacts/design-audit-2026-07-15/mobile-route-metrics.json`.
- Representative screenshots: `01-home-desktop.png`, `32-answer-home-mobile.png`, `46-formulation-builder-mobile.png`, `53-formulation-builder-mobile-after.png`, `54-formulation-builder-320-after.png`, `55-formulation-builder-768-after.png`, and `56-formulation-builder-1920-after.png` in `artifacts/design-audit-2026-07-15/`.

## Comparison history

### Iteration 0 — full live audit

- Long phone pages in the shared standalone shell exposed two independent vertical scroll surfaces: the document root and `#main-content`.
- On `/formulation/builder` at 390 × 844, the document scroll height reached 2813 px while the app scrollport also contained the long page.
- No true root horizontal overflow was found. Wide children reported in the phone metrics were contained horizontal scrollers rather than page overflow.
- No reproducible clipping, overlap, inaccessible touch target, or console warning was found in the accepted route screenshots.

### Iteration 1 — scroll ownership remediation

- Anchored the phone standalone shell to the dynamic viewport and retained `#main-content` as its only vertical scroll owner.
- After the change, `/formulation/builder` at 390 × 844 reported document `clientHeight=844` and `scrollHeight=844`; `#main-content` reported `clientHeight=772` and `scrollHeight=3778`.
- The same invariant held at 320 × 700 and 639 × 900. At 768 px and wider, the shell returns to normal document scrolling so sticky desktop descendants keep working.
- Added a Playwright regression test for the single-scrollport contract.

### Iteration 2 — interaction and design-system polish

- Closing the app-mode menu on Tab prevents an abandoned floating menu after keyboard focus leaves it.
- Decorative dynamic icons now declare `aria-hidden` explicitly.
- Press-scale feedback is restricted to motion-safe environments.
- Sheet overlays now use `--overlay-backdrop`; forced-colors remaps glass, gloss, and backdrop tokens to solid system colours.

## Verification evidence

- Browser inspection: accepted screenshots at the viewports above; no root horizontal overflow; no warning/error console entries on inspected representative pages.
- Focused scroll regression: 1/1 passed.
- Keyboard mode-menu regression: 1/1 passed.
- Accessibility media suite: 5/5 passed (reduced motion, forced colours, 200% zoom, default axe WCAG A/AA, forced-colours axe WCAG A/AA).
- `npm run verify:cheap`: passed; 259 test files passed, 1 skipped; 2417 tests passed, 1 skipped; lint and TypeScript clean.
- `npm run verify:ui`: passed; 175/175 Chromium and Chromium-mockups tests.
- Focused Prettier, ESLint, TypeScript, type-scale, and icon-scale checks passed.

## Findings disposition

- P0/P1: none reproduced.
- P2 double phone scroll surface: fixed and regression-tested.
- P2 keyboard mode menu remains open after Tab: fixed and regression-tested.
- P3 motion/forced-colours/design-token consistency: fixed and accessibility-tested.
- Residual product risk: content and clinical governance were outside this design-only change; provider-backed behavior was deliberately not exercised.

## Final result

Final result: **blocked**.

Blocker: an independent source visual target is required to certify concept fidelity. Implementation-level responsive, interaction, accessibility, and regression QA passed, but the design cannot honestly be declared pixel-faithful to an external approved concept that was not supplied.
1 change: 1 addition & 0 deletions docs/branch-review-ledger.md
Original file line number Diff line number Diff line change
Expand Up @@ -557,4 +557,5 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD
| 2026-07-15 | PR #679 / claude/therapy-compass-pages-rz0m5l | f5ca25f8b6b14f41e63f708933fe4bb311994795 | Therapy Compass production promotion and review-followup | All five review findings are fixed on the reviewed head: run-enabled shared-composer links stay on the Therapy Compass route, production data loads outside `/mockups`, deep links seed the in-tool search, and same-route query changes remount the provider. No additional high-confidence defect remains in the changed scope. | GitHub review-thread inventory; exact-head hosted build, critical UI smoke, UI regression, unit coverage, static, security, and image checks green; focused Vitest 28/28; TypeScript; `git diff --check`. No live Supabase/OpenAI checks run. |
| 2026-07-15 | PR #680 / claude/rag-scalability-review-x0s55l | 32e242ab7fc386ea82b19c7cfc2112aa41f06f9a | privacy, public-catalog throttling, ingestion-recovery, and merge-readiness review | Audit remediation wave 1 plus review follow-ups. Confirmed and fixed: mixed-owner document list/detail responses exposed nested summary internals and free-form document metadata for public rows; anonymous catalog rate limiting skipped known-slug detail routes; and ingestion recovery could retry a failed row without seeing a legitimate pending/fresh-processing sibling. Ownership-specific projections/redaction now cover list and detail responses, every catalog detail path is throttled, and both recovery scripts pass every open sibling to the planner. No remaining unresolved review thread or high-confidence defect. | GitHub exact-head review-thread inspection (0 unresolved); hosted required CI, UI regression, migration replay, build, coverage, static, and security checks green; local focused route/recovery Vitest 163/163; TypeScript; earlier full `verify:cheap`; Prettier; `git diff --check`. No live Supabase/OpenAI/provider checks run. |
| 2026-07-15 | PR #682 / claude/codex-builder-perf-flakiness-ck5yv7 | dc942dddb87c8a1fa8d84d88052b3d50dd883a43 + reviewed follow-up diff | CI performance, cache correctness, browser-lane routing, and merge-readiness review | Confirmed and fixed two P2 CI safety gaps: the cached `node_modules` key ignored `package.json` and `.npmrc`, allowing install-contract changes to reuse stale modules and skip `npm ci`; and removing the advisory quarantine job made any newly tagged UI test run nowhere. The cache now covers every install input and the quarantine lane performs only a cheap source scan when empty, preserving the speed improvement without losing coverage. No remaining unresolved review thread or high-confidence defect. | GitHub exact-head checks green before final main sync; focused Vitest 6/6; GitHub Actions pin check; CI-scope self-test; TypeScript; Prettier; `git diff --check`. Final hosted exact-head checks required after push. No live Supabase/OpenAI checks run. |
| 2026-07-15 | codex/design-polish-pass | 0c56f27a37af88a073d2bb695d2cf4c05067ff4f + reviewed working diff | full live design, responsive, UX, accessibility, design-system, routing, performance, lint, testing, documentation, and release-readiness review | No P0/P1 reproduced. Fixed the P2 duplicate phone scroll surface in the shared standalone shell and added a regression test; fixed mode-menu Tab dismissal; explicitly hid decorative dynamic icons; restricted press scaling to motion-safe environments; moved sheet backdrops and forced-colour glass/backdrop behavior onto design tokens. External target fidelity remains blocked because no independent Figma/mockup source was supplied. | Live 30-route desktop + 21-route phone sweep; targeted 320/390/639/768/1440/1920 proofs; `npm run verify:cheap` (2417 passed/1 skipped); `npm run verify:ui` (175/175); focused keyboard 1/1; accessibility media/axe 5/5; scoped Prettier, ESLint, TypeScript, type-scale, and icon-scale passed. Provider-backed checks not run. |
| 2026-07-15 | codex/documents-closed-default | 49f63791bced2b1764a11ab723aea94b45b026b6 | documents viewer disclosure defaults and related defect hunt | Fixed the inconsistent default-open document viewer sections by making indexed text, high-yield summary, tables/diagrams, and indexing details a native mutually exclusive closed disclosure group. The section navigation opens its requested disclosure and deep-linked evidence still reveals its target. The hunt also removed the explicitly open nested table-review queue, preserved printable summary content through the browser print lifecycle, and added cold-server readiness guards to the affected viewer tests. No other high-confidence default-open defect remains in the live Documents scope. | `npm run verify:cheap`; TypeScript; focused ESLint/Prettier; clean-worktree mocked Chromium coverage for deep-linked evidence, structured summary, closed/mutually-exclusive disclosures, navigation opening, and print state restore; `git diff --check`. Turbopack could not run through the local external `node_modules` junction, so clean browser verification used Next's supported Webpack dev mode. No Supabase/OpenAI/live-provider checks run. |
1 change: 1 addition & 0 deletions docs/codex-review-protocol.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Use this protocol for every Codex review, audit, bug hunt, PR review, release-re
- Do not expand into stale branches or unrelated modules unless a confirmed defect crosses that boundary.
- Before branch or PR review, check `docs/branch-review-ledger.md`: resolve the target with `git rev-parse`, compare the HEAD and scope, and skip unchanged completed reviews unless the user asks for a fresh pass.
- Treat GitHub automatic review as one pass per pull request. A repair commit or later head does not authorize another automatic pass; require an explicit human request before re-reviewing.
- Route automatic repair only for high-risk paths, at least 10 changed non-test source files, at least 300 changed non-test source lines, or an explicit `codex-review` label. `skip-codex-review` always opts out, including when both labels are present. Small low-risk, docs-only, test-only, and generated-only changes should not receive the automatic repair request.

## Review Output

Expand Down
Loading