T055 follow-up: confine to the resolved entry, no second lookup (default_registry.resolve_source)

[brettheap]

What this is

A small follow-up to T055 (#59, landed as fa140875). It takes the one finding Copilot's review of #59 at 0c946f4e raised as "previously missed", after the last push that could take it.

The finding, quoted

Copilot review overview on #59 at 0c946f4e (review 5368674478), medium, "Avoid unstable second lookup during source path confinement", src/opendox/default_registry.py:444:

resolve_source() performs a second lookup by (repository, ref) after the caller has already resolved an entry. A concurrent refresh can replace that key between the two operations, so the returned path can be confined under a different entry's source_root than the entry whose metadata/listed paths the caller is using (for example, serve_project._resolved_listed_edit_entry). Expose an entry-based confinement operation or make the caller pass the resolved entry so lookup, validation, and path confinement use one stable entry.

It is real. #59 already closed the same pattern in serve.py's /source arm (Copilot r4136585695 and r4136863569): that arm resolves the entry once and confines to that entry's own root with resolve_source_path(Path(root), rest). serve_project._resolved_listed_edit_entry was the one caller left.

What changed

• src/opendox/serve_project.py: _resolved_listed_edit_entry resolves the entry ONCE and confines the path to THAT entry's own root through the registry seam's declared resolve_within (read as projection_seams.registry.current() inside the function). The listed-path check already read the entry in hand, and the editor is launched over entry.source_root, so the lookup, the validation and the confinement are now one entry. An entry with no root serves nothing, as before.
• The caller needs no method the seam does not declare. resolve_source is on no seam's list (REGISTRY_CALLABLES), so a contributed registry is never asked for one. That is why the fix confines the entry in hand rather than adding an entry-based method to openDox's own registry: a host's registry would not carry it.
• src/opendox/default_registry.py: SnapshotRegistry.resolve_source keeps its behaviour (one lookup, confined to that entry). Its docstring now says it is for a caller that holds only a pair, and why a caller that already holds an entry must not ask again by its pair.
• tests/test_edit_action_one_entry.py (new, 8 cases).

No module-level proxy is bound in serve_project.py: tests/test_projection_seams.py::test_no_proxy_over_a_seam_is_read_at_import_time pins the exact set of modules that bind one (serve.py, serve_workbench.py), and this PR does not edit that file.

Every caller of the two-step path

git grep resolve_source -- src at fa140875 finds the definition and exactly one production caller, serve_project.py:111. The other registry lookups in src/ are one resolution each: serve.py _serve_snapshot and _serve_source (already one entry), serve_workbench.py (resolve then resolve_within(entry.source_root, ...), three sites), and branch_session.py (stamping an entry after a register, no confinement). _keyed_source's registry.get(*parsed) is a parse-time existence check whose result is a key, and _serve_source then resolves that key once.

The new test test_no_module_asks_a_registry_for_a_path_by_a_pair holds that set empty, so a future caller has to be argued for.

Evidence

Red at main, green after. The new module against fa140875's sources (serve_project.py and default_registry.py as on main):

FAILED test_the_edit_arm_asks_the_registry_once_and_never_for_a_path_by_a_pair
FAILED test_no_module_asks_a_registry_for_a_path_by_a_pair
FAILED test_a_host_registry_needs_only_what_the_seam_declares
FAILED test_a_refresh_that_replaces_the_key_cannot_take_the_file_the_route_refuses
FAILED test_a_refresh_that_replaces_the_key_cannot_take_the_file_the_route_accepts
5 failed, 3 passed

With this PR: 8 passed. The three that pass at main are the control (a listed file opens with no refresh), confinement kept, and "no root serves nothing", which pin what must NOT change.

The race is tested at the ROUTE, both ways, with a real server, a real POST /actions/edit and the console token. A registry whose key is replaced right after the route's first resolve (as a refresh on another thread would) lands the replacement between the resolution and the confinement (asserted):

• Entry's root has NO file, the replacement's root has it. At main the route took the file from the replacement's root, read the first entry's listing, and started the editor over the first entry's root: 200 and an editor over a file that is not there. Now: 404 document_unavailable, no editor.
• Entry's root has the file, the replacement's root has none. At main the route refused a file its own entry holds (404). Now: 200, and the editor is started over that entry's root.

Mutants of the fix, all killed (the new module only, serve_project.py restored after each):

mutant	killed by
M1 the second lookup again (registry.resolve_source(repository, ref, path), i.e. main)	one-lookup count, the no-module-asks scan, the host registry, both race cases
M2 confine by Path(root) / path, no containment rule	test_the_entrys_own_root_still_confines_what_the_route_accepts
M3 the no-root check dropped	host-registry and no-root cases
M4 the listed-path check dropped	the confinement case (an unlisted file inside the root)
M5 the listing read through a second lookup	the one-lookup count (the race cases cannot see it, as both entries share a snapshot)

T056's module against this fix. Fetched #66's head 38761c76 read-only into a scratch worktree, merged main (fa140875; the two add/add conflicts, default_registry.py and tests/test_projection_seams.py, resolved to main's blobs, as T056's own diff does not touch either), cherry-picked this commit on top, and ran tests/test_standalone_generate_path.py (its children run from that worktree's src via PYTHONPATH):

tests/test_standalone_generate_path.py + tests/test_edit_action_one_entry.py: 14 passed
control, this commit reverted: tests/test_standalone_generate_path.py: 6 passed

That module's requests are GET /source/notes-toolshed-inventory.md (200, byte-equal) and GET /source/.git/config (404). They go through serve.py's /source arm (resolve_source_path), which #59 already moved off resolve_source, not through serve_project, so this PR leaves them as they were. The module passes identically with and without this commit.

Whole suite at 5666505b, LANG=C.UTF-8, run in the foreground with a throwaway postgres:16 and OPENDOX_TEST_DATABASE_URL set as CI sets it:

python -m pytest -q tests          2379 passed, 11 skipped
python -m pytest -q tests_runtime   607 passed
total                              2986 passed, 11 skipped, 0 failed

#59's CI at 0c946f4e was selected=2989 passed=2978 skipped=11. This PR adds 8 cases: 2997 selected, 2986 passed, 11 skipped.

Overlap with open PRs

None. gh pr diff --name-only on #60 to #69, read against each PR's own merge-base (#66 and #68 carry #59's commits, which lists default_registry.py spuriously): their own diffs touch neither serve_project.py nor default_registry.py. #66's own serve.py change is one flushed print near line 2216, outside the /source arm.

Review rounds

• Copilot at 0471f8c7: "Approval recommended", no findings, 0 threads. The SonarCloud quality gate failed there on 4.9% duplication on new code (required at most 3%): the new test module carried a copy of test_projection_seams.py's autouse isolation fixture and git helper.
• 5666505b: imports both instead of copying them (the tests/test_doxbench_*.py precedent), a test-only change. The autouse fixture still applies to all eight cases (eight SETUPs under --setup-show), the eight cases pass, and M1 to M5 are still killed.
• Copilot at 5666505b: "Approval recommended", no findings, 0 threads. validate and SonarCloud ("Quality Gate passed") green at 5666505b.

Arc: neutral-product-standalone-operability
Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

🤖 Generated with Claude Code

[sourcery-ai]

Reviewer's Guide

The PR removes the remaining unstable second registry lookup from the project edit route by confining paths through the entry returned by its single resolution, while retaining existing listing and containment rules. It documents the intended scope of resolve_source and adds focused tests covering races, security checks, and contributed registry compatibility.

Sequence diagram for single-entry project edit path resolution

sequenceDiagram
    participant Client
    participant EditRoute as serve_project
    participant Registry
    participant Seam as projection_seams.registry
    participant Entry
    participant Editor

    Client->>EditRoute: POST /actions/edit
    EditRoute->>Registry: resolve(repository, ref)
    Registry-->>EditRoute: Entry
    EditRoute->>Entry: read listed source paths
    EditRoute->>Seam: resolve_within(Path(Entry.source_root), path)
    Seam-->>EditRoute: confined target
    alt target is a listed file
        EditRoute->>Editor: launch editor over Entry.source_root
        EditRoute-->>Client: 200
    else target unavailable or unlisted
        EditRoute-->>Client: 404 document_unavailable
    end

Loading

Sequence diagram for refresh-safe source confinement

sequenceDiagram
    participant EditRoute as serve_project
    participant Registry
    participant Seam as projection_seams.registry
    participant FirstEntry as resolved entry
    participant Replacement as refreshed entry

    EditRoute->>Registry: resolve(repository, ref)
    Registry-->>EditRoute: FirstEntry
    Note over Registry,Replacement: A refresh may replace the key after resolution
    EditRoute->>Seam: resolve_within(Path(FirstEntry.source_root), path)
    Seam-->>EditRoute: target confined to FirstEntry
    EditRoute->>FirstEntry: read listed source paths
    EditRoute-->>Replacement: no lookup by repository and ref

Loading

File-Level Changes

Change	Details	Files
Confine edit requests using the already-resolved registry entry instead of performing a second (repository, ref) lookup.	Resolve the entry once and reject entries without a source root. Use the projection seam's declared resolve_within(Path(entry.source_root), path) for confinement. Keep listed-path validation tied to the same entry and avoid requiring contributed registries to implement resolve_source. Clarify SnapshotRegistry.resolve_source as a pair-based API for callers that do not already hold an entry.	src/opendox/serve_project.py src/opendox/default_registry.py
Add regression coverage for stable-entry behavior, confinement, host registry compatibility, and refresh races.	Verify the edit route performs one lookup and no module calls resolve_source by pair. Exercise both refresh race directions at the real edit route, including editor launch behavior. Preserve root confinement, listed-path checks, no-root handling, and seam-only host requirements.	tests/test_edit_action_one_entry.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[Copilot]

Copilot review overview

🟢 Approval recommended

The focused change correctly removes the second lookup and is comprehensively covered.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes a registry-refresh race by confining edit paths to the already-resolved snapshot entry.

Changes:

• Uses one stable registry entry throughout edit validation.
• Documents resolve_source’s pair-only contract.
• Adds focused confinement and race coverage.

File	Description
src/​opendox/​serve_project.py	Confines edits to the resolved entry’s root.
src/​opendox/​default_registry.py	Clarifies lookup semantics.
tests/​test_edit_action_one_entry.py	Tests single lookup, confinement, host seams, and refresh races.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

Copilot review overview

🟢 Approval recommended

The implementation closes the identified race while preserving confinement and is thoroughly covered by focused tests.

Review effort: Balanced
Findings: None

[brettheap]

READY at 5666505 — Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

[sourcery-ai]

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 1 day and 20 hours by commenting @sourcery-ai review. Upgrade to get a review now.

[brettheap]

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

LANDED — lane openxfactory-4, 2026-09-30T18:19:52Z, PR #70 → 75bd870 (opensoft/openDox-code main; plain gate)

Brett: land the phase 1 PRs when green