Skip to content

Harden review workflows and CI coverage - #71

Merged
itsjling merged 9 commits into
mainfrom
codex/improve-all
Aug 2, 2026
Merged

Harden review workflows and CI coverage#71
itsjling merged 9 commits into
mainfrom
codex/improve-all

Conversation

@itsjling

@itsjling itsjling commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • harden review watch fingerprints and contain poll failures
  • handle Windows repository paths, age cache leases by heartbeat, and index picker files once
  • add local-server coverage, test pnpm caching in CI, and align provider docs with the product

Why

The watch path could hash transformed or unchecked file contents, stale cache leases could survive on PID reuse, and picker rows repeated the same file scan. Gaps in server coverage, CI cache checks, and provider copy made those faults harder to catch.

Impact

Reviews use safer fingerprints, stale cache leases expire, large picker lists do less work, and tests cover more of the shipped paths.

Checks

  • corepack pnpm install --frozen-lockfile
  • corepack pnpm run lint
  • corepack pnpm test
  • corepack pnpm run docs:check
  • corepack pnpm run fallow:audit
  • git diff --check 3fba94d..HEAD

@itsjling
itsjling marked this pull request as ready for review August 2, 2026 22:08

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a747ecf0b4

ℹ️ 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".

Comment on lines +20 to 23
- name: Enable the pinned pnpm version
run: corepack enable
- name: Set up Node
uses: actions/setup-node@v6

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Install pnpm before setup-node switches runtimes

On a clean runner where the preinstalled Node differs from the matrix version, this enables Corepack in the old Node installation and then replaces it on PATH; as corepack enable --help states, the shims are installed next to the currently resolved corepack binary. The cache: pnpm setup subsequently needs to invoke pnpm while actions/setup-node is running, so it can fail before the install step, and the same ordering appears in all five jobs in test-lanes.yml. Install pnpm independently before setup-node (for example with pnpm/action-setup), or otherwise ensure the selected Node toolchain exposes the shim before enabling pnpm caching.

AGENTS.md reference: AGENTS.md:L10-L17

Useful? React with 👍 / 👎.

Comment on lines +1164 to +1166
content.update(String(stat.size));
content.update('\0');
content.update(String(stat.mtimeNs));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include regular-file contents in watch fingerprints

When an untracked regular file is rewritten with the same byte length while its mtime is preserved—such as by a timestamp-preserving generator, rsync-like tool, or on a coarse-timestamp filesystem—its kind, size, mtime, path, and Git status all remain unchanged. The watcher therefore never calls refresh(), leaving the displayed patch stale until some unrelated change occurs; the previous implementation detected this by hashing the file bytes. Continue avoiding symlink targets, but hash regular-file content or another reliably content-sensitive value.

Useful? React with 👍 / 👎.

@itsjling
itsjling merged commit 7e14a87 into main Aug 2, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant