Skip to content

fix(watch): write manifest.json to the target's graphify-out (#2316) - #2345

Open
Rishet11 wants to merge 2 commits into
Graphify-Labs:v8from
Rishet11:fix/2316-update-manifest-target-dir
Open

fix(watch): write manifest.json to the target's graphify-out (#2316)#2345
Rishet11 wants to merge 2 commits into
Graphify-Labs:v8from
Rishet11:fix/2316-update-manifest-target-dir

Conversation

@Rishet11

Copy link
Copy Markdown
Contributor

Closes #2316

graphify update <target> wrote manifest.json into the process CWD's graphify-out/ while every other artifact went to the target. Reproduced on v8 4fe1109 (0.9.31), and the reproduction turned up two more defects in the same function that the report does not cover. All three are fixed here; graphify/watch.py is the only file touched.

Root cause

_rebuild_code computes the correct output dir at watch.py:897:

out = watch_path / _GRAPHIFY_OUT

and routes every artifact through it, including one line above each manifest write (out / ".graphify_root", out / "needs_update"). The three save_manifest(...) calls omitted manifest_path=, so they fell back to detect._MANIFEST_PATH (detect.py:29) — a module-import-time constant built from the relative out_path("manifest.json") (paths.py:288), resolved against the process CWD. cli.py:2815 is the reference-correct wiring and passes it explicitly at 2853/3404/3510/3657.

Three defects

1. Location — as reported. The reporter's line numbers (970/1009/1087, from 0.9.13) have drifted to 1232/1275/1421 at 4fe1109.

2. It is data loss on the invoking project, not just a stray file. save_manifest read-modify-writes the same path (detect.py:1615) and full-scan callers pass scan_corpus (#1908 pruning), so the invoking project's own rows are pruned as out-of-corpus and overwritten:

$ cd projA && graphify update .
$ cat projA/graphify-out/manifest.json
{"a_file.py": {...}}

$ graphify update /tmp/repro/projB          # from projA
$ cat projA/graphify-out/manifest.json
{"b_file.py": {...}}                        # a_file.py destroyed

Both projects then need a full rescan, not only the target.

3. A relative target stored absolute keys. root= followed project_root, which is CWD-derived for a relative invocation, so graphify update ../projB put the target's files out-of-root and _to_relative_for_storage kept them absolute — breaking the #777/#1964 portability guarantee even once the file is in the right directory:

$ cd projA && graphify update ../projB
{"/private/tmp/repro/projB/b_file.py": {...}}     # not "b_file.py"

Fixed by passing root=watch_root at those three calls. project_root itself is deliberately left alone — the persisted graph rehomes source_file against it when the user switches between absolute and relative invocation styles (tests/test_watch.py:1389, :1428). The manifest and the persisted graph are different artifacts with legitimately different anchors; the manifest's consumer is detect_incremental(root=target).

4. built_at_commit came from the wrong repo. _git_head() ran git rev-parse HEAD with no cwd=, so it read the caller's repository:

projB HEAD:          66dc5e6d2fab2ca4e623a1a70ee4a4ea67828388
projA HEAD:          011fc671d2336d3c56c2df5376243096bc70ed46
projB graph.json:    "built_at_commit": "011fc671..."     # projA's

This is #2081 in the watch path — wrong provenance data rather than a misplaced file. One cwd= parameter.

Before / after, the reporter's exact repro

before after
projA/graphify-out/ manifest.json, containing projB's files not created at all
projA pre-existing manifest overwritten, own rows pruned byte-identical
projB/graphify-out/manifest.json absent present, key b_file.py
relative-target key /private/tmp/.../projB/b_file.py b_file.py
projB built_at_commit projA's HEAD projB's HEAD

Verification

  • tests/test_watch_manifest_location.py, 7 tests, all 7 red at 4fe1109 and green with the fix (verified by reverting only watch.py). They cover all three write sites: clustered, --no-cluster, and the unchanged-topology early return. The early-return test asserts on the "No code-graph topology changes detected" line so it cannot silently drift onto another branch and keep passing.
  • Full suite -p no:randomly: base 4 failed / 3895 passed / 3 skipped4 failed / 3902 passed / 3 skipped, failure-set diff empty. The 4 are pre-existing backend-detection tests that read real env vars.
  • ruff check . clean. tools.skillgen --check (134 artifacts), --audit-coverage, --schema-singleton all clean.

Known gaps, stated openly

  • The three call sites remain wrapped in bare except Exception: pass, so a manifest write that fails for any other reason is still silent. The reporter raises this; it is a different change and I left it alone rather than widen the diff. Happy to add a log line here if you want it.
  • tools/skillgen/fragments/references/shared/update.md:163 has the same missing manifest_path in the agent runbook (save_manifest(..., root='INPUT_PATH')). Same bug class, different artifact, and it needs regenerated skill markdown — not folded in here.
  • export.py:173 has its own _git_head() with the same no-cwd= shape, used by the extract path. Untouched.
  • _report_root_label (watch.py:268) falls back to Path.cwd().name for a relative path. Cosmetic label only, left as is.

Rishet11 added 2 commits July 31, 2026 14:42
…y-Labs#2316)

_rebuild_code computed the correct output dir as `out = watch_path /
_GRAPHIFY_OUT` and routed every artifact through it except the three
save_manifest calls, which omitted manifest_path= and fell back to
detect._MANIFEST_PATH — a module-import-time constant built from the
relative out_path("graphify-out/manifest.json"), so it resolved against
the process CWD. `graphify update <target>` therefore wrote the target's
manifest into the invoking project and left the target with none.

save_manifest also read-modify-writes that path and full-scan callers
pass scan_corpus (Graphify-Labs#1908 pruning), so the invoking project's own rows were
pruned as out-of-corpus and overwritten. Both projects lost their
incremental baseline, not just the target.

Anchor the manifest keys to watch_root at the same three calls. root=
previously followed project_root, which is CWD-derived for a relative
invocation, so `graphify update ../other` stored out-of-root files under
absolute keys and broke Graphify-Labs#777/Graphify-Labs#1964 portability. project_root itself is
left alone: the persisted graph deliberately rehomes source_file against
it across invocation styles.

Also pass cwd= to _git_head. Without it `git rev-parse HEAD` inherited
the caller's directory, stamping the invoking repo's commit into the
target's graph.json built_at_commit (cf. Graphify-Labs#2081).
…raphify-Labs#2316)

Seven tests, all red on 4fe1109 and green with the fix:

- location, parametrized over the clustered and --no-cluster write sites
- the unchanged-topology early return, the third write site; asserts the
  "No code-graph topology changes detected" line so the test cannot
  silently drift onto a different branch and keep passing
- the invoking project's own manifest survives an update of another
  project (the data-loss half)
- a relative target stores relative keys
- the resulting manifest is actually consumable by detect_incremental
- built_at_commit matches the target repo's HEAD, not the caller's
Copilot AI review requested due to automatic review settings July 31, 2026 09:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).


Graphify review — findings

This PR changes _rebuild_code in graphify/watch.py so that manifest writes and git-HEAD lookups are anchored to the watch/target directory rather than the process CWD: the three save_manifest calls now pass an explicit manifest_path and root=watch_root, and _git_head gains a cwd parameter that is passed the target's watch_root. It also adds a new test file tests/test_watch_manifest_location.py covering where the manifest lands, non-destruction of the CWD project's manifest, relative-key portability, and the commit stamped into the target graph. The surface area is the manifest/provenance handling for graphify update <target> when invoked from a different directory. Reviewers should confirm the intended anchoring behavior across the clustered, --no-cluster, and unchanged-topology early-return paths.

No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 283 functions depend on the 77 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: _rebuild_code() — 62 callers, 48 callees

Verification — 283 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 157 function(s) in the blast radius were not formally verified this run

· 1 more finding(s) on lines outside this diff (see the check run).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

update writes manifest.json to the CWD instead of the target — cross-project contamination, and the target gets no manifest

2 participants