Development record
memory ingest input-boundary defects: HTTP status is never checked so 404/500 error pages are silently ingested as source content (internal/core/memory/ingest.go:558-575); tilde expansion mangles ~user paths into home+user concatenations (ingest.go:579-584); a --keep-original failure after the page write reports total failure although pages and registry were durably mutated (ingest.go:301-311); CRLF pages are accepted by parseFrontmatter but rejected by splitFileFrontmatter so hashes and summaries silently degrade (yaml.go:558-591); the URL-ingest success path, content-type handling, PDF extraction, and original-storage are untested, as are YAML block scalars and double-quoted escapes. Detector: an ingest-boundary test suite — fetch status matrix, content-type matrix, CRLF round-trip, tilde cases, partial-failure reporting, parser-parity cases. Acceptance corpus: the six instances above.
memory ingest input-boundary defects: HTTP status is never checked so 404/500 error pages are silently ingested as source content (internal/core/memory/ingest.go:558-575); tilde expansion mangles ~user paths into home+user concatenations (ingest.go:579-584); a --keep-original failure after the page write reports total failure although pages and registry were durably mutated (ingest.go:301-311); CRLF pages are accepted by parseFrontmatter but rejected by splitFileFrontmatter so hashes and summaries silently degrade (yaml.go:558-591); the URL-ingest success path, content-type handling, PDF extraction, and original-storage are untested, as are YAML block scalars and double-quoted escapes. Detector: an ingest-boundary test suite — fetch status matrix, content-type matrix, CRLF round-trip, tilde cases, partial-failure reporting, parser-parity cases. Acceptance corpus: the six instances above.
Progress (2026-07-12, /abcd:run burst 2 — partial, issue stays OPEN): two instances drained behind their detectors:
--keep-originalpartial-failure reporting — FIXED. AstoreOriginalfailure after the durable page+registry write now recordsIngestResult.KeepOriginalErrorand returns the successful ingest (both theingestedandregistry_onlypaths); the CLI renders a warning + non-zero exit. The message is path-safe (strips*os.PathError/*os.LinkErrorpaths; repo-relativesourcesRelPath). Tests:TestIngestKeepOriginalFailureStillReportsIngest,TestKeepOriginalErrorMessageNoPathLeak,TestMemoryIngestKeepOriginalPartialFailure.- CRLF parser-parity — FIXED.
splitFileFrontmatternormalises line endings likeparseFrontmatter/frontmatterKeyLine, so CRLF documents split identically to their LF form. Test:TestSplitFileFrontmatterCRLFParity.
HTTP-status and tilde-expansion instances were addressed earlier (PR #38).
Remainder (kept this issue open until 2026-08-01): the untested ingest surfaces named in the detector — URL-ingest success path, content-type matrix, PDF extraction, and YAML block-scalar / double-quoted-escape parser cases. PDF extraction coverage may require a dependency; assess against the no-new-dep STOP before adopting.
Closed (2026-08-01, v0.5.0 item C7): the remainder is drained and the
acceptance corpus is complete. The four untested surfaces now have detectors in
internal/core/memory/ingest_boundary_test.go and
internal/core/memory/yaml_boundary_test.go, both driven through the existing
injectable seams — Fetcher for the URL path, PDFExtractor for PDF text — so
the no-new-dep STOP never fired: go.mod is unchanged, and the nil-extractor
refusal is itself one of the covered behaviours rather than a reason to adopt a
parser. Covered: the URL-ingest success path (requested vs final URL, headers
reaching licence detection, kept-original extension from the URL path); the
content-type matrix (text/* prefix, the three-entry allowlist and a near-miss
outside it, application/pdf, the non-text rejection, and a lying text/plain
whose bytes still fail the decode); PDF extraction on the fetched path and both
local sniffs (.pdf extension, %PDF- magic) across nil/error/empty/success
extractors; and the parser cases — literal | block scalars (de-indent,
interior and trailing blank lines, opaque content, block as last key) with the
unsupported >/|-/|+ indicators pinned as loud refusals, plus the full
unescapeDoubleQuoted escape set, both error paths, and both call sites.
Original-storage — the fourth instance-5 sub-surface — was already covered by
TestIngestKeepOriginalWritesSourceCanonically and
TestIngestKeepOriginalFailureStillReportsIngest in
internal/core/memory/memory_test.go, and the new URL and PDF cases add the
extension-derivation and original-bytes assertions those did not reach, so the
completeness claim rests on both. Every case was mutation-checked against a
targeted reversion of the behaviour it claims to pin.
Correction (2026-08-01, same day): the closure above originally claimed "no
production behaviour was found to diverge from its documentation, so this
increment is test-only". That was WRONG, and pre-PR review caught it: the new
parser detectors, once written against the real writer/reader boundary rather
than the parser's internals, reproduced TWO defects in
internal/core/memory/yaml.go, both fixed here.
collectBlockScalartook the block's indent from the first non-blank line without requiring it to exceed the indent of thekey: |line that opened the block. A|with no INDENTED body therefore read the next unindented line as content and kept consuming — the rest of the document, other keys included, disappeared into the value with NO error. Fix: an unindented first line ends an EMPTY block and is left for the top-level parse. Detectors:TestParseBlockScalar/empty_block_does_not_swallow_the_next_key(+ its blank-line sibling) andTestParseBlockScalarEmptyBlockKeepsLaterKeys.dumpStringtriggered quoting on\n/\tbut not\r, so a bare carriage return was written raw into the YAML region.parseFrontmatternormalises\rto\nBEFORE splitting, so a value carrying\r---re-read as an early frontmatter terminator and every key below it fell into the page body — a distiller-suppliedrecallstring was enough to makeIngestreport success while writing a page that reads back with nosourceblock and no source hashes, the state the repair path treats as an orphan. Fix:\rjoins the quote trigger (doubleQuotealready escaped it). Detectors: the file-boundary call site added toTestDoubleQuoteEscapeRoundTrip(the pre-existing\rsubjects passed only because they never crosseddumpFrontmatter→joinFileFrontmatter→parseFrontmatter) andTestIngestDistilledControlCharsKeepProvenance, which drives the whole chain throughIngest. Both were watched fail before the fix and pass after.
So the increment is NOT test-only: it carries two production fixes and a CHANGELOG entry under Fixed.
Frontmatter
| id | iss-30 |
|---|---|
| type | issue |
| lifecycle | resolved |
| severity | major |
.abcd/work/issues/resolved/iss-30-memory-ingest-boundary.md
open on GitHub ↗ · commit history ↗
mentions5
- dcn-26071200000000012026-07-12 — clean-slate hardening run STEP 0 triage. A fresh adversarial
- dcn-26071200000000032026-07-12 — iss-30 (memory ingest boundary) partially resolved: the fetch/read
- dcn-26072900000000022026-07-29 — v0.5.0 scoped as "security & consistency" (plan: plans/2026-07-29-v0.5.0-security-and-consistency.md): the security half closes the NEXT.md leak class end-to-end (itd-74/spc-20; the atomic iss-154+157+125+153 detector item; iss-155/156; guard batch iss-159+144+148; boundary fixes iss-30/34), the consistency half retires the record-currency majors (iss-37..44, iss-80) — maintainer added the latter rather than deferring them. Version is derived by launch ship, not declared; 0.5.0 is the prediction given itd-74 additive. Deferred majors listed explicitly in the plan (largest: iss-124); the 2026-07-24 queue's pick-up role is superseded.
- dcn-26080100000000142026-08-01 — iss-30 (v0.5.0 item C7), FINAL increment, test-only: the memory-ingest boundary's last four acceptance instances were absences, not defects, and are now drained behind detectors in `internal/core/memory/ingest_boundary_test.go` and `internal/core/memory/yaml_boundary_test.go`. The no-new-dep STOP the entry flagged for PDF extraction never fired, and the reason is the design rather than luck: `PDFExtractor` is a plain `func([]byte) (string, error)` the caller supplies, so a fake extractor covers the nil / error / empty-text / success matrix on the fetched path and both local sniffs (`.pdf` extension, `%PDF-` magic) without a parser ever entering `go.mod` — the nil case (no parser installed) is itself one of the behaviours under test. Likewise `Fetcher` carries the whole URL-ingest success path with no network: the fetcher is asked for the REQUESTED url, the FINAL url after redirects is what reaches the citation and the registry origin, the response headers reach licence detection (an HTTP `License:` header has no local-path equivalent), and `--keep-original` takes its extension from the final url's PATH, so a query string or fragment cannot leak into it. Two judgement calls worth recording. (1) On the parser's block-scalar limit: only the literal `|` opens a block — `>`, `|-` and `|+` are read as ordinary scalars — and rather than treat that as a defect to fix, the increment pins the property that makes it safe, which is that an unsupported indicator fails LOUDLY (the indented lines beneath it are refused as top-level content) rather than silently swallowing or dropping them. Widening the parser was rejected as scope the memory store has no shape asking for; the dumper never emits a block scalar at all. (2) No CHANGELOG entry: nothing user-facing changed, and the repo's own definition of done ties an entry to a user-facing change — the precedent is the behaviour-preserving `internal/urlguard` extraction, which carried none either. Every case was mutation-checked (sixteen targeted reversions of shipped behaviour, each turning at least one new case red), and no production behaviour was found to diverge from its own documentation, which is why this increment adds no fix.
- dcn-26080100000000152026-08-01 — iss-30 (v0.5.0 item C7), CORRECTION to the entry immediately above, and two production fixes: that entry closed the item as "test-only" and stated that "no production behaviour was found to diverge from its own documentation, which is why this increment adds no fix". That claim was false and is retracted here. Two independent fresh pre-PR reviews (correctness and security) each reproduced a real defect in `internal/core/memory/yaml.go`, both now fixed detector-first on the same branch. (1) `collectBlockScalar` set the block's indent from the first non-blank line WITHOUT requiring it to exceed the indent of the `key: |` line that opened the block. Since the function is only ever reached for a top-level key (`parseYAMLLines` refuses any other indent), a `|` with no indented body took the next COLUMN-0 line as content and went on consuming every line after it: the remaining keys — `domain`, `slug`, `source` — were absorbed into the value and no error was raised. An unindented first line now ends an EMPTY block and is left unconsumed for the top-level parse. (2) `dumpString` triggered its quoting on `\n` and `\t` but not `\r`, so a bare carriage return was emitted RAW into the YAML region, while `parseFrontmatter` normalises `\r` to `\n` before splitting lines — the writer and the reader disagreed about what a line is. A value carrying `\r---` therefore re-read as an early frontmatter terminator and every key below it fell into the page body; a distiller-supplied `recall` string was enough to make `Ingest` report success while writing a page that reads back with an EMPTY source block and no source hashes, which is exactly the state the reconcile/repair path treats as an orphan. `\r` now joins the quote trigger; `doubleQuote` already escaped it correctly, so only the trigger was missing. The lesson worth keeping is about detector siting rather than either bug: the earlier increment's `\r` cases DID pass, because they round-tripped through `parseScalar`/`parseFlowMap` directly and never crossed the `dumpFrontmatter` → `joinFileFrontmatter` → `parseFrontmatter` boundary where the invariant is actually stated (yaml.go:11-16) and where the defect lived. A round-trip test that does not use the real entry points tests the parser's internals, not the invariant; the new cases go through the file boundary and, for the CR case, through `Ingest` itself. Both detectors were watched fail before the fix and pass after. Consequently the increment is no longer test-only and DOES carry a CHANGELOG entry under Fixed — the second judgement call in the entry above ("no CHANGELOG entry: nothing user-facing changed") lapses with the claim it rested on.