chore(ingest): extract the shared local ingestion core (walk, cursor, change gate, report) from the qoder connector

[waterbro-8]

schemaVersion: requirement-record.v1
revision: R1
status: needs-design
priority: P2
productOwner: "@PeterGuy326"
technicalOwner: "@waterbro-8"
userOutcome: "A mem maintainer changes local ingestion behaviour once, in one core, instead of twice: the qoder transcript connector and the future put --watch daemon share walk, cursor, change-detection and report code, so #110 does not write a second state layer."
requirements:
  - REQ-001
  - REQ-002
  - REQ-003
acceptanceCriteria:
  - AC-001
  - AC-002
  - AC-003
parent: null
dependencies:
  - "https://github.com/bytefolk/mem/pull/108 (must merge first)"
supersedes: []
lastDecisionAt: null

User problem and observable outcome

Adjudicated on #110 (R2, D-4): #110 and #103 share an ingestion core, not a write surface. The core already exists in working form inside #103's connector, and everything in it was earned through review on PR #108 — a cursor that must not advance under --dry-run, truncate/rewrite detection, atomic cursor writes, per-file degradation on a 409 instead of aborting the run, and a run summary.

Concrete duplication cost if this is not done: put --watch would re-implement the same cursor store, the same state-root layout (cliStateRoot(), server/cmd/mem/cmds_ingest.go:104-112), the same failure-classification table and the same report vocabulary, in a second file, with a second set of edge-case tests, and the two copies would drift. Observable outcome: one internal package owns local ingestion mechanics; both call sites are thin.

Requirements

• REQ-001: Extract the local ingestion core — deterministic recursive walk, per-path cursor store (atomic rename write, truncate/rewrite reset), stat-gate-then-hash change decision, cycle report aggregation, closed failure-code classification, and the --dry-run / --limit semantics — out of server/cmd/mem/cmds_ingest.go + qoder_checkpoint.go into a reusable internal package, preserving behaviour exactly.
• REQ-002: Point the qoder connector at the core with no observable behaviour change: same memories payload shape, same Idempotency-Key derivation, same stdout summary, same cursor file format and location (existing users' cursors must remain readable).
• REQ-003: Make the report and failure vocabulary explicit and shared, so feat(sync): put --watch one-way directory watch (minimal tier, carved from Phase 2 sync drive) #110 consumes names instead of inventing them: counts scanned / ingested / deduped / unchanged / changed / local_gone / failed, and failure codes auth / plan_quota / provider_timeout / network / read_denied / upload_rejected / root_missing / state_corrupt.

Acceptance criteria

• AC-001: Behaviour-preserving — the qoder test suite from PR feat(ingest): add mem ingest qoder for AI-agent conversation transcripts (#103) #108 passes with import-path changes only, no assertion edits; a pre/post comparison on one fixture transcript tree yields identical memories bodies and identical cursor bytes.
• AC-002: Core-level fixtures pin the edge cases that review found: --dry-run writes neither a cursor nor a request; a rewritten-shrunk file resets its cursor; a 409 degrades one file and continues the rest; a corrupt cursor degrades to state_corrupt without blocking the run.
• AC-003: The core has no cobra/stdout dependency (it returns a report struct, the command prints it), carries no new third-party dependency, and documents its contract in the package doc comment for feat(sync): put --watch one-way directory watch (minimal tier, carved from Phase 2 sync drive) #110 to consume.

Non-goals and forbidden shortcuts

• No --watch behaviour here; no fsnotify or any new dependency; no server-side ledger; no change to the memory payload or the file upload path.
• Do not implement this before PR feat(ingest): add mem ingest qoder for AI-agent conversation transcripts (#103) #108 merges — refactoring an in-flight branch would put two issues' diffs into one PR, which AGENTS.md step 4 forbids.
• Do not "improve" qoder parsing semantics while extracting; any behaviour delta belongs in its own decision on feat: ingest AI-agent conversation transcripts (Qoder/CLI sessions) as a first-class input source #103.

Lifecycle, status, priority, blockers, and open decisions

• status=needs-design; priority P2; technicalOwner @waterbro-8; independent reviewer: a CODEOWNERS owner (not the author).
• Blocker: merge of PR feat(ingest): add mem ingest qoder for AI-agent conversation transcripts (#103) #108 (open decision on feat: ingest AI-agent conversation transcripts (Qoder/CLI sessions) as a first-class input source #103's record: whether the source-flag vocabulary generalises beyond qoder — resolved here as "cursor/report shape is shared, source naming stays per-connector").
• Open decisions for the owner at kickoff: package location (server/internal/ingest vs server/cmd/mem internal file) — must stay CLI-side, since both sinks are CLI-driven and the server has no knowledge of local paths; and whether cursors key on absolute path or on a path-plus-device id (symlinked roots).
• Sequencing: this lands before feat(sync): put --watch one-way directory watch (minimal tier, carved from Phase 2 sync drive) #110's implementation PR; feat(sync): put --watch one-way directory watch (minimal tier, carved from Phase 2 sync drive) #110 records it as a dependency.

Evidence plan

• E2 now: source-level — the duplication is locatable at server/cmd/mem/qoder_checkpoint.go:37-79 and cmds_ingest.go:115-262, and feat(sync): put --watch one-way directory watch (minimal tier, carved from Phase 2 sync drive) #110's D-4 ruling on this issue names it.
• E3 for delivery: identical cursor bytes and memories bodies before/after extraction on a fixture tree, plus the untouched PR feat(ingest): add mem ingest qoder for AI-agent conversation transcripts (#103) #108 assertions passing.

Decisions

• None yet; created from the feat(sync): put --watch one-way directory watch (minimal tier, carved from Phase 2 sync drive) #110 R2 adjudication (feat(sync): put --watch one-way directory watch (minimal tier, carved from Phase 2 sync drive) #110 R2 decision comment, D-4) on 2026-08-30.

Revision history

• R1 (2026-08-30): issue created as the predecessor of feat(sync): put --watch one-way directory watch (minimal tier, carved from Phase 2 sync drive) #110 by the technical-owner adjudication on feat(sync): put --watch one-way directory watch (minimal tier, carved from Phase 2 sync drive) #110 D-4.

---

需要 maintainer 打标(本账号无 label 写权限):建议 type:maintenance、area:cli、priority:p2、evidence:e2-source、status:needs-triage。

[PeterGuy326]

schemaVersion: requirement-decision.v1
canonicalIssue: "https://github.com/fullstack-ai-infra/mem/issues/111"
previousRevision: R1
resultingRevision: R2
resultingStatus: ready
decisionType: narrowing-and-dependency-gate
approver: "@PeterGuy326"
decidedAt: "2026-09-01T02:54:08Z"
dependency: "https://github.com/fullstack-ai-infra/mem/pull/140 accepted and merged"

R2 owner decision — strict mechanical ingest extraction

This is the effective product-owner decision for R2. The contract is ready, but implementation is prohibited until #140 has been independently accepted and merged. It is not a PR approval or merge authorization.

Approved scope

• Extract only the already observable qoder-ingestion mechanics into server/internal/ingest; the work is a mechanical refactor, not a behavior change.
• Preserve the existing request bodies, cursor bytes and locations, report/output behavior, dry-run handling, shrunk-or-rewritten-file handling, per-file 409 degradation, and corrupt-cursor behavior.
• Carry forward the independent concurrent-checkpoint max-progress safeguard from fix(ingest): serialize qoder checkpoint writers #140.

Acceptance evidence

• On the same deterministic fixture, a byte-level pre/post comparison proves identical request payloads, cursor artifacts, and command output.
• Focused tests prove dry-run makes neither request nor cursor write; a shrunk/rewritten file resumes under the existing semantics; a 409 degrades one file without stopping the run; and a corrupt cursor degrades under the existing failure semantics.
• The package has no Cobra or stdout dependency and introduces no third-party dependency.

Non-goals

No new content-hash or change gate, path rekeying or replay behavior, change to payloads, failure codes, or report vocabulary, watch daemon, fsnotify, or semantic parser improvement is authorized. Any such behavior change requires a separate owner decision.

After #140 is accepted and merged, implementation may be planned against R2 with normal independent review, required CI, and merge evidence still required.

[sun-970]

Implementation: PR #147 (supersedes #129, rebased onto latest main).

Changes:

• Extracted walk, cursor, change gate, report, and failure classification to server/internal/ingest
• Added OS-backed cursor lock (flock/fcntl/LockFileEx) for concurrent-writer safety per R2 decision
• All existing tests pass; new tests pin edge cases

Head SHA: 785de61e00661f4e5115129500be642c47422108

[waterbro-8]

Design decision requested — I am the technicalOwner here, and I am not taking it unilaterally. Disclosure, because it matters for how to read this: two of #147's three commits (e9e17f8b, ab5bc035) are authored by this account, and #129 (the draft this one supersedes) is ours too. So the ruling below is @PeterGuy326's as productOwner, not ours to make.

Status vs reality

The requirement record is still status: needs-design, and it says "Do not implement this before PR #108 merges" — #108 merged 2026-08-30T06:14:00Z, so that blocker is genuinely cleared. But an implementation is open now as #147 (fork sun-970/mem, head 785de61e), carrying our two extraction commits plus a third that lands #139's lock inside the same PR. #129, our own draft of the same work, sits DRAFT + CHANGES_REQUESTED. Someone should decide whether #111 moves to in-design on the strength of #147 or whether the sequencing gets reset; either is defensible, "neither" is the current state.

The open decision got exercised without a ruling

"Open decisions for the owner at kickoff" lists: whether cursors key on absolute path or on a path-plus-device id (symlinked roots). #147 answers it by adding ingest.CanonicalRoot with filepath.EvalSymlinks (server/internal/ingest/ingest.go:196-205) and routing the root through it before anything is keyed. That resolves the collision bug it cites — two cwd's each holding sessions/p.jsonl do now get distinct cursors — and the collision fix needs only filepath.Abs. The EvalSymlinks step is what has a user-visible cost:

• the cursor filename is sha1(abs), so a root spelled relatively or reached through a symlink loses its existing cursors;
• abs also feeds ingestIdempotencyKey(abs, line) (server/cmd/mem/cmds_ingest.go:317-320), so every per-line key changes for that root;
• the server replays on idempotency key alone — internal/memory/service.go:107 is ON CONFLICT (workspace_id, idempotency_key_sha256) DO NOTHING, the replay lookup at :141-148 selects on the same pair, and no migration adds content-, path- or source.ref-based uniqueness. A new key is therefore a new row: Replayed: false.

So the re-post is not a replay. For a store behind a symlink (a dotfile manager symlinking ~/.qoder is the ordinary case) one upgrade pass duplicates every memory in it. #147's docs/integrations/qoder-ingest.md and CHANGELOG both describe this as "re-posted once under new keys" / "replays those files once", which understates it, and REQ-002's own words are "existing users' cursors must remain readable" — readable, yes; equivalent, no. AC-001 asks for more than that: "a pre/post comparison on one fixture transcript tree yields identical memories bodies and identical cursor bytes", which cannot hold for this class of root. #147's requirements trace restates AC-001 without that clause.

Three ways out, in rough order of how little they cost users:

1. Key on the absolute-but-unresolved path (filepath.Abs only). Collision bug fixed, no re-key, no duplicates. Cost: the same store reached via two spellings keeps two cursors — which is a re-ingest saved by idempotency, not a correctness loss.
2. Keep canonical keys but make it a decision with a migration: read the legacy sha1(spelled) cursor as a fallback on miss, and derive the idempotency key from a stable identity that predates the change.
3. Accept the re-key, and say in the CHANGELOG that upgrading duplicates that store's memories, so the operator can clear or dedupe deliberately.

Option 3 is the only one that is strictly a documentation fix, and it is the one I'd not pick: it makes the tool write duplicates as a side effect of a refactor PR.

Two smaller things from the same record

• REQ-003's vocabulary is present in ingest.Report, but Unchanged and LocalGone are reserved-at-zero with no test giving them meaning (ingest.go:151, :153). feat(sync): put --watch one-way directory watch (minimal tier, carved from Phase 2 sync drive) #110 is the consumer that needs those names, so "consumes names instead of inventing them" is only half-exercised today.
• Non-goals say "no change to the memory payload". source.ref changes value for symlinked roots under refactor(ingest): extract shared local ingestion core with concurrent-writer safety #147 (the field's shape does not). Depends how you read "payload" — flagging it rather than deciding it.

No PR opened, nothing merged or closed, no workflow approval, no review submitted on any of #129/#139/#140/#147.

[waterbro-8]

Coverage read-back: this work is orphaned by a handoff that no longer exists

Three things happened to the shared ingestion core in four days, and the end state is that no vehicle exists for it:

1. refactor(ingest): extract the shared local ingestion core #129 (ours, opened by waterbro-8, head 6e864e1e) was closed unmerged on 2026-09-02T05:36Z with the reason recorded in-thread: "Closing this as superseded. It is our PR and our draft, and refactor(ingest): extract shared local ingestion core with concurrent-writer safety #147 states 'Supersedes refactor(ingest): extract the shared local ingestion core #129' in its own body."
2. refactor(ingest): extract shared local ingestion core with concurrent-writer safety #147 was a fork pull request. It was closed on 2026-09-03 under the decision that fork workflow permissions stay off and this work is redone in-repo, and no code was imported.
3. So the handoff in step 1 pointed at a PR that no longer exists, and the fallback in step 2 has not started. refactor(ingest): extract the shared local ingestion core #129's Supersedes line and refactor(ingest): extract shared local ingestion core with concurrent-writer safety #147's close comment are the only two records of the transfer, which is why this comment exists.

Measured on main@1332bf46, the extraction never landed: server/internal/ingest/ does not exist, and server/cmd/mem/qoder_checkpoint.go -- the file #147 proposed deleting -- is still there. #147's own file list showed the new package as seven added files under server/internal/ingest/.

Reopening #129 is not the clean fix it looks like

The attribution problem does not resolve by falling back to our own PR, because #129's head is itself mixed:

commit	author	committer
3c123cdb refactor(ingest): extract the shared local ingestion core	waterbro-8 <waterbro-8@users.noreply.github.com>	same
6e864e1e fix(ingest): give one transcript store one identity	勒布朗-詹姆斯 <2986253039@qq.com>	same

Two separate defects in that table, and neither is cosmetic. 6e864e1e is authored by a personal @qq.com address, which is the shape today's rule excludes. And 3c123cdb's email is waterbro-8@users.noreply.github.com -- not the ID-prefixed 318569545+waterbro-8@users.noreply.github.com form -- so GitHub does not map it to the account; it renders as an unlinked author string, which is what #138's "clean candidate supersedes #132" reasoning was about. #129 also still carries an unresolved CHANGES_REQUESTED from @PeterGuy326 (audit at 3c123cdb, response posted 08-31).

Re-authoring those commits under one identity would be the other horn of the same problem: it silently rewrites who wrote what. That is a call for the owners of this repository, not something to do quietly in a rework.

What I did not do

Did not reopen #129, did not cherry-pick from #147, did not rewrite any commit authorship, did not start the in-repo branch, and changed no label, status, assignment, or body here. The decision recorded today says redo in-repo; #111 is the one item where "redo" needs a prior ruling on whose work the two commits are, because the code exists and the attribution is contested.

Related: #110 was covered by the same #147 and by #150, and has its own read-back there.

[sun-970]

Path forward for #111

The extraction work currently has no vehicle: #129 is closed with attribution concerns, and #147 (my fork PR) was closed under the in-repo workflow decision. Summarising what I believe is needed to unblock:

1. Attribution ruling (owner call)

#129's two commits have authorship issues documented by @waterbro-8. I defer to the repo owners on how to resolve this — options are: (a) cherry-pick the code changes onto new clean commits, (b) re-author under corrected identities with explicit acknowledgment, or (c) redo from scratch. I am willing to do (c) if that is the cleanest path.

2. Symlink cursor key: option 1

Per @waterbro-8's analysis, filepath.Abs only (without EvalSymlinks) fixes the collision bug without re-keying existing cursors or duplicating memories. I will implement option 1 unless the owners direct otherwise.

3. Proposed next step

With owner confirmation on the above two points, I will open a fresh in-repo PR that:

• Extracts walk, cursor, change gate, report, and failure classification into server/internal/ingest (mechanical, behaviour-preserving per AC-001)
• Keys cursors on filepath.Abs only (no EvalSymlinks)
• Carries forward the concurrent-checkpoint lock from fix(ingest): serialize qoder checkpoint writers #140/[Bug] Prevent concurrent Qoder checkpoint regression and staging collisions #139
• Passes the full pre/post byte-comparison on a fixture tree
• Introduces no new dependency

No scope creep beyond R2's non-goals.

@PeterGuy326 could you confirm the direction on (1) and (2) so this can move?

[PeterGuy326]

Initial triage — labels applied

Per docs/maintainers/triage.md §"Label taxonomy" ("During initial triage, a mem maintainer applies the type, status, area, evidence, and—when applicable—severity labels"), this issue carried no labels at all, so it was invisible to every label-based view. Applied the set the author requested at the foot of the body, which matches the taxonomy:

type:maintenance (refactoring, no user-visible behaviour change) · area:cli (both sinks are CLI-driven; the issue itself rules out a server-side home) · evidence:e2-source (the duplication is locatable at server/cmd/mem/qoder_checkpoint.go and cmds_ingest.go; nothing was executed) · status:needs-triage.

Not applied: severity: — maintenance issues do not normally carry one. Not raised: evidence: stays at E2; E3 requires independent reproduction, which is a delivery-time check here (identical cursor bytes and memories bodies before/after).

The named blocker is cleared

The record says: "Blocker: merge of PR #108". PR #108 is MERGED — 365ff09b2327, 2026-08-30T06:14:00Z, feat(ingest): add mem ingest qoder for AI-agent conversation transcripts (#103). So the precondition for starting this work is satisfied and the issue is no longer blocked.

Current state on main (verified at 55d09b203ba9, 2026-09-10T16:23:43Z)

Not started. grep -rn 'package ingest' server/ returns nothing; there is no server/internal/ingest. server/cmd/mem/qoder_checkpoint.go (89 lines) still owns qoderCheckpointPath / loadQoderCheckpoint / saveQoderCheckpoint / fileState locally, and git log --follow shows its only commit is 365ff09b2327 (#103) — it has never been touched since. cmds_ingest.go is 345 lines. Both prior implementation attempts, #129 and #147, were closed unmerged.

Readiness-gate note (not a status:ready recommendation)

Bullets 1–6 and 8–9 pass on the record as written (not a duplicate; no sensitive detail; labels now accurate; clear outcome; AC-001/002/003 are objective and testable; dependencies and risks named; validation approach is the byte-identical before/after comparison). Two bullets do not yet pass:

• Bullet 7 (scope and explicit non-goals recorded) — non-goals are recorded, but the scope is split across two still-open owner decisions: package location (server/internal/ingest vs an internal file under server/cmd/mem), and whether cursors key on absolute path or path-plus-device id. The issue assigns these to "the owner at kickoff", and technicalOwner is @waterbro-8 while productOwner is @PeterGuy326.
• Bullet 10 (small enough for one reviewable PR) — plausible but unproven; refactor(ingest): extract shared local ingestion core with concurrent-writer safety #147 attempted it and did not land.

There is also a sequencing contradiction to resolve before promotion: this record says "Sequencing: this lands before #110's implementation PR", while #110's own record says "Blockers: none". One of the two should be corrected.

Left at status:needs-triage deliberately — the gate is not stated bullet-by-bullet as passing, so status:ready is not applied here.

[waterbro-8]

Not implemented on main (server/internal/ingest does not exist). Previous candidates #129 and #147 were closed under the 2026-09-03 fork-workflow decision.

Blocker PR #108 has merged, so extraction can start, but #147 was explicitly warned as unsafe to reuse blindly (cross-process cursor vs #139). Sequencing: land #191 first, then extract the core onto an organization branch, then #110.

I am not opening a watch or ingest-core PR ahead of that order.

[waterbro-8]

Organization-branch successor to #147 is up: #217.

Cherry-picked onto current main (1b27aac): extract server/internal/ingest, keep qoder as a thin connector, and include the OS-backed cursor lock so this does not revive the shared .tmp race that #139/#191 describe.

#110 can follow this PR. I am not implementing --watch here.