From 87e90a48df73ad2fe1531f714c33f6ce5997f8e6 Mon Sep 17 00:00:00 2001 From: Seoyun Lee Date: Wed, 29 Jul 2026 11:44:16 +0900 Subject: [PATCH 1/4] =?UTF-8?q?refactor:=20=EB=B3=80=ED=99=98=EB=B3=B8?= =?UTF-8?q?=EC=9D=98=20=EC=9B=90=EB=B3=B8=20=EC=B6=9C=EC=B2=98=EB=A5=BC=20?= =?UTF-8?q?sources=20=EC=83=81=EB=8C=80=20=EA=B2=BD=EB=A1=9C=EB=A1=9C=20?= =?UTF-8?q?=EB=B3=B4=EA=B4=80=ED=95=9C=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `conv_origin` 이 provenance 헤더의 값을 basename 으로 축약해 저장하던 것을, 변환본 자신의 미러 서브디렉터리 + 헤더의 basename 으로 재구성한 sources 상대 경로로 바꾼다. ingest 는 미러 서브디렉터리와 헤더 라벨을 같은 `src_rel` 에서 파생하므로 헤더의 디렉터리 성분은 새 정보가 아니고, 손편집으로 둘이 갈릴 때 믿어야 하는 쪽은 변환본이 실제로 미러링한 자기 위치다. basename 만 필요한 소비자는 `origin_name()` 을 거치게 하고, 고아 판정은 저장값이 이미 서브디렉터리를 품고 있으므로 `sources/` 존재 확인으로 줄인다. 헤더에 파일명 성분이 없는 값(`source: /`)은 빈 origin 센티널을 유지해, 서브디렉터리 이름을 원본 이름으로 착각하지 않게 한다. 동작 변화는 없다(다음 커밋의 경로 분기 수정을 위한 준비). 업스트림 c6d359d 와 동일 입력에서 `eject --dry-run` 선택 결과 27건, `eject --orphans --dry-run` 6시나리오 출력이 모두 바이트 단위로 동일함을 실측했다. 동봉한 핀 3건은 회귀 방지용이며 수정 전에도 통과한다: 경로형 헤더 + 중첩 변환본, 경로형 헤더 + flat 변환본, 파일명 없는 헤더. 업스트림 #324 --- factlog/cli.py | 54 ++++++++++++++++++++++++++++------------- tests/test_eject_cmd.sh | 53 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 17 deletions(-) diff --git a/factlog/cli.py b/factlog/cli.py index 35d3cc83..d634dc61 100644 --- a/factlog/cli.py +++ b/factlog/cli.py @@ -2317,6 +2317,11 @@ def _select_eject_sources(args, rows, disk_refs, all_refs, target, nfc): # so a stem guess would let `eject report.docx` wrongly pull report.pptx's # conversion; the recorded origin name disambiguates. Falls back to a stem # match only when no header is present (a hand-made conversion). + # + # The stored value is the original's path *relative to sources/* — the + # conversion's own mirrored subdir joined with the basename read from the + # header (see the loop below). Consumers that only want the basename go + # through origin_name(). conv_origin: dict[str, str] = {} for ref, p in disk_refs.items(): if not ref.startswith("runs/sources/"): @@ -2336,12 +2341,30 @@ def _select_eject_sources(args, rows, disk_refs, all_refs, target, nfc): origin = nfc(m.group(1).strip()) if origin: # #214: the header may now record a sources/-relative path - # (sub_a/data.hwpx) rather than a bare basename. Reduce it to the - # basename so the pairing/orphan reconstruction below — which - # rebuilds sources// from the conversion's own - # mirrored subdir — stays correct for both header formats and no - # legacy basename header regresses. - conv_origin[ref] = PurePosixPath(origin).name + # (sub_a/data.hwpx) rather than a bare basename. Rebuild the + # original's sources-relative path from the conversion's *own* + # mirrored subdir plus the header's basename, so both header + # formats yield the same value and no legacy basename header + # regresses: ingest derives the mirrored subdir and the header + # label from one src_rel, so the header's directory component is + # never new information (and is the wrong signal to trust when a + # hand-edit makes the two disagree — the file's own location is + # what the conversion actually mirrors). + # Only PurePosixPath joining is used: it folds "//" and "./" but + # keeps ".." verbatim, so a stored origin still spells the header + # the same way a consumer would. + subdir = PurePosixPath(ref[len("runs/sources/"):]).parent + base = PurePosixPath(origin).name + # A header with no filename component ("source: /") is as + # unusable as an empty one; keep the empty-origin sentinel rather + # than inventing an original named after the subdir. + conv_origin[ref] = (subdir / base).as_posix() if base else "" + + def origin_name(ref: str) -> str | None: + """Basename of the original a conversion was made from — None when the + conversion carries no provenance header at all.""" + origin = conv_origin.get(ref) + return PurePosixPath(origin).name if origin is not None else None def matches(ref: str, name: str) -> bool: name = nfc(name) @@ -2353,11 +2376,11 @@ def matches(ref: str, name: str) -> bool: # A path was given: the exact original is handled above; for a # binary original also match the conversion it produced (by # recorded origin). Same-basename files elsewhere are NOT matched. - return is_conv and conv_origin.get(ref) == np_.name + return is_conv and origin_name(ref) == np_.name if np_.suffix: # a bare filename with an extension if not is_conv: return rp.name == np_.name # an original with that filename - origin = conv_origin.get(ref) # the conversion made from this original + origin = origin_name(ref) # the conversion made from this original # Provenance is the reliable signal. A headerless conversion falls # back to its own name minus the ingest out-suffix: since ingest now # keeps the original's extension (report.pptx -> report.pptx.md), the @@ -2367,7 +2390,7 @@ def matches(ref: str, name: str) -> bool: # one (matched via its recorded origin so the source's own extension in # the new naming — report.pptx.md — does not defeat the stem compare). if is_conv: - origin = conv_origin.get(ref) + origin = origin_name(ref) return Path(origin if origin else rp.name).stem == np_.stem return rp.stem == np_.stem @@ -2393,21 +2416,18 @@ def matches(ref: str, name: str) -> bool: # a subtree to mirror, so the subdir is unknown) has only the # provenance basename as an origin signal; match by basename and keep # erring toward retention. - from pathlib import PurePosixPath - src_basenames = {Path(r).name for r in disk_refs if not r.startswith("runs/sources/")} for ref in all_refs: if ref.startswith("runs/sources/"): if ref in disk_refs: origin = conv_origin.get(ref) # origin is not None == has a factlog provenance header - # (hand-placed conversions are kept). + # (hand-placed conversions are kept). It already carries the + # conversion's mirrored subdir, so a "/" in it *is* the + # mirrored case and sources/ is the exact original. if origin is not None: - conv_rel = ref[len("runs/sources/"):] - subdir = PurePosixPath(conv_rel).parent - if subdir.as_posix() != ".": - expected = (PurePosixPath("sources") / subdir / origin).as_posix() - paired = expected in disk_refs + if "/" in origin: + paired = f"sources/{origin}" in disk_refs else: paired = origin in src_basenames if not paired: diff --git a/tests/test_eject_cmd.sh b/tests/test_eject_cmd.sh index a708db5f..b1b43272 100644 --- a/tests/test_eject_cmd.sh +++ b/tests/test_eject_cmd.sh @@ -317,6 +317,59 @@ printf '%s\n%s\n%s\n' "$H" \ [ ! -f "$KB/runs/sources/a/report.md" ] && ok "orphaned subdir conversion (deleted original) is ejected" || bad "same-basename collision masked the orphan" [ -f "$KB/runs/sources/b/report.md" ] && ok "surviving subdir sibling's conversion is kept" || bad "surviving sibling wrongly orphaned" +# --- a #214 path-form header pairs with its mirrored original (no orphan) ----- +# ingest records `source: sub/report.html` for sources/sub/report.html, and the +# conversion sits in the matching mirrored subdir. Both same-name originals are +# present, so neither conversion may be scanned as an orphan. +KB="$(mktemp -d)/wiki" +"$PYTHON" -m factlog init --target "$KB" >/dev/null +mkdir -p "$KB/sources/sub" "$KB/runs/sources/sub" +printf 'top\n' > "$KB/sources/report.html" +printf 'nested\n' > "$KB/sources/sub/report.html" +printf '\nx\n' \ + > "$KB/runs/sources/report.html.md" +printf '\nx\n' \ + > "$KB/runs/sources/sub/report.html.md" +printf '%s\n%s\n%s\n' "$H" \ + 'A,rel,B,runs/sources/report.html.md,confirmed,0.9,' \ + 'C,rel,D,runs/sources/sub/report.html.md,confirmed,0.9,' > "$KB/facts/candidates.csv" +out="$("$PYTHON" -m factlog eject --orphans --target "$KB" 2>&1)" +printf '%s' "$out" | grep -qF "no orphaned sources found" \ + && [ -f "$KB/runs/sources/report.html.md" ] && [ -f "$KB/runs/sources/sub/report.html.md" ] \ + && ok "path-form provenance header: mirrored conversions are not orphans" \ + || bad "path-form header wrongly orphaned a live conversion" + +# --- a path-form header on a FLAT conversion still pairs by basename ---------- +# An original named by an explicit path converts flat (no subtree to mirror) +# while its header can still spell a subdir. Reconstructing the original's +# location from the header instead of the conversion's own path would look for +# sources/sub/... under a flat conversion and auto-delete every such file. +KB="$(mktemp -d)/wiki" +"$PYTHON" -m factlog init --target "$KB" >/dev/null +mkdir -p "$KB/sources/sub" +printf 'nested\n' > "$KB/sources/sub/report.html" +printf '\nx\n' \ + > "$KB/runs/sources/report.html.md" +printf '%s\n%s\n' "$H" 'A,rel,B,runs/sources/report.html.md,confirmed,0.9,' > "$KB/facts/candidates.csv" +out="$("$PYTHON" -m factlog eject --orphans --target "$KB" 2>&1)" +printf '%s' "$out" | grep -qF "no orphaned sources found" && [ -f "$KB/runs/sources/report.html.md" ] \ + && ok "path-form header on a flat conversion pairs by basename (not auto-deleted)" \ + || bad "flat conversion with a path header wrongly ejected" + +# --- a header with no filename component is kept, like an empty one ----------- +# `source: /` carries no original name; it must not be reconstructed into an +# original named after the conversion's subdir. +KB="$(mktemp -d)/wiki" +"$PYTHON" -m factlog init --target "$KB" >/dev/null +mkdir -p "$KB/sources/sub" "$KB/runs/sources/sub" +printf 'PK\003\004\000' > "$KB/sources/sub/real.pdf" +printf '\nx\n' \ + > "$KB/runs/sources/sub/slash.md" +printf '%s\n%s\n' "$H" 'A,rel,B,runs/sources/sub/slash.md,confirmed,0.9,' > "$KB/facts/candidates.csv" +set +e; "$PYTHON" -m factlog eject sub --target "$KB" --dry-run >/dev/null 2>&1; rc=$?; set -e +[ "$rc" -ne 0 ] && ok "a filename-less header ('source: /') names no original" \ + || bad "a filename-less header matched the subdir name" + # --- non-KB path errors ------------------------------------------------------- set +e; "$PYTHON" -m factlog eject anything --target "$(mktemp -d)" >/dev/null 2>&1; rc=$?; set -e [ "$rc" -ne 0 ] && ok "eject on a non-KB path errors" || bad "non-KB path should error" From de04d3aecd4f3733fc2108821d6c74c5d25b4af5 Mon Sep 17 00:00:00 2001 From: Seoyun Lee Date: Wed, 29 Jul 2026 11:50:26 +0900 Subject: [PATCH 2/4] =?UTF-8?q?fix:=20=EA=B2=BD=EB=A1=9C=EB=A1=9C=20?= =?UTF-8?q?=EC=A7=80=EC=A0=95=ED=95=9C=20eject=20=EA=B0=80=20=EB=8B=A4?= =?UTF-8?q?=EB=A5=B8=20=EB=94=94=EB=A0=89=ED=84=B0=EB=A6=AC=20=EB=8F=99?= =?UTF-8?q?=EB=AA=85=20=EC=9B=90=EB=B3=B8=EC=9D=84=20=EA=B1=B4=EB=93=9C?= =?UTF-8?q?=EB=A6=AC=EC=A7=80=20=EC=95=8A=EB=8A=94=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `eject sub/report.html` 이 `sources/report.html` 의 변환본까지 지웠다. 경로 분기가 양변을 basename 으로 줄여 비교해, 경로 지정이 사실상 basename 매칭으로 퇴화해 있었다. 사용자가 명시적으로 좁혀 지정했는데 요청하지 않은 변환본이 종료 코드 0 과 함께 사라진다. 인자를 먼저 정규화(`selector()`)한 뒤 경로 분기가 sources 상대 경로끼리 비교하도록 바꾼다. - `sub/report.html`·`./report.html` → sources 기준 경로. 그 경로에서 만들어진 변환본만 매칭한다. - `sources/...` 접두는 상대형에서도 벗겨 KB 기준 ref 와 sources 상대 경로를 함께 얻는다(업스트림은 절대·상대 어느 쪽에서도 접두를 벗기지 않았고, `eject sources/report.html` 이 3건이던 것은 정확 ref 1건 + 동명 basename 변환본 2건이었다). - `runs/sources/...` 는 변환본 자신을 가리키므로 정확 ref 매칭만 한다. - 절대 경로만 `resolve()` 로 환원한다. 상대 경로를 cwd 기준으로 resolve 하면 KB 안에서 친 `sub/report.html` 이 KB 밖 경로가 되어 결함이 되살아난다. KB 루트도 함께 resolve 한다(/tmp -> /private/tmp 심링크). - `sources/` 밖의 원본은 ingest 가 항상 flat 변환본을 만들므로, 그 경로 지정은 flat 변환본(재구성 origin 이 bare basename 인 것)만 매칭한다. KB 밖 원본을 적재하는 정당한 용례는 유지되고 중첩 변환본에는 닿지 않는다. - `normpath`/casefold 는 쓰지 않는다. 경로는 적힌 그대로 비교한다. 의도된 부수 변화 4건(핀으로 고정): 1. `sub/../report.html` 처럼 접히지 않은 상대 경로는 2건 매칭 → 0건(exit 1). 2. 대소문자가 다른 경로는 대소문자 무시 파일시스템에서도 매칭하지 않는다. 3. 심링크 디렉터리 이름을 거친 상대 경로는 매칭하지 않는다(실제 경로나 절대 경로로 지정). 한계로 문서에 기록한다. 4. 절대 경로와 정규화 가능한 상대 경로(`./sources/x`, `sources//x`)가 그 ref 자체를 매칭하게 된다. 새로 걸리는 것은 언제나 인자가 가리킨 그 파일 하나뿐 이고 미러 변환본이 새로 걸리는 입력은 없지만, `--delete-original` 과 결합하면 업스트림이 지우지 않던 원본 1건을 지운다. KB 밖 심링크가 `sources/` 안을 가리키는 경우도 같다(resolve 로 실제 원본에 도달한다). 이슈 수용 기준 AC2("`eject report.html` 이 sub/ 를 안 건드린다")는 코드로 만족시키지 않는다. bare filename 은 경로 지정이 아니라 의도적으로 넓은 매칭이며 (AC3 가 그 불변을 요구한다) `/` 가 없어 경로 분기에 도달하지도 않는다. 좁히려면 `./report.html` 또는 `sources/report.html` 로 지정한다 — 문서에 명시했다. `eject --orphans` 출력은 6시나리오(정상 ingest 산출물 / legacy bare 헤더 / flat 변환본 + 경로 헤더 / 헤더·미러 불일치 / 진짜 고아 / 헤더 없음)에서 업스트림 c6d359d 와 바이트 단위로 동일함을 실측했다. 업스트림 #324 --- docs/reference/ignore-eject.en.md | 37 ++++++++++++ docs/reference/ignore-eject.md | 35 ++++++++++++ factlog/cli.py | 84 +++++++++++++++++++++++---- tests/test_eject_cmd.sh | 95 +++++++++++++++++++++++++++++++ 4 files changed, 240 insertions(+), 11 deletions(-) diff --git a/docs/reference/ignore-eject.en.md b/docs/reference/ignore-eject.en.md index bab5ef99..a9d6763f 100644 --- a/docs/reference/ignore-eject.en.md +++ b/docs/reference/ignore-eject.en.md @@ -50,6 +50,43 @@ factlog eject report.pdf --delete-original # also delete the user's original un factlog eject report.pdf --dry-run # show the planned changes, modify nothing ``` +### How a source is named — filenames are wide, paths are narrow + +| What you name | What it matches | +|---------|---------| +| stem `report` | **every** source with that stem, and their conversions | +| filename `report.html` | **every** source with that filename in any directory, and their conversions | +| path `sub/report.html`, `./report.html` | only the conversion made from the original at that path under `sources/` | +| KB-relative ref `sources/sub/report.html` | that original + the conversion made from it | +| absolute path `/kb/sources/sub/report.html` | the same (it reduces to the KB-relative ref) | + +**A bare filename is not a path.** `factlog eject report.html` matches widely by +design, so it also catches `sources/sub/report.html`. To take only the top-level +one, name a path: `./report.html` or `sources/report.html`. + +Given a path, only the conversion made from *that* path matches — conversions +mirror the original's subdirectory under `runs/sources/`, so +`factlog eject sub/report.html` deletes `runs/sources/sub/report.html.md` and +leaves `runs/sources/report.html.md`, made from a same-name original in another +directory, alone. + +A path is compared **as written**: no `..` folding and no case folding, so +`sub/../report.html` and `SUB/report.html` match nothing and exit 1 — even on a +case-insensitive filesystem. Only an absolute path is resolved (symlinks +included) and reduced to a KB-relative ref; a relative path is not, so one +spelled through a symlinked directory name (`link/report.html`) matches nothing — +use the real path (`real/report.html`) or an absolute path. + +To delete the original too (`--delete-original`), name it by its KB-relative ref +(`sources/sub/report.html`) or by absolute path. A sources-relative path +(`sub/report.html`) names the **conversion** made from it. + +An original ingested from outside `sources/` (e.g. +`factlog ingest /elsewhere/report.html`) has no subtree to mirror, so its +conversion is written flat under `runs/sources/`. Naming that original by path +matches the flat conversion only: a conversion in a subdirectory cannot have come +from it. + ### Removing a single fact (`--fact`) When a source is fine but one extracted fact is wrong, retire just that fact — diff --git a/docs/reference/ignore-eject.md b/docs/reference/ignore-eject.md index a7bb10a0..4ac12b98 100644 --- a/docs/reference/ignore-eject.md +++ b/docs/reference/ignore-eject.md @@ -49,6 +49,41 @@ factlog eject report.pdf --delete-original # also delete the user's original un factlog eject report.pdf --dry-run # show the planned changes, modify nothing ``` +### 이름 지정 방식 — 파일명은 넓고, 경로는 좁습니다 + +| 지정 형태 | 매칭 범위 | +|---------|---------| +| 어간 `report` | 그 어간을 가진 **모든** 소스와 그 변환본 | +| 파일명 `report.html` | 디렉터리를 가리지 않고 그 이름을 가진 **모든** 소스와 그 변환본 | +| 경로 `sub/report.html`, `./report.html` | `sources/` 기준 그 경로의 원본에서 만들어진 변환본만 | +| KB 기준 ref `sources/sub/report.html` | 그 원본 + 그 원본에서 만들어진 변환본 | +| 절대 경로 `/kb/sources/sub/report.html` | 위와 동일(KB 기준 ref 로 환원) | + +**파일명은 경로 지정이 아닙니다.** `factlog eject report.html` 은 의도적으로 넓게 +매칭되므로 `sources/sub/report.html` 쪽도 함께 걸립니다. 최상위 것만 빼려면 +`./report.html` 이나 `sources/report.html` 처럼 경로로 지정하십시오. + +경로를 주면 그 경로에서 만들어진 변환본만 매칭됩니다 — 변환본은 `runs/sources/` +아래에 원본의 서브디렉터리를 미러링하므로, `factlog eject sub/report.html` 은 +`runs/sources/sub/report.html.md` 만 지우고 다른 디렉터리의 동명 원본이 만든 +`runs/sources/report.html.md` 는 건드리지 않습니다. + +경로는 **적힌 그대로** 비교합니다. `..` 를 접거나 대소문자를 무시하지 않으므로, +`sub/../report.html` 과 `SUB/report.html` 은 (파일시스템이 대소문자를 구분하지 +않더라도) 아무것도 매칭하지 않고 종료 코드 1 로 끝납니다. 심링크가 풀리는 것은 +절대 경로뿐입니다 — 상대 경로는 풀지 않으므로, 심링크 디렉터리 이름을 거쳐 적은 +상대 경로(`link/report.html`)는 매칭되지 않습니다. 실제 경로(`real/report.html`)나 +절대 경로로 지정하십시오. + +`--delete-original` 로 원본까지 지우려면 원본의 KB 기준 ref(`sources/sub/report.html`) +나 절대 경로로 지정하십시오. `sources/` 기준 경로(`sub/report.html`)는 그 경로에서 +만들어진 **변환본**을 가리킵니다. + +`sources/` 밖의 원본을 적재했다면(예: `factlog ingest /elsewhere/report.html`) +미러링할 서브트리가 없어 변환본이 `runs/sources/` 바로 아래 평면으로 만들어집니다. +그 원본을 경로로 지정하면 평면 변환본만 매칭되고, 서브디렉터리의 변환본은 그 +경로에서 만들어졌을 수 없으므로 절대 걸리지 않습니다. + ### 사실 하나만 제거 (`--fact`) 소스 자체는 멀쩡한데 추출된 사실 하나가 잘못된 경우, 그 사실만 폐기할 수 diff --git a/factlog/cli.py b/factlog/cli.py index d634dc61..f72e63b9 100644 --- a/factlog/cli.py +++ b/factlog/cli.py @@ -2366,17 +2366,73 @@ def origin_name(ref: str) -> str | None: origin = conv_origin.get(ref) return PurePosixPath(origin).name if origin is not None else None - def matches(ref: str, name: str) -> bool: - name = nfc(name) - rp, np_ = Path(ref), Path(name) - if ref == name: # exact KB-relative ref + try: + troot = target.resolve() + except OSError: + troot = target + + def selector(name: str) -> tuple[set[str], str | None, str]: + """Canonicalise one `eject ` argument into what the matcher needs: + + refs — the KB-relative ref(s) the argument names exactly; + src_rel — the original's path *relative to sources/*, when a path was + given; compared against conv_origin to reach the conversion + that path produced. None when the argument names no original + under sources/ (a conversion ref, or a bare name); + raw — the argument as written, for the bare filename / stem rules. + """ + raw = nfc(name) + p = Path(raw) + if p.is_absolute(): + # Resolve an *absolute* argument, and the root with it: this machine + # can reach the KB through a symlink (/tmp -> /private/tmp), and an + # unresolved argument would then look like it lies outside the KB. + # A relative argument is never resolved — resolving it against the + # cwd would turn `sub/report.html`, typed inside the KB, into an + # outside-the-KB path and drop it back to basename matching, which + # is the deletion #324 is about. + try: + p = p.resolve() + except OSError: + pass + try: + kb_rel = nfc(p.relative_to(troot).as_posix()) + except ValueError: + kb_rel = None + if kb_rel is not None and (kb_rel == "sources" or kb_rel.startswith("sources/")): + return {kb_rel}, kb_rel[len("sources/"):] or None, raw + # An original outside sources/ — anywhere else in the KB, or outside + # it entirely — has no subtree for ingest to mirror, so it always + # converts to a *flat* runs/sources/ whose rebuilt origin is a + # bare basename. Comparing against that basename keeps the legitimate + # case working while leaving every mirrored conversion out of reach. + return ({kb_rel} if kb_rel is not None else set()), nfc(p.name), raw + # A relative argument is read KB-relative (`sources/...`, `runs/...`) or + # sources-relative (`sub/report.html`). PurePosixPath folds "./" and "//" + # but keeps ".." verbatim: no normpath/realpath here, so the comparison + # never rewrites a path into something the recorded origin spells + # differently. + norm = PurePosixPath(raw).as_posix() if "/" in raw else raw + if norm.startswith("sources/"): + return {raw, norm}, norm[len("sources/"):] or None, raw + if norm.startswith("runs/sources/"): + return {raw, norm}, None, raw # names a conversion, not an original + return {raw, norm}, norm, raw + + def matches(ref: str, sel: tuple[set[str], str | None, str]) -> bool: + refs, src_rel, name = sel + if ref in refs: # exact KB-relative ref return True is_conv = ref.startswith("runs/sources/") if "/" in name: # A path was given: the exact original is handled above; for a - # binary original also match the conversion it produced (by - # recorded origin). Same-basename files elsewhere are NOT matched. - return is_conv and origin_name(ref) == np_.name + # binary original also match the conversion it produced — the one + # whose recorded origin *is* that sources-relative path. #324: a + # same-name original in another directory is NOT matched, because + # both sides keep their directory instead of collapsing to a + # basename. + return is_conv and src_rel is not None and conv_origin.get(ref) == src_rel + rp, np_ = Path(ref), Path(name) if np_.suffix: # a bare filename with an extension if not is_conv: return rp.name == np_.name # an original with that filename @@ -2445,7 +2501,8 @@ def matches(ref: str, name: str) -> bool: print(f"factlog eject (KB: {target}): orphan scan — {len(matched)} orphaned source(s)") else: for name in args.sources: - hits = {ref for ref in all_refs if matches(ref, name)} + sel = selector(name) + hits = {ref for ref in all_refs if matches(ref, sel)} if hits: matched |= hits else: @@ -2489,9 +2546,14 @@ def cmd_eject(args: argparse.Namespace) -> int: by default (kept for audit), or removed entirely with --purge; - optionally deletes the user's original under sources/ with --delete-original (off by default: ingest never created it). - A source is named by its filename, stem, or KB-relative path. Naming the - binary original (e.g. report.pptx) also matches its runs/sources/ - conversion; a bare stem matches every source with that stem. eject also + A source is named by its filename, stem, or path. Naming the binary original + (e.g. report.pptx) also matches its runs/sources/ conversion; a bare + stem matches every source with that stem. A filename is deliberately wide — + it matches that name in every directory — while a *path* is narrow: it + selects the original at that path and the conversion made from it, never a + same-name original elsewhere (#324). Paths are read relative to sources/ (or + as a KB-relative ref / absolute path) and compared as written: no ".." + folding, no case folding, and only an absolute path is resolved. eject also catches a source cited only in candidates.csv (an already-orphaned ref). Orphan mode (`eject --orphans`) selects every orphaned source automatically diff --git a/tests/test_eject_cmd.sh b/tests/test_eject_cmd.sh index b1b43272..663fd159 100644 --- a/tests/test_eject_cmd.sh +++ b/tests/test_eject_cmd.sh @@ -12,6 +12,9 @@ # - --purge deletes the candidate row instead of superseding it # - --delete-original also removes the user's original under sources/ # - a bare stem matches; an unknown name errors (rc != 0); non-KB path errors +# - naming a *path* stays inside that path: `eject sub/report.html` never +# reaches a same-name original in another directory (#324), while a bare +# filename keeps matching every directory # # Usage: bash tests/test_eject_cmd.sh @@ -111,6 +114,98 @@ printf '%s\n%s\n%s\n' "$H" \ [ ! -f "$KB/sources/a/dup.md" ] && [ -f "$KB/sources/b/dup.md" ] && ok "full path ejects only that file, not the same-name sibling" || bad "full path matched across directories" grep -q "sources/b/dup.md,confirmed," "$KB/facts/candidates.csv" && ok "sibling's fact preserved" || bad "sibling fact wrongly retired" +# ============================================================================= +# #324: naming a path must not reach a same-name original in another directory +# ============================================================================= + +# Two same-name originals (sources/report.html and sources/sub/report.html), each +# with its mirrored conversion. $1 = KB, $2 = header style (path|legacy): the +# nested conversion's provenance records either the #214 sources-relative path or +# a legacy bare basename. Both must select identically — the conversion's own +# mirrored location is what pairs it with its original. +seed_dup() { # $1 = KB path, $2 = path|legacy + local kb="$1" nested_src="sub/report.html" + [ "$2" = "legacy" ] && nested_src="report.html" + "$PYTHON" -m factlog init --target "$kb" >/dev/null + mkdir -p "$kb/sources/sub" "$kb/runs/sources/sub" + printf 'top\n' > "$kb/sources/report.html" + printf 'nested\n' > "$kb/sources/sub/report.html" + printf '\ntop\n' \ + > "$kb/runs/sources/report.html.md" + printf '\nnested\n' \ + "$nested_src" > "$kb/runs/sources/sub/report.html.md" + printf '%s\n%s\n%s\n%s\n%s\n' "$H" \ + 'A,rel,B,runs/sources/report.html.md,confirmed,0.9,' \ + 'C,rel,D,runs/sources/sub/report.html.md,confirmed,0.9,' \ + 'E,rel,F,sources/report.html,confirmed,0.9,' \ + 'G,rel,H,sources/sub/report.html,confirmed,0.9,' > "$kb/facts/candidates.csv" +} + +# --- a sources-relative path ejects only the conversion made from that path ---- +for style in path legacy; do + KB="$(mktemp -d)/wiki"; seed_dup "$KB" "$style" + "$PYTHON" -m factlog eject sub/report.html --target "$KB" >/dev/null 2>&1 + [ ! -f "$KB/runs/sources/sub/report.html.md" ] && [ -f "$KB/runs/sources/report.html.md" ] \ + && ok "[$style header] 'sub/report.html' ejects the nested conversion only" \ + || bad "[$style header] path eject hit the same-name conversion in another directory" + grep -q "A,rel,B,runs/sources/report.html.md,confirmed," "$KB/facts/candidates.csv" \ + && ok "[$style header] the top-level conversion's fact is not retired" \ + || bad "[$style header] unrequested fact retired" + [ -f "$KB/sources/report.html" ] && [ -f "$KB/sources/sub/report.html" ] \ + && ok "[$style header] both originals kept" || bad "[$style header] an original was deleted" +done + +# --- './name' narrows to the root-level original's conversion ----------------- +KB="$(mktemp -d)/wiki"; seed_dup "$KB" path +"$PYTHON" -m factlog eject ./report.html --target "$KB" >/dev/null 2>&1 +[ ! -f "$KB/runs/sources/report.html.md" ] && [ -f "$KB/runs/sources/sub/report.html.md" ] \ + && ok "'./report.html' ejects the root-level conversion only" || bad "'./report.html' reached into sub/" + +# --- a KB-relative 'sources/...' path matches that original + its conversion --- +KB="$(mktemp -d)/wiki"; seed_dup "$KB" path +out="$("$PYTHON" -m factlog eject sources/report.html --target "$KB" --delete-original 2>&1)" +[ ! -f "$KB/sources/report.html" ] && [ ! -f "$KB/runs/sources/report.html.md" ] \ + && ok "'sources/report.html' ejects that original and its conversion" || bad "KB-relative path eject incomplete" +[ -f "$KB/sources/sub/report.html" ] && [ -f "$KB/runs/sources/sub/report.html.md" ] \ + && ok "'sources/report.html' leaves the sub/ pair untouched" || bad "KB-relative path eject hit sub/" + +# --- a bare filename stays deliberately wide (a filename is not a path) ------- +KB="$(mktemp -d)/wiki"; seed_dup "$KB" path +out="$("$PYTHON" -m factlog eject report.html --target "$KB" --dry-run 2>&1)" +[ "$(printf '%s' "$out" | grep -c '^ - ')" -eq 4 ] \ + && ok "a bare filename still matches every source with that name (4 refs)" \ + || bad "bare filename matching narrowed: $(printf '%s' "$out" | grep -c '^ - ') refs" + +# --- an absolute path inside the KB resolves to its KB-relative ref ----------- +KB="$(mktemp -d)/wiki"; seed_dup "$KB" path +"$PYTHON" -m factlog eject "$KB/sources/sub/report.html" --target "$KB" >/dev/null 2>&1 +[ ! -f "$KB/runs/sources/sub/report.html.md" ] && [ -f "$KB/runs/sources/report.html.md" ] \ + && ok "an absolute path under sources/ ejects only its own conversion" || bad "absolute path matched by basename" + +# --- an absolute original outside the KB matches only a FLAT conversion ------- +# ingest gives a path outside sources/ no subtree to mirror, so its conversion is +# flat; a mirrored conversion can never have come from that path. +KB="$(mktemp -d)/wiki"; seed_dup "$KB" path +OUTDIR="$(mktemp -d)"; printf 'elsewhere\n' > "$OUTDIR/report.html" +"$PYTHON" -m factlog eject "$OUTDIR/report.html" --target "$KB" >/dev/null 2>&1 +[ ! -f "$KB/runs/sources/report.html.md" ] && [ -f "$KB/runs/sources/sub/report.html.md" ] \ + && ok "an outside-the-KB original matches its flat conversion, not a mirrored one" \ + || bad "outside-the-KB path reached a mirrored conversion" + +# --- a path is compared as written: '..' and case differences do not match ---- +# Deliberate: eject never normalises a path away from the form a provenance +# header records, and never case-folds. Both now select nothing (rc != 0) +# instead of falling back to a basename match. +KB="$(mktemp -d)/wiki"; seed_dup "$KB" path +set +e +"$PYTHON" -m factlog eject sub/../report.html --target "$KB" --dry-run >/dev/null 2>&1; rc=$? +"$PYTHON" -m factlog eject SUB/report.html --target "$KB" --dry-run >/dev/null 2>&1; rc2=$? +set -e +[ "$rc" -ne 0 ] && ok "a '..' path selects nothing instead of falling back to the basename" \ + || bad "'..' path still matched by basename" +[ "$rc2" -ne 0 ] && ok "a case-different path selects nothing (no case folding)" \ + || bad "case-different path matched" + # --- a candidates.csv whose header lacks 'status' is not truncated ------------- KB="$(mktemp -d)/wiki" "$PYTHON" -m factlog init --target "$KB" >/dev/null From a574ecaf3a9a0f077ef9b30d46d7744dfd755974 Mon Sep 17 00:00:00 2001 From: Seoyun Lee Date: Wed, 29 Jul 2026 12:20:56 +0900 Subject: [PATCH 3/4] =?UTF-8?q?fix:=20=ED=99=98=EC=9B=90=20=EB=B6=88?= =?UTF-8?q?=EA=B0=80=EB=8A=A5=ED=95=9C=20eject=20=EC=9D=B8=EC=9E=90?= =?UTF-8?q?=EA=B0=80=20=EC=A3=BD=EA=B1=B0=EB=82=98=20=EB=B9=88=20origin=20?= =?UTF-8?q?=EA=B3=BC=20=EC=B6=A9=EB=8F=8C=ED=95=98=EC=A7=80=20=EC=95=8A?= =?UTF-8?q?=EB=8A=94=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 리뷰에서 나온 두 건을 고치고 핀 3건을 동봉한다. 1) `Path.resolve()` 는 심링크 루프를 `OSError` 가 아니라 `RuntimeError` 로 보고하고(3.12 `pathlib.py` check_eloop), NUL 이 박힌 경로는 `ValueError` 를 던진다. `except OSError` 로만 감싼 탓에 `eject <루프경로>` 가 사용자에게 전체 트레이스백을 뱉었다 — 삭제 이전 단계라 데이터 손실은 없지만 c6d359d 대비 회귀다. 두 곳(KB 루트 환원, 인자 환원) 모두 확장한다. 실측: `ln -s b a; ln -s a b` 후 `eject <루프>/absent.html --dry-run` 이 트레이스백 + exit 1 이었던 것이, c6d359d 와 같은 `no source matches` + exit 1 로 돌아온다. 2) 경로 분기의 `src_rel is not None` 가드가 빈 문자열을 배제하지 못했다. 루트 경로(`/`, `//`)는 `p.name == ""` 라 `src_rel` 이 "" 가 되고, 이는 `source: /` 헤더에 저장하는 빈-origin 센티널과 같은 값이다. 그래서 `eject /` 가 그 변환본을 선택했다(c6d359d 도 동일해 회귀는 아니다). `if src_rel:` 로 바꿔 원천 차단한다. 빈 `src_rel` 은 어차피 가리킬 원본이 없으므로 매칭이 좁아지는 방향으로만 바뀐다. 핀 3건(전부 수정 전 실패 확인): - 심링크 루프 인자: 종료 코드 != 0 이고 출력에 Traceback 이 없다. - `eject /`: 아무것도 선택하지 않는다. - 절대 경로 + `--delete-original`: 지목한 원본과 그 변환본 **정확히 1쌍**만 지우고 다른 디렉터리의 동명 형제는 원본·변환본 모두 남긴다. 기존 절대경로 케이스가 `--delete-original` 없이 변환본만 검사해 비어 있던 자리다. `eject --orphans` 6시나리오 출력은 이 변경 뒤에도 c6d359d 와 동일하고, select 27케이스도 직전 커밋과 동일하다(정상 경로는 좁아지지 않았다). 업스트림 #324 --- factlog/cli.py | 15 +++++++++++---- tests/test_eject_cmd.sh | 35 ++++++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/factlog/cli.py b/factlog/cli.py index f72e63b9..d5c05d95 100644 --- a/factlog/cli.py +++ b/factlog/cli.py @@ -2366,9 +2366,14 @@ def origin_name(ref: str) -> str | None: origin = conv_origin.get(ref) return PurePosixPath(origin).name if origin is not None else None + # resolve() reports a symlink loop as RuntimeError (not OSError) and a path + # with an embedded NUL as ValueError, and neither may reach the user as a + # traceback: an unresolvable argument simply matches nothing. + _RESOLVE_ERRORS = (OSError, RuntimeError, ValueError) + try: troot = target.resolve() - except OSError: + except _RESOLVE_ERRORS: troot = target def selector(name: str) -> tuple[set[str], str | None, str]: @@ -2393,7 +2398,7 @@ def selector(name: str) -> tuple[set[str], str | None, str]: # is the deletion #324 is about. try: p = p.resolve() - except OSError: + except _RESOLVE_ERRORS: pass try: kb_rel = nfc(p.relative_to(troot).as_posix()) @@ -2430,8 +2435,10 @@ def matches(ref: str, sel: tuple[set[str], str | None, str]) -> bool: # whose recorded origin *is* that sources-relative path. #324: a # same-name original in another directory is NOT matched, because # both sides keep their directory instead of collapsing to a - # basename. - return is_conv and src_rel is not None and conv_origin.get(ref) == src_rel + # basename. An *empty* src_rel (a root path like "/" names no file) + # must not compare equal either: the empty string is also the + # no-usable-origin sentinel stored for a header like "source: /". + return bool(is_conv and src_rel and conv_origin.get(ref) == src_rel) rp, np_ = Path(ref), Path(name) if np_.suffix: # a bare filename with an extension if not is_conv: diff --git a/tests/test_eject_cmd.sh b/tests/test_eject_cmd.sh index 663fd159..d1edb5d5 100644 --- a/tests/test_eject_cmd.sh +++ b/tests/test_eject_cmd.sh @@ -182,6 +182,32 @@ KB="$(mktemp -d)/wiki"; seed_dup "$KB" path [ ! -f "$KB/runs/sources/sub/report.html.md" ] && [ -f "$KB/runs/sources/report.html.md" ] \ && ok "an absolute path under sources/ ejects only its own conversion" || bad "absolute path matched by basename" +# --- an absolute path + --delete-original removes exactly that one original --- +# Reducing an absolute path to its KB-relative ref makes it match the original +# itself, so --delete-original now reaches an original that a basename fallback +# never selected. It must stay at exactly one file: the same-name sibling in the +# other directory keeps both its original and its conversion. +KB="$(mktemp -d)/wiki"; seed_dup "$KB" path +"$PYTHON" -m factlog eject "$KB/sources/sub/report.html" --target "$KB" --delete-original >/dev/null 2>&1 +[ ! -f "$KB/sources/sub/report.html" ] && [ ! -f "$KB/runs/sources/sub/report.html.md" ] \ + && ok "absolute path + --delete-original removes that original and its conversion" \ + || bad "absolute path + --delete-original did not remove the named original" +[ -f "$KB/sources/report.html" ] && [ -f "$KB/runs/sources/report.html.md" ] \ + && ok "absolute path + --delete-original leaves the same-name sibling intact" \ + || bad "--delete-original deleted a file in another directory" + +# --- an unresolvable path errors like any unknown name (no traceback) --------- +# resolve() raises RuntimeError on a symlink loop, not OSError; letting it escape +# turned an ordinary no-match into a crash. +KB="$(mktemp -d)/wiki"; seed_dup "$KB" path +LOOP="$(mktemp -d)"; ln -s "$LOOP/b" "$LOOP/a"; ln -s "$LOOP/a" "$LOOP/b" +set +e +err="$("$PYTHON" -m factlog eject "$LOOP/a/absent.html" --target "$KB" --dry-run 2>&1)"; rc=$? +set -e +[ "$rc" -ne 0 ] && ! printf '%s' "$err" | grep -qF "Traceback" \ + && ok "a symlink-loop path reports no match instead of crashing" \ + || bad "symlink-loop path crashed: $(printf '%s' "$err" | tail -1)" + # --- an absolute original outside the KB matches only a FLAT conversion ------- # ingest gives a path outside sources/ no subtree to mirror, so its conversion is # flat; a mirrored conversion can never have come from that path. @@ -461,9 +487,16 @@ printf 'PK\003\004\000' > "$KB/sources/sub/real.pdf" printf '\nx\n' \ > "$KB/runs/sources/sub/slash.md" printf '%s\n%s\n' "$H" 'A,rel,B,runs/sources/sub/slash.md,confirmed,0.9,' > "$KB/facts/candidates.csv" -set +e; "$PYTHON" -m factlog eject sub --target "$KB" --dry-run >/dev/null 2>&1; rc=$?; set -e +set +e +"$PYTHON" -m factlog eject sub --target "$KB" --dry-run >/dev/null 2>&1; rc=$? +# ...and a root path, which names no file either, must not collide with that +# sentinel and select the conversion. +"$PYTHON" -m factlog eject / --target "$KB" --dry-run >/dev/null 2>&1; rc2=$? +set -e [ "$rc" -ne 0 ] && ok "a filename-less header ('source: /') names no original" \ || bad "a filename-less header matched the subdir name" +[ "$rc2" -ne 0 ] && ok "a root path selects nothing (no empty-origin collision)" \ + || bad "'/' matched a conversion through the empty-origin sentinel" # --- non-KB path errors ------------------------------------------------------- set +e; "$PYTHON" -m factlog eject anything --target "$(mktemp -d)" >/dev/null 2>&1; rc=$?; set -e From e8916b6139215c2fe5eb7e5a6d2e447f39524098 Mon Sep 17 00:00:00 2001 From: Seoyun Lee Date: Wed, 29 Jul 2026 12:21:08 +0900 Subject: [PATCH 4/4] =?UTF-8?q?docs:=20=EA=B2=BD=EB=A1=9C=20=ED=98=95?= =?UTF-8?q?=ED=83=9C=EB=B3=84=EB=A1=9C=20=EC=9B=90=EB=B3=B8=EC=9D=B4=20?= =?UTF-8?q?=EB=8C=80=EC=83=81=EC=97=90=20=EB=93=A4=EC=96=B4=EA=B0=80?= =?UTF-8?q?=EB=8A=94=EC=A7=80=20=EA=B5=AC=EB=B6=84=ED=95=B4=20=EC=A0=81?= =?UTF-8?q?=EB=8A=94=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "최상위 것만 빼려면 `./report.html` 이나 `sources/report.html`" 이 두 형태를 등가처럼 나열했는데, 바로 위 표는 둘을 구분한다 — `./report.html` 은 변환본만, `sources/report.html` 은 원본까지 매칭한다. `--delete-original` 을 함께 쓰는 독자가 원본이 지워지는 조건을 오해할 수 있어 한정어를 넣는다. ko/en 동일 문단. 업스트림 #324 --- docs/reference/ignore-eject.en.md | 5 ++++- docs/reference/ignore-eject.md | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/reference/ignore-eject.en.md b/docs/reference/ignore-eject.en.md index a9d6763f..c2b5c38f 100644 --- a/docs/reference/ignore-eject.en.md +++ b/docs/reference/ignore-eject.en.md @@ -62,7 +62,10 @@ factlog eject report.pdf --dry-run # show the planned changes, modify noth **A bare filename is not a path.** `factlog eject report.html` matches widely by design, so it also catches `sources/sub/report.html`. To take only the top-level -one, name a path: `./report.html` or `sources/report.html`. +one, name a path — `./report.html` for its conversion alone, or the KB-relative +ref `sources/report.html` to include the original itself (which is what makes +`--delete-original` actually delete it). The two table rows above are that +difference. Given a path, only the conversion made from *that* path matches — conversions mirror the original's subdirectory under `runs/sources/`, so diff --git a/docs/reference/ignore-eject.md b/docs/reference/ignore-eject.md index 4ac12b98..50ff1dd9 100644 --- a/docs/reference/ignore-eject.md +++ b/docs/reference/ignore-eject.md @@ -60,8 +60,10 @@ factlog eject report.pdf --dry-run # show the planned changes, modify noth | 절대 경로 `/kb/sources/sub/report.html` | 위와 동일(KB 기준 ref 로 환원) | **파일명은 경로 지정이 아닙니다.** `factlog eject report.html` 은 의도적으로 넓게 -매칭되므로 `sources/sub/report.html` 쪽도 함께 걸립니다. 최상위 것만 빼려면 -`./report.html` 이나 `sources/report.html` 처럼 경로로 지정하십시오. +매칭되므로 `sources/sub/report.html` 쪽도 함께 걸립니다. 최상위 것만 빼려면 경로로 +지정하십시오 — 변환본만 지우려면 `./report.html`, 원본까지 대상에 넣으려면(즉 +`--delete-original` 이 실제로 원본을 지우게 하려면) `sources/report.html` 처럼 KB +기준 ref 로 지정합니다. 위 표의 두 줄이 그 차이입니다. 경로를 주면 그 경로에서 만들어진 변환본만 매칭됩니다 — 변환본은 `runs/sources/` 아래에 원본의 서브디렉터리를 미러링하므로, `factlog eject sub/report.html` 은