Skip to content

fix(core): date undated okf-local sections from git history, not mtime - #67

Merged
siracusa5 merged 4 commits into
mainfrom
c/agitated-ritchie-cda8bb
Jul 29, 2026
Merged

fix(core): date undated okf-local sections from git history, not mtime#67
siracusa5 merged 4 commits into
mainfrom
c/agitated-ritchie-cda8bb

Conversation

@siracusa5

@siracusa5 siracusa5 commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Problem

An OKF document with no updated: in frontmatter and no {updated=} heading attr resolved to the file mtime through a files layer and to null through an okf-local layer. Same bytes, two different sourceUpdated values, differing only by which adapter read them.

What this does

Rather than copy files.mjs's mtime into okf-local (the first attempt — see below), okf-local now dates undated sections from git history, which is what github.mjs already reports for the same bytes.

The rule, in order: explicit {updated=} attr → frontmatter updated → the file's last author date → the mtime, but only where no history can exist. Shared with files.mjs/github.mjs through withDocumentDate.

Why not mtime

Three independent adversarial reviewers blocked the mtime version, and they were right. Git does not preserve mtimes; a clone or a pull stamps the working tree with the operation time, and files that don't change afterward keep the clone date permanently. Verified: a doc committed 2024-03-01 and cloned today read as 2026-07-29. Through the resolver, a 2019-authored company doc dissented against a 2026-05-12 team decision carrying a date two months newer than the thing it disagreed with.

specs/contextcake-core/design.md:45-57 states the thesis verbatim — the updated-date is the staleness signal, replacing the hash-drift subsystem with "one honest glance." Mtime makes the glance lie, always in the fresher direction. Aggravating: renderCapture emits captured: and never updated:, so every capture concept took that path, and git-sync.mjs:83 requires the live team layer to be okf-local — maximum harm on the flagship flow.

The four ways this still got it wrong before landing

Each was found by review, fixed, and is now covered by a test that fails when the fix is reverted:

Failure Fix
Concurrency The memo published an empty Map before its own await, so every reader arriving during the in-flight git log missed and fell back to mtime. mcp-server.mjs:253 does not await its request handler, so pipelined reads hit this deterministically — only the first caller got a real date. Memoize the promise, not the map
Rebase %cs is the committer date, and git-core runs pull --rebase as its standard divergence recovery, which rewrites committer dates to now — re-dating the whole bundle to today on the flow team-sync depends on. %as (author date) survives a rebase
Shallow clones The boundary commit lists the entire tree as added at the truncation date. service.mjs:539 clones --depth 1, so this was every repo added through the app. Skip boundary commits; leave those sections undated
Unreadable history A git log/ls-files failure inside a repo returned the same shape as "not a repo", so the file looked untracked and got the mtime — the branch violating its own documented invariant. ls-files did it silently. inRepo distinguishes them; warn on both

Plus: a 30s TTL on the memo (a session-lifetime MCP server was serving the history it saw at boot), --relative instead of prefix arithmetic (a layer rooted in a subdirectory missed every lookup and silently fell back to mtime), a guarded statSync, and error.stderr || error.message in git-core so the new warnings don't render as cannot read commit dates ().

Also fixes a pre-existing UTC bug

toISOString().slice(0, 10) converts a local wall-clock event to a UTC calendar date, so a doc edited after 20:00 EDT reported tomorrow. Confirmed: a fixture written at 21:52 local resolved to 2026-07-29. Pre-existing at files.mjs:52 since #65; both adapters now format via localDate(). The tests derive their expected date from date rather than from the adapter's own formula, so they catch this class of bug instead of mirroring it.

Tests

resolver-test.sh gained a commit-dates step covering six shapes: repo root, subdirectory root, untracked doc, concurrent reads, shallow clone, and a git that fails inside a repo (via a PATH shim). Each was verified non-vacuous by reverting its specific fix and watching the matching assertion — and only that assertion — fail. files-source-test.sh keeps the cross-adapter parity check for the plain-folder case, where both adapters legitimately land on the mtime.

  • npm test → exit 0, all 10 suites, also green under TZ=Pacific/Kiritimati (UTC+14) and TZ=Etc/GMT+12 (UTC-12)
  • npm run build --prefix apps/site → exit 0
  • Site demo data unchanged — every doc in apps/playground/demo-layers/ carries an authored updated:

Known costs, not fixed here

  • Performance. git log --name-only -- . walks the whole history regardless of pathspec: measured 6ms → 3675ms on a 6335-commit repo, amortized by the 30s TTL but paid on the first read of each window. Reachable via "add local source" pointed at a large repo. A --max-count bound would cap it at the price of leaving old files undated; a HEAD-keyed disk cache would be better. Worth a follow-up.
  • Merge-only changes and quoted/NFD filenames fall through to undated rather than wrong — the safe direction, but they are gaps.

🤖 Generated with Claude Code

An OKF document with no `updated:` in frontmatter and no `{updated=}` heading
attr resolved to the file mtime through a `files` layer and to null through an
`okf-local` layer — the same bytes, two different `sourceUpdated` values,
depending only on which adapter read them.

Extract the date fallback `files.mjs` already applied into `withDocumentDate`
in okf-local.mjs (explicit section attr > OKF frontmatter `updated` > the
adapter's document date) and call it from both adapters. `github.mjs` inherits
it unchanged via `parseDocument`.

`parseConcept` stays pure: promote.mjs, team-activity.mjs, and the capture
tests read the literal document and must keep seeing exactly what was authored.

Display metadata only — precedence uses concept-level frontmatter and conflict
detection compares content — but it is an adapter disagreement about identical
input, which is the class of bug the shared parseDocument seam exists to prevent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@siracusa5

Copy link
Copy Markdown
Collaborator Author

Adversarial review — three independent reviewers, all three block

Ran three reviewers with different lenses (correctness / test-quality / design), each instructed to refute rather than approve. They converged independently on the same blocking finding.

BLOCKING — the mtime fallback inverts the staleness signal for okf-local

Git does not preserve mtimes. An okf-local layer is defined as "a git repo of OKF markdown" (okf-local.mjs:2, CLAUDE.md), so for this adapter the mtime is essentially never the authorship date — a clone or a pull stamps the working tree with the operation time, and files that don't change afterward report the clone date permanently.

Verified end-to-end on this branch. A doc committed 2024-03-01, cloned today:

mtime       = 2026-07-29   ← what okf-local now reports
last commit = 2024-03-01   ← what github.mjs reports for the same bytes

Through the resolver, a 2019-authored company doc dissenting against a team decision dated 2026-05-12:

"sourceUpdated": "2026-05-12",
"conflicts": [ { "layer": "company", "updated": "2026-07-29", ... } ]

The 2019 doc now reads as two months newer than the team's real decision. specs/contextcake-core/design.md:45-57 states the thesis verbatim — "the updated-date is the staleness signal … this replaces the entire hash-drift / shadow-detection subsystem with one honest glance." This makes that glance lie, in the one comparison the product exists to make. The error is always directional: it claims fresher, never staler.

Aggravating factors found by the reviewers that I had not weighted:

  • renderCapture (capture.mjs:164-171) emits captured:, never updated: — so every capture concept in a live/team layer takes the mtime path. This is not a rare shape.
  • git-sync.mjs:83 mandates that the live team layer be okf-local — the flagship team-sync path is the guaranteed-cloned path, so maximum harm lands where the product is sold.
  • The payload contradicts itself: contributors[].updated is null (from frontmatter.updated, resolver.mjs:67) while that same layer's sourceUpdated is today (resolver.mjs:133). One resolve, one layer, two disagreeing fields. This fixed a cross-adapter disagreement by creating a cross-field one.
  • mcp-server.mjs:633 already had a designed representation for "unknown date" — (updated ?). This makes it unreachable for okf-local and swaps honest ? for a confident wrong number.
  • Shipped pack docs state the opposite contract. specs/contextcake-packs/…/writing-a-layer.md:32-35: "updated is not required by the engine but you should always set it — it is the entire staleness signal." Undated is supposed to mean unknown.

The premise is also weaker than the PR claims

resolver.mjs:133 already computes section.updated ?? c.updated ?? null, where c.updated is frontmatter.updated. So for okf-local the helper's first two rungs are a no-op — the resolver already produced that answer. The entire net effect of this change on okf-local is injecting the mtime. The PR body oversells it as restoring a fallback chain.

And the adapter asymmetry was arguably information, not inconsistency:

kind date source justified
files mtime yes — arbitrary folder, no VCS, mtime is the only signal
github last commit yes — the correct answer, and available
okf-local mtime (this PR) no — git-backed, so mtime is known wrong

Confirmed non-findings

The reviewers tried hard to break the rest and could not:

  • The files.mjs refactor is behavior-preserving. Differential run of old inline logic vs withDocumentDate across a 9x4 matrix of frontmatter values x document dates: identical except documentDate === undefined (old undefined, new null), which no caller reaches — files.mjs:53 always passes a string, github.mjs:169-183 returns string | null. resolver.mjs:133 collapses both anyway.
  • fs.statSync adds no new throw path. readFileSync already threw first in every non-race case (EISDIR, EACCES, dangling symlink). Only a new TOCTOU window, same class as the pre-existing existsSyncreadFileSync one.
  • No merge decision changes. orderContributors ties on frontmatter updated; sectionBeats is level-only; conflict emission filters on content; git-sync.mjs decay uses frontmatter.captured; whats_new uses frontmatter.updated. Blast radius is confined to sourceUpdated and conflicts[].updated.
  • No golden-output breakage. fixtures/mcp-tools-baseline.json is schemas, not output.
  • The new test is not flaky. Both statSync calls read the same stored inode mtime and toISOString() is timezone-independent, so UTC-midnight crossing, coarse granularity, TZ, and CI checkout timing cannot desynchronize them. No fixture pollution either — undated enters step 3's id list but no later assertion depends on count, ordering, or set membership.
  • withDocumentDate is idempotent. withDocumentDate(withDocumentDate(p, "2026-01-01"), "2026-07-28")2026-01-01. No double-application trap.
  • Engine stays dependency-free.

Separate pre-existing bug found in passing

mtime.toISOString().slice(0, 10) converts a local wall-clock event to a UTC calendar date, so a doc edited after 20:00 EDT reports tomorrow's date. Confirmed: a fixture written at 21:52 local resolved to 2026-07-29. This is pre-existing at files.mjs:52; this PR extends it to okf-local. The new test cannot catch it because it computes its expectation with the identical formula. Worth its own fix (use local date components) regardless of what happens to this PR.

Options

  • A — drop the mtime rung from okf-local, and invert the new test to assert the asymmetry is deliberate, with a comment explaining why. Cost ≈ zero; restores honest ? rendering. This is the pre-PR behavior, now documented rather than accidental.
  • B — use the last-commit date for okf-local (matches github.mjs; runGit is already exported from git-core.mjs:35, so no new dependency). The honest version of parity. Costs: mcp-server.mjs sweeps loadConcept over every id at 325/369/429/474/512, so it needs github.mjs's memo pattern (one batched git log --name-only --format=%cs, cached per generation) or a search becomes N subprocess spawns; and okf-local roots aren't required to be git repos, so it still needs a null fallback underneath. Deserves its own commit.
  • C — merge as-is, accepting that undated okf-local docs display checkout time.

Holding the merge pending a call on these.

siracusa5 and others added 3 commits July 28, 2026 22:18
Adversarial review of the previous commit rejected the mtime fallback for
okf-local, and it was right: an okf-local bundle is a git repo, and git does not
preserve mtimes. Clone, checkout and pull all stamp the working tree with the
operation time, so a decision written in 2019 presented as written today — in
the one field the product offers as the staleness signal. The error was always
directional (fresher, never staler), and files that never change after a clone
would have reported the clone date forever.

Read the last-commit date per path instead, which is what github.mjs already
reports for the same bytes. One batched `git log` per generation, memoized and
dropped on sync(), because mcp-server sweeps loadConcept over every id for
search/list and a per-file spawn is not affordable. The walk is bounded to the
layer with `-- .`, and paths are un-prefixed via `rev-parse --show-prefix` —
git reports paths from the repo root, so a layer rooted in a subdirectory would
otherwise miss every lookup and silently fall back to the mtime.

An untracked doc, or a root that is not a repo at all, still uses the mtime:
there no commit date exists and the mtime IS the real edit time, which is the
same signal files.mjs uses for the plain folders it points at.

Also fixes a pre-existing UTC slip: mtimes are local wall-clock events, so
`toISOString().slice(0, 10)` filed an edit made in the evening under tomorrow's
date. Both adapters now format via localDate(). The tests derive their expected
date from `date` rather than from the adapter's own formula, so they can catch
this class of bug instead of mirroring it.

Tests: resolver-test.sh now covers the three shapes that differ — repo root,
subdirectory root, and untracked doc — with a back-dated commit and a touched
mtime, so an mtime regression cannot pass. files-source-test.sh keeps the
cross-adapter parity check for the plain-folder case, where both adapters
legitimately land on the mtime.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…shallow clones

Second adversarial review pass found four ways the commit-date lookup still
reported the mtime — the exact answer it was written to avoid. All four are now
covered by tests that fail if the fix is reverted.

Concurrency: the memo published an empty Map before its own await, so every
reader arriving during the in-flight `git log` found it, missed, and fell back to
the mtime. mcp-server does not await its request handler, so pipelined reads hit
this deterministically — only the first caller got a real date. Memoize the
promise instead of the map.

Rebase: `%cs` is the committer date, and git-core runs `pull --rebase` as its
standard divergence recovery, which rewrites committer dates to now. The live
team layer would re-date its whole bundle to today on the very flow team-sync
depends on. `%as` (author date) survives a rebase.

Shallow clones: the boundary commit lists the entire tree as added at the
truncation date, so every file read as written at clone time. service.mjs clones
`--depth 1` for git-backed sources, so this was every repo added through the app.
Skip boundary commits, and leave those sections undated — a tracked file git
cannot date has no honest answer, and the mtime there is the checkout time.
`git ls-files` distinguishes that from an untracked file, whose mtime IS its
real edit time.

Staleness: the memo had no TTL, so a session-lifetime MCP server served the
history it saw at boot. Now 30s, still one `git log` per search sweep.

Also: warn once when git is unavailable or refuses (safe.directory), instead of
silently degrading to mtimes; a plain non-git root stays silent, since that is a
legitimate okf-local layout. Path prefixing is gone in favour of `--relative`,
which reports paths relative to the layer root directly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Final review round found the branch violating its own documented invariant. When
`git log` or `git ls-files` failed *inside a real repo*, readHistory returned the
same shape as "this is not a repo at all", so documentDate concluded the file was
untracked and fell back to the mtime — dating every section today, which is the
exact lie the branch exists to remove. The ls-files failure did it with no
warning at all.

Distinguish the two: inRepo says whether git answered at all. Inside a repo, a
path with no date is undated, whether that is a truncated history, a change that
only landed in a merge commit, or a history we could not read. Only a genuinely
untracked file, or a root that is no repo, still uses the mtime — there it is the
real edit time.

Also: guard the statSync in documentDate. The window between reading the file and
stat-ing it used to be nothing and is now a git round trip, so a file that
disappears mid-read rejected loadConcept where it previously returned a concept.

git-core: report `error.stderr || error.message`. A maxBuffer or timeout kill
leaves stderr empty, and `??` does not fall through on an empty string, so the
new warnings rendered as "cannot read commit dates ()".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@siracusa5 siracusa5 changed the title fix(core): make okf-local and files agree on undated section dates fix(core): date undated okf-local sections from git history, not mtime Jul 29, 2026
@siracusa5
siracusa5 merged commit 0b8dcff into main Jul 29, 2026
7 checks passed
@siracusa5
siracusa5 deleted the c/agitated-ritchie-cda8bb branch July 29, 2026 03:10
siracusa5 pushed a commit that referenced this pull request Jul 29, 2026
Integrates the github source adapter (#65), git-history section dates (#67)
and remote source health (#68) with background indexing and the utilityProcess
engine. Four conflicts, all union rather than either-or:

- okf-local: keeps main's commit-date history AND the async/bounded walk.
  documentDate's mtime fallback is now an async stat, since it runs in the
  read path the desktop engine must not block on.
- files: keeps the shared parseDocument seam AND the async parseFile. Dates go
  through localDate, so a folder source and a repo source agree on the calendar
  day instead of one of them slicing UTC.
- service: index state and adapter health are orthogonal, so both are reported.
  A source is 'indexing' until read, then 'degraded' if its adapter says its
  last request failed, then 'ok'. Resolution runs over anything with a
  snapshot, so a degraded source keeps contributing — main's warn-and-continue
  intent, expressed through the snapshot model.
- types.ts: GraphSource carries indexing progress and the health timestamps.

Two integration defects the merge exposed, both fixed:

- httpError lost its `detail` argument when the HTTP internals moved to
  http-util.mjs, so a failed remote Sync answered with a bare message instead
  of ok:false and the health timestamps the API promises.
- github-source-test asserted a settled source status straight after a Sync.
  Sync re-indexes, and reads answer from the background index, so the
  assertion raced the re-read. It now passes ?wait=, the same contract the
  service and playground suites use.

Verified on the merged tree: engine 13/13 suites, console typecheck + 125
tests + build, site build, desktop unit (34) + navigation + cli-status +
smoke + isolation (2ms main-loop lag while indexing 2,500 documents).
siracusa5 added a commit that referenced this pull request Jul 29, 2026
Setup froze the Mac app on "Resolving…" until it stopped responding: an
unbounded synchronous directory walk, every file read and tokenized twice per
request, no timeout anywhere, and the engine sharing an event loop with the
window that draws.

- Sources are read by background jobs. /api/graph and /api/resolve-all answer
  from what is indexed right now and report `indexing`/`indexingSources`;
  clients render what they have and poll. Reads are partial by design, so
  completeness assertions pass ?wait=. First graph request against a
  901-document folder: ~6ms.
- The engine runs in its own Electron utilityProcess. Worst-case main-loop
  stall while indexing 2,500 documents: ~694ms sharing the loop -> ~30ms
  isolated, enforced by a new isolation test in CI.
- Indexing limits are manifest-backed settings (Settings -> Indexing), not
  environment variables. Precedence is manifest > env > default.
- New Files view: browse and edit the context files behind each source, as
  rendered Markdown or raw text. The layer file APIs moved into the engine so
  the desktop app can reach them, and now cover markdown-folder layers.
- Markdown renders as typed data through React elements — no HTML string, no
  dangerouslySetInnerHTML, no hand-rolled sanitizer.
- Adding a source only rejects what the user can fix on the form. A too-large
  folder is accepted, then surfaces as a visible source error pointing at
  Settings.
- An unexpected engine exit is fatal, taking the same clean dialog-and-exit
  path as a failed boot.
- The Mac app icon is regenerated from the canonical brand mark via
  `npm run icon`; the stale hand-maintained master is gone.

Integrates the github source adapter (#65), git-history section dates (#67)
and remote source health (#68): index state and adapter health are orthogonal,
so a source reads `indexing` until read, `degraded` if its adapter reports its
last request failed, then `ok`.

Reviewed adversarially in a second session, which found real defects here —
a rejected oversized body that kept buffering, a containment guard whose
realpath result was discarded, and an inline SVG preview path. All fixed,
along with revision-aware saves, cooperative index cancellation, and moving
the API bearer out of renderer argv onto trusted IPC.

Validation: engine 13/13 suites, console typecheck + 125 tests + build, site
build, desktop unit/navigation/cli-status/smoke/boot-failure/isolation.
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