feat(learner): collapse cross-OS duplicate projects in the registry - #22
Conversation
Luispitik
left a comment
There was a problem hiding this comment.
The diagnosis is right and in scope: project ids are sha256(remote || root), so a no-remote project gets a different id per OS and duplicates in synced registries. The diff applies clean on v4.6.1 and the new code is safe (no eval, execFileSync with args). But I reproduced three functional problems by running the patched hook, so this needs another iteration:
- Same-machine false merge (regression, data loss). Two different no-remote projects that share a folder name on the same machine (think
clientA/scriptsandclientB/scripts) collapse silently into one entry, and one root disappears from the registry (roots.windowsoverwritten — same os-family). Reproduced with the real hook./eodresolves roots by name, so it would attribute git commits to the wrong project. Fix: only collapse by name when the os-family actually differs, e.g.registry.projects.find(p => p && p.crossKey === crossKey && p.root && projectRoot && osFamily(p.root) !== osFamily(projectRoot))
- Pre-existing duplicates never merge. The lookup matches by id first, so a registry that already contains both entries (the synced-via-Nextcloud scenario that motivates this PR) keeps both forever — each machine just updates its own entry. Reproduced. Add a post-upsert migration pass that merges entries with identical
crossKey(union of aliases/roots, oldestcreated, newestlast_seen) — or hook it into_dream.shhygiene. As written, the PR only prevents future duplicates from a clean registry; it doesn't cure the reported symptom. - The new test fails on Windows Git Bash (suite exit 1; 1 of the 5 cases): MSYS converts the posix fixture path
/Users/tester/CrossProjpassed as argv intoC:/Program Files/Git/Users/..., soroots.posixnever registers. ExportMSYS_NO_PATHCONV=1/MSYS2_ARG_CONV_EXCL='*'at the top of the test, or pass fixture paths via env vars instead of argv. Also drop the|| r.projects[0].id==='bbbbbbbbbbbb'clause in test 3 — as written it passes even when the posix run was never processed, which is exactly what happens on Git Bash today.
Please also:
- Add an anti-regression test: two same-name no-remote projects with different roots in the same os-family must stay 2 entries.
- Teach the consumers about
aliases:commands/projects.md(step 3) should also count observations fromhomunculus/projects/{alias}/;_eod-gather.shshould register aliases in itsregistry[]map (for (const a of p.aliases||[]) registry[a] = {name:p.name, root:p.root}). - Document the known asymmetry in a code comment: if remote detection fails on one machine (git not on PATH for
execFileSync), that sighting falls back to aname:crossKey and won't match theremote:entry — duplicate persists.
Good direction — with the cross-OS restriction and the migration pass this becomes a clean merge. Windows Git Bash compatibility is non-negotiable in this repo (it's where most users hit bugs).
|
Hola Luis, ¿pudiste echarle un ojo a este PR? Lo llevo corriendo en local desde principios de junio (Mac y PC Windows con el registry compartido por Nextcloud) sin ningún problema, y la regresión de upstream me salió verde (install 21/21, registry-isolation 4/4, eod-gather 8/8). Si le ves algo raro o prefieres otro enfoque para el aliasing de roots, me dices y lo ajusto. Sin prisa 🙂 |
Luispitik
left a comment
There was a problem hiding this comment.
The cross-OS diagnosis is correct and the hermetic test harness (real hook, sandbox HOME, no mocks) is exactly how this repo tests — thank you. One design flaw blocks the merge as-is:
The name-key fallback merges two genuinely different projects — and not only across machines. crossKey = "name:" + projectName matches ANY other no-remote project with the same folder name, including on the same machine: ~/work/app and ~/personal/app (both no remote) get different ids (roots differ), but the second sighting finds the first entry via crossKey === "name:app" and collapses them — aliases them together, overwrites root/roots.posix, and from then on /projects, /eod and cross-project search treat two projects as one. Your Notes section flags the cross-machine variant as theoretical; the same-machine variant is the common case (demo, test, scratch, api…), and a false merge is worse than the duplicate it replaces — a duplicate is cosmetic, a merge corrupts the registry.
Suggested fix — restrict the name-key match to the actual cross-OS scenario: only accept a crossKey name match when the candidate root's OS family is NOT already recorded on the entry, e.g.:
|| (crossKey !== "name:" ? registry.projects.find(p => p && p.crossKey === crossKey
&& !(p.roots && p.roots[osFamily(projectRoot)])
&& osFamily(p.root || "") !== osFamily(projectRoot)) : null)That still collapses the Mac+Windows sighting you're targeting (families differ), but two same-family same-name projects stay separate. Please add the regression test: two same-named no-remote projects on the SAME OS remain 2 entries.
Also needed after the fix:
- Rebase onto v4.6.2 — main now routes registry reads through a BOM-safe
readJson()in_session-learner.sh(#25); your hunk sits right below it. - Register
tests/test-crossos-registry.shin.github/workflows/tests.yml— the suite list is explicit, new suites don't run otherwise. - Minor:
osFamilyclassifies Git-Bash/c/...as windows butobserve_v3.pynormally emitsC:/...cwds — fine, just add one test line covering the/c/branch since you handle it.
Happy to merge once the name-key guard is in.
Critical fixes: - Luispitik#1-3: install.sh preserves user data on upgrade (instincts, rules, projects) - Luispitik#4/5A: execSync → execFileSync in 3 hooks (command injection prevention) - Luispitik#5: auto-promote works — drafts track occurrences without injecting - Luispitik#6: race condition — instinct-activator checks dream lock before write Security fixes: - #5B: +4 secret scrubbing patterns (JWT, GitHub, AWS, PEM) in observe_v3.py - #5C/Luispitik#12: ReDoS protection — reject nested quantifiers before RegExp - #5D: chmod 600 on data files during install - #5E/Luispitik#7: fcntl.flock() on JSONL append and rotation - #5F: inject sanitization — 500 char limit + blocked prompt injection patterns Bug fixes: - Luispitik#8: token catalog corrected (9,995 vs 2,700 declared) - Luispitik#9: install.bat synced to v4.3.1 with missing hooks - Luispitik#10-11: instinct-status + passive-status use actual schemas - Luispitik#13: Jaccard Unicode support (\p{L}\p{N}) - Luispitik#14: contradiction detection — \bdo\b scoped to actionable verbs - Luispitik#15: session-end documents relationship with /eod - Luispitik#16: tmpdir flag cleanup (>24h stale files) - Luispitik#17: session-learner selects most recent project by mtime - Luispitik#18: operator-state schema validation flag - Luispitik#19: KNOWLEDGE_FILE dead code removed - Luispitik#20: synapis → sinapsis directory naming + catalog IDs - Luispitik#22: SINAPSIS_DEBUG=1 redirects stderr to debug log Tests: 51/51 GREEN (15 install + 11 security + 25 dream cycle) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
The per-project id is sha256(remote || root). A project with no git remote
therefore gets a DIFFERENT id on each OS (its root differs: /Users/me/Proj
vs C:/Users/Me/Proj), so with a shared registry (e.g. a synced folder) the
same project appears twice — duplicated in /projects, /eod and cross-project
instinct search.
Match on a cross-OS-stable key instead: the git remote when present, else
the project name. The two sightings collapse into one entry — each per-OS id
kept in aliases[], each observed root under roots.{posix,windows}. Projects
with a remote already shared one id and are unaffected; the remote key beats
the name so two distinct same-named projects never merge.
Purely additive: aliases, roots and crossKey are optional fields ignored by
readers that don't use them.
New tests/test-crossos-registry.sh (5 hermetic tests). Existing suites pass:
test-install-upgrade 21/21, test-registry-isolation 4/4, test-eod-gather 8/8.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
… (review) Addresses both review rounds on Luispitik#22: - Name-key fallback now matches ONLY in the genuine cross-OS scenario: the candidate entry's root must belong to a different OS family and the entry must have no root of the sighting's family recorded. Two same-named no-remote projects on the same machine (~/work/app vs ~/personal/app) stay separate — a false merge corrupts the registry where a duplicate is merely cosmetic. - Post-upsert migration pass cures PRE-EXISTING duplicates (the synced-via- Nextcloud symptom motivating the PR): same-crossKey entries merge under the same family guard, union of aliases/roots, oldest created, newest last_seen; crossKey backfilled for legacy entries. - Consumers learn aliases: _eod-gather.sh maps aliased per-OS ids to the merged project; /projects counts observations from alias directories. - Known asymmetry documented in a comment: a sighting whose remote detection fails falls back to a name: key and won't match the remote: entry until the key upgrade heals it. - Tests: suite now 10 hermetic cases — adds same-name same-family anti-regression (both roots intact), pre-existing duplicate migration (merge + union semantics), and the Git Bash /c/ osFamily branch. MSYS_NO_PATHCONV / MSYS2_ARG_CONV_EXCL exported so posix fixtures survive Git Bash; the id-fallback clause in the alias assertion removed. Suite registered in .github/workflows/tests.yml. - Rebased onto v4.8.1 (readJson BOM-safe base). All affected suites green: crossos-registry 10/10, install-upgrade 21/21, registry-isolation, eod-gather, bom-decay-python, v45-opus47, v46-opus48. Co-Authored-By: Claude Fable 5 <[email protected]>
8d5c38d to
c4316cd
Compare
|
Hecho todo lo de la review, Luis:
La suite queda en 10 casos, 10/10 en local (Linux). Re-corridas las afectadas, todas en verde: install-upgrade 21/21, registry-isolation, eod-gather, bom-decay-python, v45-opus47 y v46-opus48. |
The cross-OS suite could not run under Git Bash at all. MSYS path conversion is disabled suite-wide (correctly — posix fixtures must survive argv), but newhome() still returned a posix mktemp path, and node is a native Windows binary that resolves "/tmp/..." to a nonexistent "C:\tmp\...". Every fixture write died with ENOENT. Handing node a native path takes the suite from 0/10 to 10/10 on Windows; no product logic changed. Accept the residual name-key risk, but make it visible. Two unrelated projects that merely share a folder name, seen from different OS families with no remote, still merge — nothing is left to tell them apart. Tightening the guard by requiring a matching parent directory would break the legitimate case the feature exists for (C:/dev/app and /Users/x/code/app are one project at different paths). Instead every name-based fusion now stamps the entry (name_merged + name_merge_log) and appends a line to _session-learner.log, so a wrong fusion is an auditable event rather than silent registry drift. The log deliberately does not go to stderr: the node block is invoked with 2>/dev/null, so stderr writes are discarded — including the pre-existing lock-contention and upsert-failure messages. Also document that osFamily() collapses macOS and Linux into "posix", so mac<->linux duplicates are not de-duplicated by design. Adds 3 regression tests: a name fusion is stamped, both roots are recorded for audit, and a remote-keyed merge is NOT stamped. Suite 13/13, repository 201/201 on Windows. Co-Authored-By: Claude Opus 5 <[email protected]>
|
Gracias, Sergio, y perdona la espera — abriste esto el 2 de junio y te he tenido dos ciclos de review con semanas de por medio. La segunda tanda la resolviste entera y bien: el guard cross-OS en el name-key era exactamente lo que pedía. Lo he verificado por mi cuenta ejecutando el hook real, no leyendo el diff. El caso que me preocupaba queda cerrado: Y el Test 6 que añadiste cubre justo eso. La pasada de migración también hace lo que dice: fusiona duplicados preexistentes con unión de Una cosa que no viste, y es culpa míaEl CI de este PR no ha llegado a ejecutarse nunca. Al venir de un fork y tocar Ya lo he aprobado. Y al correr tu suite en Windows con Git Bash me salió esto: La causa es fina: Nada de esto toca la lógica de producto, que es correcta. Lo comprobé a mano con un HOME de estilo Windows y sale lo que tiene que salir: { "id": "aaaaaaaaaaaa", "crossKey": "name:crossproj",
"roots": { "posix": "/Users/tester/CrossProj",
"windows": "C:/Users/Tester/CrossProj" },
"aliases": ["bbbbbbbbbbbb"] }Si el CI hubiera corrido en junio esto habría saltado en Lo que he añadido encima de tu ramaEn vez de devolvértelo otro ciclo más, lo he cerrado yo. Son 82 líneas en 2 ficheros, y prefiero que les des el visto bueno: 1. h=$(cd "$h" && { pwd -W 2>/dev/null || pwd; })Con eso tu suite pasa 10/10 en Windows sin tocar nada más. 2. El falso merge cross-familia, aceptado pero visible. Tu guard cierra el caso de misma máquina, pero queda uno que no tiene solución limpia: dos proyectos no relacionados que comparten nombre de carpeta ( No he querido endurecer el guard, porque lo obvio —exigir que coincida el directorio padre— rompería el caso legítimo ( Detalle que me costó un rato: el log no puede ir por stderr. El bloque Y otro: no se puede usar comilla simple literal dentro de ese bloque, cierra el string de bash. Ya lo avisa un comentario en la línea 538, pero me lo comí igual. 3. Nota sobre Tests: añadí 3 que fijan el comportamiento nuevo — que la fusión por nombre queda sellada, que ambos roots se registran para auditoría, y que una fusión por remote no se sella (para que el sello signifique algo). Tu suite queda en 13/13 y el repo entero en 201/201 en Windows. Antes de mergearTe dejo 48-72h para que le des un repaso antes de mergear, por si quieres cambiar algo. Sobre todo dos cosas: si el sello Si no dices nada en ese plazo, lo mergeo tal cual y va en la v4.9.0 — que sale con tu feature y el #29 de @juanparisma. Gracias otra vez por la paciencia y por dogfoodearlo dos meses en Mac y PC antes de insistir. Ese tipo de contribución es la que hace que esto funcione. |
|
Gracias por cerrarlo tú, Luis — y por cazar lo del A tus dos preguntas:
Un dato de campo sobre la nota de Y me apunto el otro follow-up que mencionas: el Por mí, mergea cuando quieras. Gracias por el tiempo de las reviews — se nota el cuidado. |
|
Mergeado. Gracias, Sergio. Tus dos respuestas cierran el diseño, y me quedo sobre todo con el argumento del sello: el registry viaja sincronizado entre máquinas y el Antes de que abras el PR del stderr: no lo abrasYa está resuelto. Llegué a la línea 593 por mi cuenta cerrando la v4.9.0, y el fix va en esa misma release:
Perdona por dejarte apuntar el follow-up sin decirte que ya estaba en marcha. Cuando esté mergeado, si al verlo crees que se queda corto, dímelo y lo ampliamos. El límite mac↔linux sí me interesaY tu setup de tres máquinas es mejor banco de pruebas para eso que el mío, así que me viene bien que lo lleves tú. Pero ábrelo como issue antes de escribir código, no como PR directo. Las dos opciones que planteas me dan respeto por motivos distintos. La heurística No digo que no haya solución, digo que la forma es la parte difícil y prefiero que la discutamos antes de que te pongas a escribir. Ya te he hecho dos ciclos de review largos con semanas de por medio; no quiero un tercero. Gracias otra vez, y por los dos meses de dogfooding en Mac y PC antes de insistir. Va en la v4.9.0 junto con el #29 de @juanparisma. |
|
Publicado: v4.9.0. Tu feature abre las notas de release, con tu nombre y lo que hiciste, porque es la mitad de lo que va dentro. Dos cosas para cerrarte los cabos: El follow-up mac↔linux ya está abierto: #32. Lo he escrito yo para que no dependa de que lo abras tú, pero está planteado como diseño, no como patch, y el port es tuyo si lo quieres — dilo ahí y es tuyo. Dentro está tu dato de campo (las tres máquinas, el patch local con las rutas a mano), el argumento de por qué la heurística Tu trampa del Gracias otra vez. |
Problem
The per-project id is
sha256(remote || root). For a project with no git remote, the id is derived from the root — which differs per OS:/Users/me/ProjC:/Users/Me/ProjSo the same project gets a different id on each machine. When the registry (
_sinapsis-projects.json) is shared across machines — e.g. a Nextcloud/Dropbox/iCloud-synced~/.claude— that one project shows up as two entries, duplicating it in/projects,/eod, and cross-project instinct search.Projects with a remote are unaffected (the remote is identical across machines, so the id already matches).
Fix
Match on a cross-OS-stable key instead of the raw id: the git remote when present, otherwise the project name. The two sightings then collapse into a single entry:
aliases[](so future sessions resolve without a re-scan)roots.{posix,windows}The change is purely additive —
aliases,rootsandcrossKeyare new optional fields; existing readers that don't use them are unaffected. Behaviour for remote-backed projects is unchanged.Tests
New
tests/test-crossos-registry.sh— 5 hermetic tests driving the real hook via a sandboxHOME:roots.posixandroots.windowsrecordedExisting suites re-run clean:
test-install-upgrade21/21test-registry-isolation4/4test-eod-gather8/8Notes
~/.claudevia Nextcloud, where non-git folders (the same logical project) appeared twice in/eod. Follow-on to the v4.5.1 cross-OS/eodwork.