Skip to content

fix(server): preserve racy edits in review diff previews - #12613

Merged
shivamhwp merged 3 commits into
pingdotgg:mainfrom
tris203:t3code/investigate-flaky-ci-test
Sep 24, 2026
Merged

shivamhwp merged 3 commits into
pingdotgg:mainfrom
tris203:t3code/investigate-flaky-ci-test

Conversation

@tris203

@tris203 tris203 commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Review diff previews could omit rapid, same-size edits when untracked files were present, causing the intermittent keeps complete stats for files beyond the combined patch limit failure seen in #12604. Copying the Git index gave it a newer timestamp and disabled Git's racy-entry content check.

Restore a conservative source timestamp on the temporary index before Git reads or rewrites it, following the existing checkpoint approach. Add deterministic regression coverage for whole-second and fractional timestamps with both regular and split indexes, including checks that the real index remains unchanged.

Validation: both regression cases failed before the fix; all 86 GitVcsDriverCore.test.ts tests pass afterward. Server typecheck and formatting pass; targeted lint reports one pre-existing warning.

Implemented with GPT-6 in the Codex harness.

Summary by CodeRabbit

  • Bug Fixes
    • Improved review diff previews for repositories with racy index timestamps, including split-index configurations.
    • Modified and untracked files now appear correctly in previews, with accurate change counts and diff content.
    • Previewing changes preserves the original repository index, including its contents and timestamp.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 19, 2026
Comment thread apps/server/src/vcs/GitVcsDriverCore.ts
@macroscopeapp

macroscopeapp Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at dcb5811

Macroscope's review found this PR approvable — This narrowly scoped server fix corrects racy Git index handling during review diff preview generation without modifying the real repository index. Targeted tests cover normal and split-index timestamp cases and verify that existing index contents remain unchanged.

Notes:

  • No code objects were reviewed. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: e622b600-02d6-4d3c-b450-eb6a1376ff1f

📥 Commits

Reviewing files that changed from the base of the PR and between dcb5811 and 4a8e3d9.

📒 Files selected for processing (2)
  • apps/server/src/vcs/GitVcsDriverCore.test.ts
  • apps/server/src/vcs/GitVcsDriverCore.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The review preview now adjusts temporary index timestamps before diff calculation. Parameterized tests cover racy indexes with split and non-split indexes, tracked-file edits, untracked files, diff content, and original index preservation.

Changes

Racy index handling

Layer / File(s) Summary
Temporary index timestamp handling and validation
apps/server/src/vcs/GitVcsDriverCore.ts, apps/server/src/vcs/GitVcsDriverCore.test.ts
prepareReviewIndex sets the temporary index atime and mtime below the source index mtime. Tests cover whole and fractional timestamps, split and non-split indexes, tracked and untracked file diffs, and unchanged original index bytes and mtime.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 4a8e3

The review preview fix appears mergeable after normal checks; no actionable risk is established.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: preserving racy edits in server review diff previews.
Description check ✅ Passed The description explains what changed, why it changed, the regression coverage, and validation results. It does not use the template headings or include the checklist, but it provides the required sub…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@shivamhwp
shivamhwp merged commit b2b43be into pingdotgg:main Sep 24, 2026
22 checks passed
@tris203
tris203 deleted the t3code/investigate-flaky-ci-test branch September 24, 2026 10:20
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 24, 2026
## What's Changed
* chore: add scratchyone to vouched list by @t3-code[bot] in pingdotgg/t3code#13353
* feat(observability): honor the OpenTelemetry kill switch by @juliusmarminge in pingdotgg/t3code#13355
* fix(server): preserve racy edits in review diff previews by @tris203 in pingdotgg/t3code#12613


**Full Changelog**: pingdotgg/t3code@v0.0.43-nightly.20260924.2187...v0.0.43-nightly.20260924.2200

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.43-nightly.20260924.2200
sheehanmunim added a commit to munimtechnologies/mtcode that referenced this pull request Sep 24, 2026
Conflicts, resolved onto the fork's versions:
- theme-token restyle (pingdotgg#13371, pingdotgg#13397): re-applied upstream's token edits
  (text-2xs, rounded-sm, bg-(--terminal-background), dropped panel max-h and
  heading tracking) onto the fork's restructured CommandPalette, composer
  chip, terminal drawer and settings panels.
- sidebar stage art: followed upstream in dropping the focus-ring offset
  helper; its fork test is gone and the compact-crop test now checks the
  new data-stage-art marker.
- review diff index (pingdotgg#12613): took upstream's rounded-down index mtime,
  which supersedes the fork's same-second copy.
- preview broker tests: kept the fork's profile-routing tests beside
  upstream's live-tab-owner tests.
- mobile environment row (pingdotgg#13302): kept the fork's rename button and added
  upstream's opensDetails chevron and switch alignment; the new environment
  detail screen now passes onRename too.
- AgentAwarenessRelay test: upstream's new ServerEnvironment mock gains the
  fork's setEnvironmentLabel.
- pnpm-lock.yaml regenerated from the merged manifests.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
aorwall added a commit to aorwall/t3code that referenced this pull request Sep 27, 2026
Merges `pingdotgg/t3code` `78af372cf..ebdcda1` (39 commits) into the
fork. The merge PR is not stacked; it branches from `main`.

## Resolution
- **8 conflicts.** Each was resolved using the verdict `preflight.mjs`
gave for it. The per-file details are in
`docs/fork/upstream-merge-log.md`.
- `apps/server/src/cli/pair.ts` stays deleted, as listed in
`deletedUpstreamPaths`.
- `ThreadPullRequestsPanel.tsx` takes upstream. Upstream pingdotgg#13061 now
names the repository on each linked PR, so the fork's delta there has
converged and the file is identical to upstream.
- **New gate.** Upstream's chat code-block **Run** button (pingdotgg#13060) runs
`scripts.run` with a synthetic `chat-code-block` id. The Moatless
backend rejects that id with `ScriptNotFound`, so `ChatView` hides the
button on environments that report `workspaceScripts`. This is recorded
in the inventory (`host-run-scripts`) and in `gaps.md`.
- **Lint.** Upstream pingdotgg#13371/pingdotgg#13397 made `shadcn(no-arbitrary-values)` a
lint error, which flagged 130 hits in fork code, mostly
`settings/moatless/**`. I mapped each one onto the type scale the same
way upstream mapped its own: `text-[13px]` becomes `text-sm`, `[11px]`
becomes `text-2xs`, `[10.5px]` and `[.65rem]` become `text-3xs`, and so
on. Some Moatless settings text therefore renders at slightly different
sizes.
- **Type fix.** The fork's Mermaid `MarkdownCodeBlock` now passes
upstream's required `isStreaming` prop.
- **Lockfile.** `pnpm-lock.yaml` was re-derived with `install.mjs`.
- **File counts.** 515 files landed, against 489 in the upstream range.
The fork delta is 756 files. The gap is explained in the log entry.

## Verification
- A full run of `verify.mjs` passes all 10 checks, tests included.
- The unsupported-method derivation reports no ADD and no DROP.
- `resolution-check.mjs` and `duplicate-adds.mjs` report nothing.

## Upstream feature classification

### Usable as-is
- The sidebar Back button always returns to the main app (pingdotgg#13516),
including Escape on settings.
- Linked PRs show repository names (pingdotgg#13061).
- Colors come from theme tokens, text uses scale values, and disabled
controls have consistent opacity (pingdotgg#13371, pingdotgg#13397, pingdotgg#11441).
- Working and monitoring threads fade in the sidebar again (pingdotgg#13506).
Sidebar terminal pulses stay in sync (pingdotgg#12962).
- Selected text stays visible on a revealed file line (pingdotgg#13548).
- The usage page has a keybinding, `usage.open` (pingdotgg#9434).

### Unsupported in Moatless / needs implementation
- **Per-thread auto-settle switch (pingdotgg#11846).** This adds the
`thread.auto-settle.set` command, the `thread.auto-settle-set` event,
and the `threadAutoSettleOptOut` capability. Moatless never auto-settles
and does not report the capability, so the menu item stays hidden.
- **Run a shell command from a chat code block (pingdotgg#13060).** Gated off on
`workspaceScripts` environments. It needs `scripts.run` to accept an
inline command.
- **iPhone Duo 3D device controls (pingdotgg#12813).** Behind
`FEATURES.deviceHub`, which is off.
- **One-click Grok CLI update (pingdotgg#13523).** Provider management is behind
`FEATURES.providerConfiguration`, which is off.
- **Relay/tunnel cleanup and a managed endpoint reaper (pingdotgg#9386,
pingdotgg#13528).** Also the forced relay deploy workflow (pingdotgg#13550). Relay is
decided out of the fork.
- **Desktop update reconnect (pingdotgg#12006).** Electron is not a target.

### Backend behavior to consider reproducing in Moatless
- **Racy edits in review diffs (pingdotgg#12613).** The review index copy rounds
its mtime down so that edits made in the same second as the copy still
appear in the diff (`apps/server/src/vcs/GitVcsDriverCore.ts`).
- **Codex 0.156 (pingdotgg#13481, pingdotgg#13480).** Codex 0.156 is now the minimum, and
the app-server protocol was regenerated
(`packages/effect-codex-app-server`).
- **Claude banked resets (pingdotgg#13118).** Upstream shows and redeems them
(`claudeResetCredits.ts`, `resetCreditCoordinator.ts`). Separately, Grok
account emails are reported so usage limits merge across environments,
and Grok accounts no longer vanish (pingdotgg#12588, pingdotgg#12799). Codex keeps its
reset answer when the re-probe fails (pingdotgg#13363).
- **ACP (pingdotgg#13386).** Keeps one answer when a running tool reports
progress.
- **Antigravity (pingdotgg#13388, pingdotgg#13389).** Stop ends commands that outlived
their turn, and Windows unpacking stays under MAX_PATH.
- **Streamed section titles (pingdotgg#13504).** A title waits for the text
beneath it.
- **OpenTelemetry (pingdotgg#13355, pingdotgg#13469).** The kill switch is honored, and a
malformed `OTEL_RESOURCE_ATTRIBUTES` no longer stops startup.
- **Settlement.** If Moatless ever adds automatic settlement, it must
honor the per-thread opt-out from pingdotgg#11846.

The gaps register (`docs/fork/gaps.md`) was updated for the auto-settle
capability, the chat-run gate, and three runtime-fix bullets.

## Left undone
- Four files that auto-merged cleanly carry small fork deltas with no
path-policy entry: `AgentsPanel.tsx`, `ThreadTerminalDrawer.tsx`,
`client-runtime/src/state/threadDetail.ts` and `threadReducer.test.ts`.
- `preflight.mjs` labels a modify/delete conflict on a
`deletedUpstreamPaths` entry as `[unlisted]`. That is a minor script bug
and was not fixed here.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---
Moatless task:
https://moatless.soaplabstest.com/tasks/8f4381ee-c9ed-4fc5-9ffd-039ee15731ca
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants