Repository navigation
chore(ingest): extract the shared local ingestion core (walk, cursor, change gate, report) from the qoder connector #111
Description
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: nullUser 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/--limitsemantics — out ofserver/cmd/mem/cmds_ingest.go+qoder_checkpoint.gointo 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-Keyderivation, 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 codesauth/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-runwrites 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 tostate_corruptwithout 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
--watchbehaviour 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.mdstep 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: aCODEOWNERSowner (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/ingestvsserver/cmd/meminternal 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-79andcmds_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。
Activity
PeterGuy326 commentedon Sep 1, 2026
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
server/internal/ingest; the work is a mechanical refactor, not a behavior change.Acceptance evidence
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 commentedon Sep 1, 2026
Implementation: PR #147 (supersedes #129, rebased onto latest
main).Changes:
server/internal/ingestHead SHA:
785de61e00661f4e5115129500be642c47422108waterbro-8 commentedon Sep 2, 2026
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 (forksun-970/mem, head785de61e), 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, sitsDRAFT+CHANGES_REQUESTED. Someone should decide whether #111 moves toin-designon 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.CanonicalRootwithfilepath.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 holdingsessions/p.jsonldo now get distinct cursors — and the collision fix needs onlyfilepath.Abs. TheEvalSymlinksstep is what has a user-visible cost:sha1(abs), so a root spelled relatively or reached through a symlink loses its existing cursors;absalso feedsingestIdempotencyKey(abs, line)(server/cmd/mem/cmds_ingest.go:317-320), so every per-line key changes for that root;internal/memory/service.go:107isON CONFLICT (workspace_id, idempotency_key_sha256) DO NOTHING, the replay lookup at:141-148selects on the same pair, and no migration adds content-, path- orsource.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
~/.qoderis the ordinary case) one upgrade pass duplicates every memory in it. #147'sdocs/integrations/qoder-ingest.mdand 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:
filepath.Absonly). 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.sha1(spelled)cursor as a fallback on miss, and derive the idempotency key from a stable identity that predates the change.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
ingest.Report, butUnchangedandLocalGoneare 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.source.refchanges 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 commentedon Sep 3, 2026
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:
waterbro-8, head6e864e1e) 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."Supersedesline 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, andserver/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 underserver/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:
3c123cdbrefactor(ingest): extract the shared local ingestion corewaterbro-8 <waterbro-8@users.noreply.github.com>6e864e1efix(ingest): give one transcript store one identity勒布朗-詹姆斯 <2986253039@qq.com>Two separate defects in that table, and neither is cosmetic.
6e864e1eis authored by a personal@qq.comaddress, which is the shape today's rule excludes. And3c123cdb's email iswaterbro-8@users.noreply.github.com-- not the ID-prefixed318569545+waterbro-8@users.noreply.github.comform -- 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 unresolvedCHANGES_REQUESTEDfrom @PeterGuy326 (audit at3c123cdb, 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 commentedon Sep 8, 2026
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.Absonly (withoutEvalSymlinks) 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:
server/internal/ingest(mechanical, behaviour-preserving per AC-001)filepath.Absonly (noEvalSymlinks)No scope creep beyond R2's non-goals.
@PeterGuy326 could you confirm the direction on (1) and (2) so this can move?
PeterGuy326 commentedon Sep 10, 2026
Initial triage — labels applied
Per
docs/maintainers/triage.md§"Label taxonomy" ("During initial triage, amemmaintainer 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 atserver/cmd/mem/qoder_checkpoint.goandcmds_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 at55d09b203ba9, 2026-09-10T16:23:43Z)Not started.
grep -rn 'package ingest' server/returns nothing; there is noserver/internal/ingest.server/cmd/mem/qoder_checkpoint.go(89 lines) still ownsqoderCheckpointPath/loadQoderCheckpoint/saveQoderCheckpoint/fileStatelocally, andgit log --followshows its only commit is365ff09b2327(#103) — it has never been touched since.cmds_ingest.gois 345 lines. Both prior implementation attempts, #129 and #147, were closed unmerged.Readiness-gate note (not a
status:readyrecommendation)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:
server/internal/ingestvs an internal file underserver/cmd/mem), and whether cursors key on absolute path or path-plus-device id. The issue assigns these to "the owner at kickoff", andtechnicalOwneris@waterbro-8whileproductOwneris@PeterGuy326.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-triagedeliberately — the gate is not stated bullet-by-bullet as passing, sostatus:readyis not applied here.waterbro-8 commentedon Sep 17, 2026
Not implemented on
main(server/internal/ingestdoes 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 commentedon Sep 17, 2026
Organization-branch successor to #147 is up: #217.
Cherry-picked onto current
main(1b27aac): extractserver/internal/ingest, keep qoder as a thin connector, and include the OS-backed cursor lock so this does not revive the shared.tmprace that #139/#191 describe.#110 can follow this PR. I am not implementing
--watchhere.refactor(ingest): extract the shared local ingestion core
fix(ingest): add OS-backed cursor lock for concurrent writer safety
refactor(ingest): extract the shared local ingestion core
fix(ingest): add OS-backed cursor lock for concurrent writer safety
refactor(ingest): extract the shared local ingestion core (#111) (#217)
waterbro-8 commentedon Sep 20, 2026
平台组分工补派(冯浩然 / waterbro-8)
xiaocui-big,仓库暂不能指派,以钉钉+本评论为准)waterbro-8 commentedon Sep 20, 2026
平台组群成员派活(冯浩然 / waterbro-8)
feat(cli): add one-way mem put --watch (#110) (#225)
waterbro-8 commentedon Sep 23, 2026
关闭验收:PR #217 已合入 main(merge f72e294;final head 2041c3c),server/internal/ingest 已提供共享 walk、cursor/change gate、closed failure codes、report 与 OS-backed lock,Qoder 变为薄连接器。final head 有多份正式 APPROVED,全部 hosted checks 通过;下游 #110 已实际复用该 core 并合入。