From 158ad4d0c68990a655b0001d8aaa66791dd51b3c Mon Sep 17 00:00:00 2001 From: yakamoz221 Date: Thu, 30 Jul 2026 04:55:35 +0300 Subject: [PATCH] docs: record push-permission blocker and clean up stale handoff notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git push to origin failed with 403 (yakamoz221 lacks write access to odysseus-dev/odysseus.git) — recorded in handoff/progress/todos as the new exact-next-step. Also resolved two stale sections left over from before Milestone 3: the "Open decision needed" (feature-flag toggle, answered by the user) and the "agent_loop.py fix is uncommitted" risk note (it's been committed since Session 4). --- docs/handoff.md | 28 +++++++++++++++++++--------- docs/progress.md | 11 +++++++++++ docs/todos.md | 7 ++++++- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/docs/handoff.md b/docs/handoff.md index 05b197c1b..bf6dca5c2 100644 --- a/docs/handoff.md +++ b/docs/handoff.md @@ -13,24 +13,34 @@ For the full checklist see `docs/todos.md`. For the session-by-session history s ## Exact next task -There is no open milestone left on the original plan. Two loose ends, neither blocking: +There is no open milestone left on the original plan. The branch is fully committed but **not pushed** — the last push attempt (`git push -u origin feature/memory-graph-view`) failed: -1. **Known limitation, not fixed**: graph node colors are only recomputed on next render, not live-reactive to a theme flip while the modal is already open (confirmed still true — switching theme via the Theme modal while Memory Graph was open did not immediately recolor node fills, though all non-canvas chrome — detail panel, chips, buttons, the new legend header — is CSS-variable-driven and updates live). Left as-is; fix if it ever actually bothers a user. +``` +remote: Permission to odysseus-dev/odysseus.git denied to yakamoz221. +fatal: unable to access '...': The requested URL returned error: 403 +``` + +The `origin` remote (`https://github.com/odysseus-dev/odysseus.git`) doesn't grant this GitHub account write access. Before this branch can be shared, the user needs to either: grant `yakamoz221` push access to that repo, point `origin` at a fork the account does own, or push some other way (e.g. a different configured remote/credential). This is a permissions/credentials matter outside what a coding session can resolve on its own. + +Two loose ends beyond that, neither blocking: + +1. **Known limitation, not fixed**: graph node colors are only recomputed on next render, not live-reactive to a theme flip while the modal is already open (confirmed still true — switching theme via the Theme modal while Memory Graph was open did not immediately recolor node fills, though all non-canvas chrome — detail panel, chips, buttons, the legend header — is CSS-variable-driven and updates live). Left as-is by deliberate choice at the end of Milestone 3, not an oversight — revisit only if a user actually notices/minds. 2. If compact/mobile icon-rail parity is wanted for the Memory Graph nav item (see Implementation notes below), that's a small standalone follow-up, not scoped into any milestone so far. For a repeatable verification loop in a future session (isolated data dir, seeding via the real API, symmetric Chroma cleanup, PID-verified process kill), the exact steps used are preserved in `docs/progress.md` Sessions 4 and 5 — worth reusing as-is rather than re-deriving. -## Open decision needed from the user +## Resolved decisions (kept for context, not open anymore) -**The "feature flag enabled (beta)" requirement was interpreted as "ship it visible, label it beta" — not as "build an actual on/off toggle."** Right now the only thing gating visibility of the "Memory Graph" nav item is the existing `can_manage_memory` privilege (the same gate the "Brain" button already uses), plus a static "beta" label next to the title in the modal header. There is no separate settings switch a user or admin could flip to hide the feature independent of that privilege, and no server-side flag either. If a real togglable flag was intended (e.g. a Settings-tab checkbox like the design doc's original Phase 2 proposal in `docs/memory-graph-design.md` §8), that hasn't been built. **Confirm which was actually wanted before Milestone 3.** +- **"Feature flag enabled (beta)"**: asked the user directly during Session 5 whether this meant a real togglable on/off switch or the shipped privilege-gate (`can_manage_memory`, same as the "Brain" button) plus a static "beta" label was enough. User confirmed the latter is sufficient — no separate settings switch was built, and none is needed. +- **The `src/agent_loop.py` one-line fix** (a genuinely pre-existing bug — `NameError: name 'Any' is not defined` from a missing `typing` import, confirmed via `git stash` on a clean `dev` checkout, blocking the whole app from starting): kept and committed separately (`fix(agent_loop): add missing typing.Any import`), not folded into any Memory Graph commit. Low-risk enough (adds a missing stdlib import; nothing downstream of that line worked before this fix either) that it didn't need further debate. ## Risks / things to watch -1. **The `src/agent_loop.py` one-line fix is uncommitted and out of this feature's actual scope.** It's a genuinely pre-existing bug (confirmed via `git stash` to reproduce identically on a clean `dev` checkout — `NameError: name 'Any' is not defined` because `dict[str, Any]` was used in a function signature without importing `Any`), and without it the entire app fails to import, which blocks any local verification at all. It's about as safe a fix as they come (adding a missing stdlib import can't break anything that currently works, since nothing downstream of that line currently works). But it wasn't asked for, so **don't fold it silently into a Memory Graph commit** — either commit it separately with a clear message, or ask the user whether they even want it touched at all (they may already have a fix for it elsewhere, or may want to report it upstream themselves). -2. **Do not seed demo/test memory data into the real ChromaDB without cleaning it up.** See step 4 above — this bit us mid-session (server was mid-startup, about to seed, when told to stop; it was killed before anything was written, so no cleanup is currently owed, but the *next* session doing this must not skip the deletion step). +1. **The branch is unpushed** — see "Exact next task" above. Anyone continuing this work locally has everything; anyone expecting to see it on GitHub does not yet. +2. **Do not seed demo/test memory data into the real ChromaDB without cleaning it up.** Sessions 4 and 5 both did this correctly (seed via the real `POST /api/memory/add`, delete via the real `DELETE /api/memory/{id}`, then directly query the shared Chroma collection's `/get` endpoint by id to *confirm* zero vectors remain rather than assuming). Any future session doing this must follow the same symmetric pattern. 3. **The dev venv (`.venv-test/`) is a throwaway, git-ignored (via `.git/info/exclude`, not `.gitignore`) local artifact**, created because this sandbox had no Python environment with the project's dependencies installed. It is NOT part of the repo and should not be referenced by anything committed. A future session may need to recreate it (`python -m venv .venv-test && .venv-test/Scripts/python.exe -m pip install -r requirements.txt`) if it's not still present. -4. **Pre-existing test failures are numerous (199) in this sandbox and are all environment-driven** (missing Node/`rg`, an `mcp` package version mismatch, Windows path-confinement assumptions) — confirmed byte-for-byte identical between this branch and a clean `dev` baseline. Don't waste time trying to fix them as part of this feature; they were already broken before this work started, and fixing them is a separate, unrelated effort. -5. **Chroma collections are global, not owner-scoped.** This was already true of the existing memory system before this feature (`docs/memory-graph-analysis.md` §10) and the graph feature doesn't change it, but it's exactly why step 4 above matters — any manual testing against a real/shared Chroma instance needs symmetric cleanup. +4. **Pre-existing test-collection errors exist in this sandbox and are environment-driven**, not caused by this feature: an `mcp` package version mismatch (`AttributeError: 'Server' object has no attribute 'list_tools'`) affecting 4 test files, plus one unrelated `UnicodeDecodeError` in a document-diff test — confirmed via `pytest --collect-only` at the end of Session 5 to be the exact same 5 errors, nothing new. (Session 2 additionally diffed a full *execution* run against a clean `dev` baseline and found 199 identical pre-existing failures/errors; that full-run comparison wasn't re-done in Sessions 4/5, only the lighter collection-only check — worth a full re-diff if a future session wants that stronger guarantee.) +5. **Chroma collections are global, not owner-scoped.** This was already true of the existing memory system before this feature (`docs/memory-graph-analysis.md` §10) and the graph feature doesn't change it, but it's exactly why point 2 above matters — any manual testing against a real/shared Chroma instance needs symmetric cleanup. ## Implementation notes worth knowing before touching this code @@ -39,6 +49,6 @@ For a repeatable verification loop in a future session (isolated data dir, seedi - **Manual links are stored as a new optional `links: []` field directly on memory JSON entries.** No migration was needed because it's a JSON file, not a SQL schema — old entries without the field are handled via `entry.get("links", [])`/`mem.get("links") or []` throughout, so nothing breaks for pre-existing memories. - **The frontend has zero bundler.** Cytoscape.js is vendored as a single UMD file (`static/lib/cytoscape.min.js`, fetched via `npm pack cytoscape@3` in a scratch dir — not a hand-typed CDN URL) and lazy-loaded via a dynamically-created `