Repository navigation
Probe checkout path identity at runner startup - #167
Open
eastagiletracker wants to merge 1 commit into
Open
eastagiletracker wants to merge 1 commit into
eastagiletracker wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR proposes probing checkout path identity at runner startup and refusing a
pathIdentitysetting 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
ReviewServicederives plan path keys from the configuredpathIdentity, but nothing checked that setting against the filesystem the task checkouts live on; onlyscripts/demo.tsprobed. On currentmain(22e76a8),setUpRunnerwithpathIdentity: { caseSensitive: false, unicodeNormalization: 'none' }on a case-sensitive Linux runner root resolves and starts normally, so plan paths likeREADME.mdandreadme.mdwould 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).setUpRunnernow 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 indocs/implementation/read-only-review.md: the setting is kept and checked, not derived, so a review never silently changes its plan keys.scripts/demo.tsuses the same probe, so its behavior is unchanged.Verification: the new
runner startup > refuses to start when pathIdentity disagrees ...test intest/runner-production.test.tsfails 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.tsunit-tests the probe directly.npm run typecheckis clean, andnpm testshows 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 touchrunner/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
If you'd rather not receive contributions like this, reply
no-more-prson 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