Skip to content

fix(diagnostics): all-LSP own-file drift (refs #3011) - #3009

Merged
apmantza merged 3 commits into
apmantza:masterfrom
nzbrian:self-drift-all-lsp-own-file
Sep 15, 2026
Merged

apmantza merged 3 commits into
apmantza:masterfrom
nzbrian:self-drift-all-lsp-own-file

Conversation

@nzbrian

@nzbrian nzbrian commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix HIGH F1 from PR #3009 / issue #3011. The fix is pushed at exact head
3cb0d4b9c0f76aaf14b6e0beec4bef12ea208a73, rebased from input head
a26c87b905ece9e3a7b3cd7452140a081234db47. All-LSP inline blockers with an
available size and sha256 baseline now force content confirmation even when the
file mtime is at or before the recorded baseline. Non-LSP records retain the
mtime fast path, and records without a hash retain the no-baseline mtime
fallback.

Finding HIGH F1: fixed. The rebased two-file diff's missing changelog fragment:
fixed with exactly one new .changelog/fix-3011-all-lsp-content-freshness.md.

Tests

  • Red-first real production path, before the fix:

    FAIL tests/clients/blocker-freshness-external-rewrite.test.ts > all-LSP own-file freshness via the content axis (#2982 remainder) > demotes a same-length all-LSP rewrite that lands at-or-before the baseline
    AssertionError: expected +0 to be 1
    

    This entered through RuntimeCoordinator.recordInlineBlockers and
    sweepInlineBlockerFreshness.

  • Compile-valid mutation (forceContent: false) after the fix:

    FAIL tests/clients/blocker-freshness-external-rewrite.test.ts > all-LSP own-file freshness via the content axis (#2982 remainder) > demotes a same-length all-LSP rewrite that lands at-or-before the baseline
    AssertionError: expected +0 to be 1
    
  • npm run build: passed before each test run and after mutation restoration.

  • Focused direct callers: 56/57 tests passed across the external-rewrite,
    blocker-freshness, delivery-cap, widget-population, widget delivery-cap, and
    hook-await registry suites. The one failure is the known offline Java grammar
    fixture (resolves a .java blocker's forward imports); 24/25 tests in that
    file passed.

  • npm run lint: passed.

  • npm run fmt:check: passed.

  • npm run changelog:check and node scripts/check-changelog-fragments.mjs:
    passed.

  • npm run preflight: blocked after 90 seconds with no gate output; interrupted
    because the environment cannot prefetch the tree-sitter grammars.

Blast radius

RuntimeCoordinator.recordInlineBlockers
  -> sweepInlineBlockerFreshness
    -> detectSelfDrift
      -> bounded(stat/readFile)
    -> detectDrift (forward-import axis)

Changed production symbols are detectSelfDrift and its single caller in
sweepInlineBlockerFreshness. The new forceContent value is true only for
all-LSP records with both baseline fields. Non-LSP callers and no-baseline LSP
fallback behavior remain unchanged. The hook-await registry entry was refreshed
because the changed call shape receives a new source key. No durable record,
host adapter, MCP route, generated JavaScript, dist/, or CHANGELOG.md
changed.

Class sweep

Swept detectSelfDrift, detectDrift, sweepInlineBlockerFreshness, all
recordInlineBlockers consumers, all recordedSize/recordedHash users, and
the bounded() call-site registry. The self-drift population remains folded
onto the shared detectSelfDrift seam. Widget rows stay distributed because
they have no re-arming setter or content baseline. Language coverage is through
the registry-backed LSP provenance; the non-TypeScript Java forward-import case
was exercised and is the only offline grammar failure.

Verdict: fold all-LSP and non-LSP inline records onto the existing content seam;
keep widget dependency rows separate by contract.

freshnessFromMtime / statMtimeMs consumers (#3011)

Every consumer of freshnessFromMtime / bare statMtimeMs in clients/, with the per-member verdict on whether its real question is "written after T?" (mtime is the right instrument) or "do the bytes still match?" (mtime is a blind proxy):

Site Real question Mtime the right instrument?
blocker-freshness.ts detectSelfDrift mtime gate performance fast-path (skip hash) yes — not a final arbiter
blocker-freshness.ts detectDrift own-file fallback "bytes still match?" — blind proxy no — known limitation: no-baseline fallback, explicitly fail-open (pre-#2982)
blocker-freshness.ts detectDrift import walk "written after T?" yes
project-diagnostics/cache.ts "written after T?" yes
advisory-provenance.ts "written after T?" yes
lsp/workspace-diagnostics-cache.ts "written after T?" yes
widget-state.ts (3 sites) "written after T?" yes
observed-mutation.ts "settled before T?" yes
opaque-mutation-scan.ts "written after T?" yes

Consolidation verdict: the single blind-proxy site is the no-baseline own-file fallback, which is deliberately fail-open and out of #3011's scope — the defect here is the baseline-present case, which the content axis now owns. No other consumer uses mtime as a content-match proxy.

Observability

The new decision is covered by existing record self-drift-unverifiable at
clients/blocker-freshness.ts:1017, which observes the content-confirmation
failure/timeout path. selfUnverifiable, revalidated, selfHealed, and
hashBudgetExhausted counters preserve the existing bounded, fail-closed
observability.

Test assessment

The regression and inverse touch case use the real coordinator and freshness
sweep. The new force guard is independently mutation-tested and the mutated
compiled production path reds with the same assertion as the original defect.
The no-baseline mtime fallback and non-LSP mtime fast path remain covered.

The prior exact-head CI for a26c87b905ece9e3a7b3cd7452140a081234db47
passed required Lint & type-check and Unit tests, with changelog fast-fail
passing; its advisory PR-body check failed and Windows later passed. Fresh
exact-head CI for 3cb0d4b9c0f76aaf14b6e0beec4bef12ea208a73 is required after the
fix push and is not yet claimed green.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for your first pull request! A maintainer will review it soon. Please make sure CI passes and the PR template is filled out.

@apmantza

Copy link
Copy Markdown
Owner

Thanks for this — and specifically for the stacking note. I verified the claim mechanically before reading any code:

$ git merge-base --is-ancestor <2983-head a76ebc4> <3009-head f3b87aa>  -> YES
$ git log --oneline a76ebc4..f3b87aa
f3b87aaf9 fix(diagnostics): content-confirm the all-LSP own-file in the turn-end sweep (refs #2982)
$ git diff --stat a76ebc4..f3b87aa
 clients/blocker-freshness.ts                          | 146 ++++++++----
 tests/clients/blocker-freshness-external-rewrite.test.ts | 174 +++++++++++++++
 2 files changed, 286 insertions(+), 34 deletions(-)

Exactly the two files you named, one commit, no drive-by edits. The 11-file / +1331 view against master is entirely #2983 underneath, as you said. That made this triage cheap, which is the point of writing it down — thank you.

Status: blocked on #2983, not on you

This is queued behind its parent. #2983 is CONFLICTING and is waiting on its author for an open HIGH on the baseline lifecycle — and this PR reuses that exact seam (setInlineBlockerContentBaseline's capture and the re-arming setter), so whatever resolves there lands on top of this. Nothing to do here until #2983 settles.

Two mechanical items, for whenever you next touch the branch

  1. One conflict against master, in tests/config/hook-await-bounds.test.ts — the turn_end/agent_end/session_start must not block: per-hook wall budgets + bounded() at every hook await + off-hook delivery for the remainder (refs #2504, #2509) #2523 bounded() call-site registry. master has moved under it; your single-call-site consolidation is the right shape, it just needs the current registry contents. No other file conflicts.

  2. The PR title needs an issue reference. It currently carries (stacked on #2983), which is a PR, not an issue, and the title gate looks for an issue ref. Your commit subject already has the right one — refs #2982 — so mirroring that into the title is all it needs. Keep the "stacked on" note in the body where it is genuinely useful.

"Allow edits from maintainers" is on, so if you would rather we carry both of those, say the word and we will commit them to your branch as review deltas with your authorship intact.

One thing review will probe hardest, flagged early

The size gate moving ahead of the mtime gate is the substance of this change, and the reasoning for it is sound — an out-of-band write really can land at-or-before recordedAtMs, so mtime alone is blind to precisely the case this axis exists to catch.

The consequence worth checking is what the reorder does to records that have no content baseline. recordedSize is optional and setInlineBlockerContentBaseline is attached off the dispatch path, so undefined is reachable. Before the reorder, such a record reached freshnessFromMtime first and could come back "unchanged"; after it, detectSelfDrift returns "unverifiable" at the new early return before mtime is ever consulted. On the non-LSP axis "unverifiable" means "decide nothing" — which also means setSelfDrift(false) is not called, so a record that was previously demoted no longer has a path back to authoritative.

If that is right, it is a behavior change to #2983's existing axis rather than to the new all-LSP one, and it would not show up in tests that always capture a baseline. If it is wrong — because recordedSize is in practice always present wherever setSelfDrift is — then the new undefined early return is unreachable and that is worth a line in the body either way. A test with the baseline deliberately absent, asserting the re-arm still happens, would settle it in both directions.

This is flagged as an angle, not a finding: no probe has been run against it yet. Full adversarial review comes once #2983 clears and this rebases to its two-file form.


This comment was written by an AI agent working on this repository, with maintainer supervision.

@apmantza

Copy link
Copy Markdown
Owner

Correction to item 2 above, after reading #2982's acceptance criteria properly.

I first read your refs #2982 as right and #2983's closes #2982 as premature. Rechecking against what #2982 actually reports, it is the other way round:

So refs #2982 would leave your work tracked on an issue that is about to close, describing a defect it does not report. I have filed #3011 for the mtime-blind own-file check, with your PR named in it, so please use refs #3011 in the title instead of what I suggested.

Sorry for the churn — you had the code boundary right, I had the issue boundary wrong.

Two things in #3011 that go slightly beyond your current diff, both standard for this repo rather than anything specific to your patch:

  • A class sweep over the other consumers of freshnessFromMtime and bare statMtimeMs comparisons, with a per-member verdict on whether that member's real question is "was this written after T" (mtime is the right instrument) or "do the bytes still match" (mtime is a blind proxy). We treat a fix's pattern as more important than its instance, so the sweep is part of the fix rather than a follow-up.
  • An explicit, tested statement of what happens when the content axis cannot decide — which is the same thing my angle above was circling. Turn-end sweep re-serves all-LSP blockers after an out-of-band rewrite: the own-file check is mtime-only #3011 makes it an acceptance criterion in both directions, so whichever way the recordedSize === undefined question resolves, the answer ends up pinned by a test.

Happy to take the sweep off your plate if you would rather keep this to the two files — say so and we will do it as a maintainer commit on your branch, or as a separate PR that lands first.


This comment was written by an AI agent working on this repository, with maintainer supervision.

@github-actions github-actions Bot added the conflict Merge-train warden - mergeStateStatus is DIRTY; required checks are silently skipped until resolved label Sep 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This PR is merge-conflicted; required checks are silently skipped until resolved.

@github-actions

Copy link
Copy Markdown
Contributor

Merge-train warden: GitHub never dispatched CI for this head.

No run exists for .github/workflows/ci.yml or .github/workflows/lint.yml on bffd29fecd90d3b599772b5f600cc448479f8af7, 239 minutes after the head commit. The pull_request dispatch was dropped.

The required checks here are ABSENT, not passing. Nothing may read this PR as green until a run exists and concludes.

The warden cannot push, so it cannot recover this itself. Re-dispatch with an empty commit on the branch, or close and reopen the PR.

Rebase the all-LSP own-file content confirmation onto the merged
blocker-freshness baseline and preserve the production baseline seam.

Refs apmantza#3011
@apmantza
apmantza force-pushed the self-drift-all-lsp-own-file branch from bffd29f to a26c87b Compare September 14, 2026 16:28
@apmantza apmantza changed the title fix(diagnostics): content-confirm the all-LSP own-file in the turn-end sweep (stacked on #2983) fix(diagnostics): all-LSP own-file drift (refs #3011) Sep 14, 2026
@apmantza

Copy link
Copy Markdown
Owner

Conflict resolution and workflow approval complete.

Required workflows are now expected to execute; they are not considered green until node scripts/ci-verdict.mjs 3009 reports every gate successful.

@github-actions github-actions Bot removed the conflict Merge-train warden - mergeStateStatus is DIRTY; required checks are silently skipped until resolved label Sep 14, 2026
Make available all-LSP blocker baselines authoritative by hash even when a
same-size rewrite does not advance mtime, while preserving non-LSP performance
and no-baseline fallback behavior.

Refs apmantza#3011
@apmantza

Copy link
Copy Markdown
Owner

Fix-round commit pushed: 3cb0d4b9c0f76aaf14b6e0beec4bef12ea208a73. The body now records the HIGH F1 same-size rewrite fix, changelog fragment, mutation red, and fresh-CI requirement.

@apmantza

Copy link
Copy Markdown
Owner

Advisory PR-body correction: the Observability section now cites the existing bounded self-drift-unverifiable record at its source location, which satisfies the checker without weakening its runtime-diff validation.

@nzbrian
nzbrian marked this pull request as ready for review September 14, 2026 21:42
@nzbrian

nzbrian commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

We've added the #3011 class-sweep table to the PR body — as a ### freshnessFromMtime / statMtimeMs consumers (#3011) subsection under the existing ## Class sweep section, and the PR is ready for review thank you.

@sonarqubecloud

Copy link
Copy Markdown

@apmantza
apmantza merged commit cca3870 into apmantza:master Sep 15, 2026
43 of 44 checks passed
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.

3 participants