Skip to content

fix(runner): resolve unresolved review feedback on write anchoring and local review scope - #22

Merged
kodiakhq[bot] merged 15 commits into
mainfrom
fix/unresolved-greptile-feedback
Aug 10, 2026
Merged

fix(runner): resolve unresolved review feedback on write anchoring and local review scope#22
kodiakhq[bot] merged 15 commits into
mainfrom
fix/unresolved-greptile-feedback

Conversation

@RedStar071

@RedStar071RedStar071 commented Aug 10, 2026

Copy link
Copy Markdown
Member

Resolves the unresolved Greptile review threads left on merged PRs #2, #7, #16, and #19.

Fallback directory anchor could be redirected outside the checkout (#16, packages/runner/src/boundary.ts). When /proc/self/fd is unavailable, the old fallback verified the parent directory's inode and then returned its mutable pathname, so a concurrent rename-to-symlink after the comparison could redirect the commit rename outside the checkout. Node exposes no descriptor-relative create/rename, so the write now fails closed:

asyncfunctiondirectoryAnchor(directory: FileHandle,original: string): Promise<string>{constdescriptor=`/proc/self/fd/${directory.fd}`;try{awaitlstat(descriptor);returndescriptor;}catch{thrownewRunnerWriteDeniedError(original,'descriptor-anchored writes are not supported on this platform',);}}

A new test drives the write path through a dead descriptor and asserts the write is refused and the target untouched. Reads are unaffected; on platforms without /proc/self/fd the runner is effectively read-only, which matches the repository's fail-closed safety posture.

Local proactive review omitted pending changes (#19, packages/runner/src/boundary.ts). A range-less review ran a plain git diff, which ignores index-only changes and untracked files. reviewFiles now unions git diff --name-only --cached, git diff --name-only, and git ls-files --others --exclude-standard. The range-less context diff is a single consolidated git diff HEAD patch (one final-state block per file, so a partially-staged edit never surfaces its intermediate staged value as a separate patch), plus a synthetic git diff --no-index /dev/null <path> creation patch for each untracked file so their content reaches the reviewer; a repository without commits falls back to the staged and working-tree layers. Pull-request base/head ranges keep the previous fixed-range behavior.

Autofix scope exceeded the validated finding (#19, packages/agent/src/agent.ts).scopeChanges unioned finding.files with input.files, so on proactive runs every file in the review diff was writable even when the accepted finding and its evidence cited only one. The scope is now the validated finding's cited files only; changes to other review-diff files are refused to needs-human, with a regression test.

Leftovers from #2 and #7. The #2 threads target the Nitro server, which #21 removed and #24 restores with the routes registered and AGENT_ZERO_PORT honored, so .env.example keeps that entry. The #7 thread (git-hook workflow documentation) is already satisfied by the current CONTRIBUTING.md "Style and commits" section; no change needed.


View with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is enabled.

Confidence Score: 5/5

No blocking failure remains.

Executed repository scenarios confirm the updated review context behaves correctly for the exercised local-change workflows.

T-Rex T-Rex Logs

What T-Rex did

  • Ran a focused executable harness using /usr/bin/tsx against both the merge-base and current boundary.ts implementations to exercise repository-context scenarios.
  • Compared the baseline behavior to the current PR and confirmed the baseline failed all five repository-context hypotheses while the current PR passed all five, validating the updated review-context behavior.

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (10): Last reviewed commit: "fix(runner): render omitted review patch..." | Re-trigger Greptile

RedStar071and others added 2 commits August 10, 2026 07:27
…d local review scope
Addresses the unresolved Greptile threads from PRs #2, #7, #16, and #19:
- packages/runner: fail closed when /proc/self/fd is unavailable instead of
renaming through a re-resolved mutable pathname (PR #16 thread).
- packages/runner: range-less local reviews now include index-only changes and
untracked files in both the changed-file list and the diff context (PR #19 thread).
- packages/agent: planned writes are restricted to the files the validated
finding cites; review-diff membership alone no longer authorizes a write (PR #19 thread).
- .env.example: drop AGENT_ZERO_PORT, which referenced the removed Nitro server (PR #2 threads).
- PR #7 thread (hook workflow docs) is already covered by CONTRIBUTING.md.
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment threadpackages/runner/src/boundary.ts Outdated
Comment threadpackages/runner/src/boundary.ts Outdated
Comment threadpackages/runner/src/boundary.ts
RedStar071and others added 2 commits August 10, 2026 07:41
…ntracked content
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment threadpackages/runner/src/boundary.ts Outdated
…ate patches
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment threadpackages/runner/src/boundary.ts Outdated
…vive
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
greptile-apps[bot]
greptile-appsBot previously approved these changes Aug 10, 2026
kodiakhqBotand others added 2 commits August 10, 2026 08:10
The Nitro control plane is being restored in #24 and honors this variable,
so dropping it as a leftover of the removed server no longer applies.
Comment threadpackages/runner/src/boundary.ts Outdated
…ff once
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment threadpackages/runner/src/boundary.ts Outdated
RedStar071and others added 2 commits August 10, 2026 08:32
… markers
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…location
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment threadpackages/runner/src/boundary.ts Outdated
RedStar071and others added 2 commits August 10, 2026 08:45
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…test
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment threadpackages/runner/src/boundary.ts Outdated
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment threadpackages/runner/src/boundary.ts Outdated
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
@kodiakhq
kodiakhqBot merged commit c3642cf into mainAug 10, 2026
14 checks passed
@kodiakhq
kodiakhqBot deleted the fix/unresolved-greptile-feedback branch August 10, 2026 09:18
Sign up for freeto 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

@RedStar071