Skip to content

fix: every write tool reports durability; resolve_thread stops claiming success (GIT-101) - #40

Merged
2 commits merged into
mainfrom
bugfix/GIT-101-write-result
Sep 22, 2026
Merged

2 commits merged into
mainfrom
bugfix/GIT-101-write-result

Conversation

@nTEG-dev

Copy link
Copy Markdown
Member

What

Every write tool now says whether its write is durable, and resolve_thread stops claiming success it doesn't have (GIT-101).

  • WriteResult { success, durable, stored_in } (src/types/index.ts, helpers in src/services/write-result.ts) is create_thread's contract, now shared by every write tool:
    • durable: the record is in Supabase. Never true on free.
    • stored_in: "supabase", "local" (free), "local_only" (pro, durable write failed, local copy kept) or null (stored nowhere).
    • success = durable || !hasSupabase(), and false whenever nothing was stored.
  • Applied to create_learning, create_decision, create_thread, resolve_thread, archive_learning, record_scar_usage, record_scar_usage_batch, session_close, promote_suggestion (passes create_thread's outcome through) and save_transcript. Their result types extend WriteResult, so the compiler made every return state its outcome.
  • resolve_thread on pro without a durable write returns success: false, durable: false, stored_in: "local_only", and its display says RESOLVED LOCALLY ONLY — not durable … other sessions still see t-… as open.
  • session_close reports on every exit whether the session row reached the store, including a failure that happens after the row landed. A partial close (a thread didn't sync) is durable: true, success: false: the row is saved, the close isn't complete.

Why

resolve-thread.ts computed supabaseSynced, logged it to metrics and then returned success: true regardless. A resolve the durable store never saw looked done, while every other session still saw the thread open.

How to test

One parameterized test (write-result-contract.test.ts): pro tier, Supabase configured, and every request to it fails at fetch. For each of the 10 tools it asserts:

  • the store was actually tried;
  • success: false, durable: false;
  • the right stored_in (local_only for create_thread, resolve_thread and promote_suggestion; null for the rest);
  • the display carries the same verdict (fail|not stored|not durable|locally only). Over MCP, server.ts sends only display when one exists, so the fields alone never reach the agent.
origin/main This branch
Contract test 10 of 12 fail. resolve_thread: success: true with the store unreachable; display "Thread resolved". 12/12

Venue (v1.8.0 setup.sql, vujndhcumgroztfkkdus; production not touched): flow now creates a second thread, resolves it, and reads gitmem_threads back: status: resolved, display "Thread resolved", no "LOCALLY ONLY". The success side lands on a customer-shaped store; resolution_note and resolved_by_session exist in v1.8.0. Exit 0; close persisted; thread counts match; only GIT-73 and GIT-105 seen (the GIT-105 400 is the resolve's triple write, which item 7 addresses).

Unit tests: 1286/1286 pass (+12). No existing test encoded the old behaviour.

Left out on purpose (flagging)

  • absorb_observations, dismiss_suggestion, contribute_feedback: they write session state or local files by design, not the durable store. Observations reach the store with session_close.
  • cleanup_threads: a report with an optional archive. archiveDormantThreads swallows errors and returns 0, so "nothing dormant" and "archive failed" look identical. Giving it a WriteResult means changing that service first. That would be a separate ticket.
  • Free-tier record_scar_usage_batch stores nothing (it defers to the single-record tool) and has always returned success. It now says stored_in: null alongside success: true (no durable store to miss). That's the one place success and stored_in: null coexist.
  • record_scar_usage_batch, partial: if some inserts land and one throws, it still reports notStored. The per-row outcome isn't tracked yet.

Checklist

  • npm run test:unit passes (1286)
  • New tests added (parameterized across all 10 tools)
  • No schema change; verified on a v1.8.0 store
  • Docs: tool reference doesn't document result fields

Breaking changes

resolve_thread (and the others, whose failure paths already said so) now return success: false when the durable write didn't land on pro. That's the fix. New fields durable and stored_in are additive.

🤖 Generated with Claude Code

Claude and others added 2 commits September 21, 2026 20:30
…hread stops claiming success (GIT-101)

resolve_thread computed supabaseSynced, logged it to metrics and returned
success: true regardless, so a resolve the durable store never saw looked
done while other sessions still saw the thread open.

- WriteResult { success, durable, stored_in } (types + services/write-result.ts)
  is create_thread's contract, now shared: success = durable || !hasSupabase(),
  and false whenever nothing was stored.
- Applied to create_learning, create_decision, create_thread, resolve_thread,
  archive_learning, record_scar_usage, record_scar_usage_batch, session_close,
  promote_suggestion (passes create_thread's outcome through) and
  save_transcript. Result types extend WriteResult, so every return states it.
- resolve_thread on pro without a durable write: success false,
  stored_in local_only, and a display that says RESOLVED LOCALLY ONLY.
- session_close reports whether the session row reached the store on every
  exit, including a failure after the row landed.

One parameterized test, Supabase failing at the network, pro tier: each of the
10 tools reports success false, durable false, the right stored_in, and a
display that says so. 10 of 12 fail on origin/main (resolve_thread: success
true). +12 tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eads (GIT-101)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@nTEG-dev nTEG-dev closed this pull request by merging all changes into main in f93c3a3 Sep 22, 2026
neoneye pushed a commit to agent-memory-atlas-archive/gitmem-dev--gitmem that referenced this pull request Sep 22, 2026
Conflicts:
- session-close.ts: gitmem-dev#39's payloadReadError and gitmem-dev#40's sessionRowStored are
  separate declarations; both kept.
- blank-supabase.mjs: gitmem-dev#40's resolve step before gitmem-dev#39's no-payload close (both
  before the real close); both result fields; all three checks.

Semantic (tsc): gitmem-dev#39's "closing-payload.json not found" return predates
WriteResult; it is a pre-write exit, so it gets ...notStored().
neoneye pushed a commit to agent-memory-atlas-archive/gitmem-dev--gitmem that referenced this pull request Sep 22, 2026
Conflicts:
- session-close.ts: gitmem-dev#40 and gitmem-dev#41 add the same sessionRowStored flag; one
  declaration kept (one assignment after the upsert), comment covers both uses.
- blank-supabase.mjs: all result fields and all checks kept.
tsc clean; no new returns to type.
@nTEG-dev
nTEG-dev deleted the bugfix/GIT-101-write-result branch September 23, 2026 05:49
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