Skip to content

Probe checkout path identity at runner startup - #167

Open
eastagiletracker wants to merge 1 commit into
codeabovelab:mainfrom
eastagiletracker:agile-board/probe-path-identity-at-runner-startup
Open

eastagiletracker wants to merge 1 commit into
codeabovelab:mainfrom
eastagiletracker:agile-board/probe-path-identity-at-runner-startup

Conversation

@eastagiletracker

Copy link
Copy Markdown

This PR proposes probing checkout path identity at runner startup and refusing a pathIdentity setting that disagrees with the filesystem (Fixes #162). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/1009. You can sign in with your GitHub ID to claim ownership of the project.

What changed

ReviewService derives plan path keys from the configured pathIdentity, but nothing checked that setting against the filesystem the task checkouts live on; only scripts/demo.ts probed. On current main (22e76a8), setUpRunner with pathIdentity: { caseSensitive: false, unicodeNormalization: 'none' } on a case-sensitive Linux runner root resolves and starts normally, so plan paths like README.md and readme.md would collapse to one key while naming two files (and the reverse on a case-insensitive checkout).

This PR moves the demo's probe into a shared runner/path-identity.ts (probePathIdentity, assertPathIdentity). setUpRunner now probes <runner.root>/<owner>/attempts (where task checkouts are created) right after the lock is verified, before recovery and the image build, and refuses to start with a message naming both the configured and probed values. As the issue asked, the choice is recorded in docs/implementation/read-only-review.md: the setting is kept and checked, not derived, so a review never silently changes its plan keys. scripts/demo.ts uses the same probe, so its behavior is unchanged.

Verification: the new runner startup > refuses to start when pathIdentity disagrees ... test in test/runner-production.test.ts fails without the startup check (AssertionError: promise resolved "{ deps: { …(12) }, …(6) }" instead of rejecting) and passes with it. It covers a case mismatch and a Unicode mismatch, checks that nothing ran past the lock verification, and checks that the probe leaves no files behind. test/path-identity.test.ts unit-tests the probe directly. npm run typecheck is clean, and npm test shows the same 27 failures before and after (all of them need Docker, which this machine does not have), with 4 more tests passing and none newly failing. Heads-up: #153 and #161 also touch runner/production.ts, in different places (push and merge wiring), so a rebase should be trivial.

How this was managed

This work was tracked as https://eastagiletracker.com/projects/1009/stories/1319101 on a board imported from this repo's issues and pull requests (134 stories): https://eastagiletracker.com/projects/1009

board

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.


Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com

Runner startup now probes case sensitivity and Unicode normalization on
the filesystem that holds the task checkouts and refuses to start when
the configured pathIdentity disagrees. The demo uses the same shared
probe.
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.

Probe checkout path identity at runner startup

1 participant