Skip to content

fix(miner): derive sdk changed files from git#5362

Merged
JSONbored merged 3 commits into
mainfrom
codex/fix-sdk-driver-lint-guard-vulnerability
Jul 12, 2026
Merged

fix(miner): derive sdk changed files from git#5362
JSONbored merged 3 commits into
mainfrom
codex/fix-sdk-driver-lint-guard-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The Agent SDK driver previously inferred changed files only from a narrow set of tool_use events, which allowed edits made by other mutating tools (e.g. Bash, MultiEdit) to go unreported and bypass downstream lint/typecheck guards.
  • The guard logic treats a successful driver result with an empty changedFiles array as nothing-to-check, so incomplete changed-file tracking is a security risk and must be derived from the actual worktree.

Description

  • Derive changed files from the actual worktree by running a git enumeration (git diff --name-only HEAD -- + git ls-files --others --exclude-standard) and merge those with any file_path observed in assistant tool_use blocks; added an injectable listChangedFiles hook for testability in packages/gittensory-engine/src/miner/agent-sdk-driver.ts.
  • Treat failure to enumerate the worktree as a fail-closed structured driver result (agent_sdk_changed_files_unavailable) so downstream guards cannot be skipped on an incomplete signal.
  • Relax foldAssistantMessage to record any input.file_path seen (supplemental metadata) and merge it with the git-derived file list before returning successful results.
  • Added regression tests covering: worktree enumeration of tracked+untracked edits, detection of untracked mutating tools via git, and enumeration failure handling in both the package and root unit suites (packages/gittensory-engine/test/agent-sdk-driver.test.ts, test/unit/agent-sdk-driver.test.ts).
  • Regenerated Cloudflare worker runtime types after the cf-typegen drift check reported stale output (worker-configuration.d.ts).

Testing

  • Ran the package unit suite with npm --workspace @jsonbored/gittensory-engine test, and all package tests passed.
  • Ran the focused repository unit run npm test -- --run test/unit/agent-sdk-driver.test.ts, and the unit tests passed (new tests included and green).
  • Ran a targeted coverage run for the added unit tests with lowered per-run thresholds (--coverage.thresholds.*=0) to validate coverage and test behavior locally, which completed successfully for the exercised tests.
  • Attempted the full local gate npm run test:ci; the run progressed through many early checks but the unsharded coverage phase did not complete in this environment (execution was terminated), so the full gate was not finished locally.
  • Attempted npm audit --audit-level=moderate; the registry audit endpoint returned 403 Forbidden in this environment so the dependency audit could not be completed.

Codex Task

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 12, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 2ff997e Commit Preview URL

Branch Preview URL
Jul 12 2026, 10:10 PM

@JSONbored JSONbored marked this pull request as draft July 12, 2026 14:57
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@gittensory-orb gittensory-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 12, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 12, 2026

Copy link
Copy Markdown

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-12 22:15:23 UTC

10 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This replaces the narrow tool_use-name allowlist (Edit/Write/NotebookEdit) with a git-derived worktree diff (tracked + untracked files) merged with any file_path seen on any tool_use block, and fails the whole result closed (agent_sdk_changed_files_unavailable) if git enumeration throws. The trace holds: `foldAssistantMessage` at agent-sdk-driver.ts:107-110 now unconditionally records `file_path`, and `listWorktreeChangedFiles` (agent-sdk-driver.ts:79-93) is invoked only on the already-established success path (agent-sdk-driver.ts:201), so failed attempts still correctly report `changedFiles: []` per the parity contract in coding-agent-driver-parity.test.ts. Tests exercise the real path via an actual temp git repo (not just a mocked function), which is the right level of rigor for this kind of fix.

Blockers

  • packages/gittensory-engine/src/miner/agent-sdk-driver.ts:81: `listWorktreeChangedFiles` only runs `git diff --name-only HEAD --`, so a reachable agent action like `Bash: git add src/changed.ts` makes the file disappear from both the unstaged diff and the untracked list; include staged changes with `git diff --cached --name-only HEAD --` and merge all three outputs.
Nits — 5 non-blocking
  • The PR description text provided doesn't show a linked issue number — per this repo's convention, confirm this closes/references an eligible open issue before merge.
  • agent-sdk-driver.ts:79-90 uses plain `git diff --name-only`/`git ls-files --others` with no rename detection (`-M`), so a renamed file could surface as a delete+add pair instead of one path — likely fine for this use case but worth a one-line comment if intentional.
  • Files created/edited by a mutating tool but excluded via `.gitignore` won't be picked up by `listWorktreeChangedFiles` (agent-sdk-driver.ts:79) since `ls-files --others --exclude-standard` respects ignore rules — probably desired, but worth confirming that's the intended scope for the guard.
  • Consider adding a short comment above `listWorktreeChangedFiles` noting that rename detection is intentionally off, to preempt a future contributor 'fixing' it into a behavior change.
  • If not already tracked, link this PR to the tracking issue in the description so the issue-closing convention is satisfied.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue — Link the relevant issue (for example `Closes #123`) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 44 registered-repo PR(s), 36 merged, 435 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 44 PR(s), 435 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 44 PR(s), 435 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@JSONbored JSONbored self-assigned this Jul 12, 2026
@JSONbored JSONbored force-pushed the codex/fix-sdk-driver-lint-guard-vulnerability branch from 438f581 to 4ef33f2 Compare July 12, 2026 21:39
@JSONbored JSONbored marked this pull request as ready for review July 12, 2026 21:39
@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.72%. Comparing base (29c18f2) to head (2ff997e).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5362   +/-   ##
=======================================
  Coverage   94.72%   94.72%           
=======================================
  Files         559      559           
  Lines       44664    44673    +9     
  Branches    14664    14667    +3     
=======================================
+ Hits        42308    42317    +9     
  Misses       1621     1621           
  Partials      735      735           
Flag Coverage Δ
shard-1 44.03% <7.69%> (-0.01%) ⬇️
shard-2 35.20% <30.76%> (-0.01%) ⬇️
shard-3 31.99% <7.69%> (-0.01%) ⬇️
shard-4 32.05% <30.76%> (+<0.01%) ⬆️
shard-5 32.68% <100.00%> (+0.01%) ⬆️
shard-6 43.69% <7.69%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...es/gittensory-engine/src/miner/agent-sdk-driver.ts 100.00% <100.00%> (ø)
...ages/gittensory-engine/src/miner/driver-factory.ts 100.00% <100.00%> (ø)
.../gittensory-miner/lib/coding-agent-construction.js 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot had a problem deploying to preview/pr-5362 July 12, 2026 21:42 Failure
@gittensory-orb gittensory-orb Bot added the manual-review Gittensor contributor context label Jul 12, 2026
@JSONbored JSONbored force-pushed the codex/fix-sdk-driver-lint-guard-vulnerability branch from 4ef33f2 to c86d328 Compare July 12, 2026 21:50
…path too

Reconciling this branch with #5356 (costUsd wiring) surfaced a gap: the new
changed-file-enumeration-failure return statement ran after the SDK result
message (and its billed total_cost_usd) was already available, but omitted
costUsd -- silently undercounting real spend for that one failure path in
budgetSpent. Added it, plus regression coverage in both the node:test source
and its vitest/Codecov mirror.
@JSONbored JSONbored force-pushed the codex/fix-sdk-driver-lint-guard-vulnerability branch from c86d328 to bcce71c Compare July 12, 2026 22:06
…eal test seam

Reconciling this branch's default agent-sdk changed-file enumeration (now a
real git diff/ls-files read) surfaced that createCodingAgentDriver,
constructProductionCodingAgentDriver, and runCodingAgentAttempt never forwarded
an injectable enumerator -- every pre-existing test that constructs a real
agent-sdk driver against a fake (non-git) working directory now fails closed
via the real default, since there was no seam to opt out with. Threaded
listChangedFiles through all three layers (same pattern as query/hooks) and
updated the 4 affected test files (contract parity, coding-agent-miner,
miner-coding-agent-construction, miner-coding-agent-house-rules) to inject a
no-op enumerator where they test other behavior.
@JSONbored JSONbored merged commit 20dc547 into main Jul 12, 2026
19 checks passed
@JSONbored JSONbored deleted the codex/fix-sdk-driver-lint-guard-vulnerability branch July 12, 2026 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark codex gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant