Skip to content

[codex] expose cached repository state in status - #849

Open
KooshaPari wants to merge 7 commits into
kunchenguid:mainfrom
KooshaPari:worktrees/upstream-diagnostics-rebuild-v2-20260826
Open

[codex] expose cached repository state in status#849
KooshaPari wants to merge 7 commits into
kunchenguid:mainfrom
KooshaPari:worktrees/upstream-diagnostics-rebuild-v2-20260826

Conversation

@KooshaPari

@KooshaPari KooshaPari commented Aug 26, 2026

Copy link
Copy Markdown

Summary

This is a clean reconstruction of the cached repository-state diagnostics on current upstream main. It supersedes the contaminated pipeline candidate without rewriting or deleting that preserved history.

no-mistakes status always renders explicitly labelled cached local evidence: branch, short HEAD, clean/dirty state, a local reason when present, and existing branch-sync guidance. The status fingerprint includes this rendered value, so sampled telemetry follows visible state.

Safety boundary

The implementation uses branchsync.Service.InspectCached, which reads local Git and gate state only. It performs no fetch or remote query, makes no remote-freshness claim, and does not mutate refs, index, worktree, or the gate database. The old fork experiment and tracked binary remain excluded.

Verification

  • Command-level clean/dirty status regression coverage.
  • Regression wrapper records and rejects git fetch and git ls-remote; the test snapshots Git metadata, refs/worktree state, and gate DB/WAL.
  • Presenter and telemetry-fingerprint tests.
  • make lint
  • go test -race ./...
  • go build -o ./bin/no-mistakes ./cmd/no-mistakes
  • git diff --check

Hosted CI and maintainer review remain required before merge. GitHub currently requires an upstream administrator to approve first-time fork workflow runs; this contributor token lacks that permission (HTTP 403).

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

This is a draft, so it will not be merged. Inspected the same cached-status intent as #846: no-mistakes status always renders branchsync.Service.InspectCached (local/read-only, labelled cached). No fetch, no ref/index/worktree mutation, no pipeline.

VISION.md:

  • R1: aligns. Status output only; core pipeline unchanged.
  • R2: aligns. InspectCached is read-only.
  • R3: aligns.
  • R4: aligns.
  • R5: aligns. Output says cached.
  • R6: aligns.
  • R7: aligns.

Please mark ready when you want hosted CI. First-time fork workflows will need a maintainer approval after that. If this replaces #846 (that one is now DIRTY vs main), please close #846 so we are not tracking two copies.

@KooshaPari
KooshaPari marked this pull request as ready for review August 26, 2026 23:00
Copilot AI lite review requested due to automatic review settings August 26, 2026 23:00

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported unavailable-state rendering, hidden-guidance, and cached Git-object mutation paths are addressed at the current head.

Reviews (4): Last reviewed commit: "fix(sync): defer cached gate recovery pr..." | Re-trigger Greptile

Comment thread internal/cli/status.go
Comment thread internal/cli/status_test.go
@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Re-triaged HEAD f8205497 after you left draft and pushed the read-only regression. First-time fork CI approved (CI / docs / Guard / Require).

Inspected internal/cli/status.go cachedBranchSummary and branchsync.worktreeClean: when Git cannot determine cleanliness, worktreeClean returns (false, "status_unavailable"), and this presenter maps !Clean to dirty (with that reason). Greptile P1 is right — unknown must not render as confirmed dirty. Please fail closed to an unavailable/unknown cleanliness label (or keep the unavailable branch path) when Reason is status_unavailable / Clean is not positively known. Greptile P2 (mutation snapshot omitting .git/objects / diverged merge-tree --write-tree path) is worth covering if InspectCached can take that path under status.

Otherwise the change is still local InspectCached only, labelled cached, no fetch/remote claim, fingerprint includes the rendered summary. Not merging while Greptile is red and that P1 is open. Hosted CI will run after the approve.

VISION.md:

  • R1: aligns. Status output only; core pipeline unchanged.
  • R2: aligns. InspectCached remains read-only.
  • R3: aligns.
  • R4: aligns.
  • R5: does not align until unavailable cleanliness stops reading as dirty (stall/lie risk for agents parsing status).
  • R6: aligns intent; agents need an honest cleanliness signal.
  • R7: aligns once the unavailable case is fixed.

#846 is still an open draft of the same idea — please close it when this one is the successor so we are not tracking two copies.

Comment thread internal/cli/status.go
Comment thread internal/branchsync/sync.go
@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Re-triaged HEAD 15796dcb after your Greptile follow-ups. Inspected cachedBranchSummary (status_unavailable → cleanliness unavailable, keeps sync guidance) and recoverySourceState (no merge-tree --write-tree on cached/check classification; explicit Recover keeps the proof). That also covers the mis-labelled preserved_head_missing path reported in #856.

First-time fork workflows re-approved on this head (CI/docs/Guard). Greptile still running at stamp time. Corrective/diagnostic status + fail-closed recovery classification; not a default gate-pass change. Waiting on hosted CI + green Greptile before merge.

VISION.md:

Please close draft #846 if this remains the successor.

@chris-fenton-rg

Copy link
Copy Markdown

Field incident from #856 confirms the recovery-planning bug on v1.57.0.

The failed run records pipeline head 828b9584ba1b9453560a4cd2a3c9d4bed4a7a938. That exact commit is present at both:

  • refs/heads/fm/emr-1453-test-fix
  • refs/no-mistakes/recover/01M0ZX7DTJDMBHX195HCY1TG4H

in the run's gate repository, with a second complete copy in the eval pool. However, axi sync --check reports blocked_recover_preserved_head_missing and only inspect_and_reconcile_manually.

Please retain a regression for this exact gate-only, non-ancestral recovery-ref case. The preserved invoking branch is clean at b814cba1023e0529fafeccd23de1ab2a4911d81d; no recovery or ref mutation has been attempted.

@KooshaPari

Copy link
Copy Markdown
Author

Hosted e2e diagnosis for 15796dc: the sole failure is a timing-dependent test-template copy race on transient .git/objects/maintenance.lock, not the cached branch-sync change. The PR diff does not touch internal/pipeline/steps; the focused reconciliation test passed 20 consecutive local runs, and build/check/all four platform test jobs plus Greptile passed.

I attempted a failed-job rerun, but GitHub rejected it because this account lacks repository-admin rights. I am preserving the exact reviewed head rather than adding unrelated test-fixture churn. A maintainer rerun of 33040340000 is the remaining verification gate; if it repeats, the narrow follow-up is to disable automatic Git maintenance in the shared test template, not broadly ignore copy errors.

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Hosted CI on 15796dcb is green except e2e (author diagnosed a maintenance.lock template-copy race unrelated to this diff). Greptile is 5/5.

You opened successor #860 as a clean replay onto current main. Please close this PR in favor of #860 so we only track one copy (same as the earlier #846 ask). #856 stays pointed at the recovery-classify fix landing with that successor.

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.

4 participants