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

@RedStar071 RedStar071 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:

async function directoryAnchor(directory: FileHandle, original: string): Promise<string> {
  const descriptor = `/proc/self/fd/${directory.fd}`;
  try {
    await lstat(descriptor);
    return descriptor;
  } catch {
    throw new RunnerWriteDeniedError(
      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

RedStar071 and 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 thread packages/runner/src/boundary.ts Outdated
Comment thread packages/runner/src/boundary.ts Outdated
Comment thread packages/runner/src/boundary.ts
RedStar071 and 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 thread packages/runner/src/boundary.ts Outdated
…ate patches

Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment thread packages/runner/src/boundary.ts Outdated
…vive

Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
greptile-apps[bot]
greptile-apps Bot previously approved these changes Aug 10, 2026
kodiakhq Bot and 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 thread packages/runner/src/boundary.ts Outdated
…ff once

Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment thread packages/runner/src/boundary.ts Outdated
RedStar071 and 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 thread packages/runner/src/boundary.ts Outdated
RedStar071 and 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 thread packages/runner/src/boundary.ts Outdated
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment thread packages/runner/src/boundary.ts Outdated
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
@kodiakhq
kodiakhq Bot merged commit c3642cf into main Aug 10, 2026
14 checks passed
@kodiakhq
kodiakhq Bot deleted the fix/unresolved-greptile-feedback branch August 10, 2026 09:18
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