Skip to content

fix: resolve .commit-info.json info/exclude path against the checkout - #37

Merged
eyevinn-developer[bot] merged 1 commit into
mainfrom
fix-commit-info-exclude-abs-path
Nov 4, 2026
Merged

eyevinn-developer[bot] merged 1 commit into
mainfrom
fix-commit-info-exclude-abs-path

Conversation

@eyevinn-developer

Copy link
Copy Markdown
Contributor

Fixes the .commit-info.json exclude entry landing in the wrong place. git -C "$repo_dir" rev-parse --git-path info/exclude prints a path relative to $repo_dir, but the entrypoint runs from /runner, so the append never reached /usercontent/.git/info/exclude. The path is now made absolute against $repo_dir before mkdir -p and append (same approach as Eyevinn/web-runner#74). excl is also now local.

write_commit_info is the only place in this repo that writes .commit-info.json or sets the exclude, so there are no sibling paths.

Added tests/test-entrypoint-commit-info-exclude.sh. It extracts the real function and calls it twice from a cwd that is not the repo. Against the old entrypoint it fails 3 checks (?? .commit-info.json in status, exclude count, stray .git in the cwd). Against the fix it passes all checks. No tracked file is touched and no commit is created.

Closes #36

Lessons

git path-printing commands (--git-path) are relative to -C, not to the caller's cwd. The old code looked fine when run from inside the repo, so only a test that calls from another cwd catches it.

What a human does next: Nothing.

Co-Authored-By: Claude noreply@anthropic.com

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>

@eyevinn-pr-reviewer eyevinn-pr-reviewer 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.

Research Review

Verified

  • git rev-parse --git-path is relative to the -C directory, not the caller's cwd. I reproduced this: running the old entrypoint's write_commit_info from a cwd outside the repo leaves ?? .commit-info.json in git status, a count != 1 for the exclude entry, and a stray .git in the caller cwd. The fix is case "$excl" in /*) ;; *) excl="$repo_dir/$excl" ;; esac in scripts/docker-entrypoint.sh. It handles both absolute output (worktrees/linked gitdirs) and relative output.
  • Test reproduced locally with git 2.39.5: on the PR head all 6 checks pass. Against origin/main's entrypoint (via ENTRYPOINT_OVERRIDE) exactly 3 checks fail, as the PR body states.
  • Sibling consistency: the fix matches the web-runner#74 diff line for line (same comment, same case), and the test is the same shape.
  • "Only place that writes .commit-info.json" confirmed by grep: only scripts/docker-entrypoint.sh and the new test reference it.

Adjacent Stubs Detected

  • Swept scripts/docker-entrypoint.sh: only || true guards on best-effort cleanup/reads (lines 136, 176-178, 218, 237). None is on the path this PR touches in a way that erases input. Nothing blocks merge.

Orphaned Files Detected

  • None. The test is a standalone script and the entrypoint is the Docker entry point.

Escape Hatch Coverage

  • No new terminal-rejection branch (no exit/throw/4xx added).

Object Shape Verification

  • No spread or re-serialisation of fetched objects.

Contract diff

  • No request handler or timeout changed. Files checked: scripts/docker-entrypoint.sh (shell function write_commit_info) and tests/test-entrypoint-commit-info-exclude.sh. The .commit-info.json content is unchanged. The only behavioural change is where the exclude line is appended, which is now $repo_dir/.git/info/exclude.

Impact Analysis

  • Blast radius: container start-up of python-runner only, in write_commit_info.
  • Downstream consumers: none in other repos for this function. web-runner already carries the same fix.
  • Coordinated changes needed: none identified.
  • Breaking change risk: low. Before the fix the append went to a path relative to /runner, i.e. outside the checkout. Existing PVC-cached checkouts just gain the missing exclude line on next start.

Risks Not Tested

  • Real container start-up on OSC was not exercised. I only exercised the extracted function in a bash harness.
  • The repo reports no CI checks on this branch, so the new test is not wired into automation. It passes only when run by hand, and I ran it locally.
  • eval of the extracted function in the test relies on the function body ending at the first ^} line. This holds today.

Research Suggestions

  • Optional: wire tests/test-entrypoint-commit-info-exclude.sh into a CI workflow so the regression stays covered.

Verdict

No blocking issues found.


Reviewed by PR Research Validator. This review checks whether code is based on verified sources rather than assumptions.

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.

fix: .commit-info.json info/exclude entry resolves against the runner cwd, not the checkout (PR #17 does not take effect)

2 participants