fix(witan-code): stop four tests asserting the machine's logged-out state

mcp/servers/witan-code/CHANGELOG.md

@@ -6,6 +6,39 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/) (pre-1.0:
 a MINOR bump may include breaking changes).
 
+## [Unreleased]
+
+### Fixed
+
+- **The test suite no longer asserts an environment.** Four tests
+  (`test_branches.py`'s three branch-view assertions and
+  `test_graph.py::test_a_shared_branch_view_needs_an_identity_to_own_it`)
+  failed on any machine whose `witan login` had resolved an actor, and passed
+  in CI, where a GitHub runner has none. They asserted un-namespaced view
+  names and the logged-out refusal prose — the shape identity resolution
+  happens to take when there is no identity — because nothing stopped
+  `identity.actor_id()` from reading the real `~/.config/witan/config.toml`
+  and OIDC token cache.
+
+  The `_fresh_identity` fixture now points `WITAN_CONFIG` and
+  `WITAN_TOKEN_CACHE` at `tmp_path` and clears the four env vars that
+  short-circuit ahead of them, so logged-out is deterministic everywhere. A
+  new `logged_in_actor` fixture opts back in through `WITAN_ACTOR`, and
+  `test_a_logged_in_writer_owns_the_views_it_indexes` uses it to cover the
+  half of `witan_code.views` a CI runner cannot reach: on a logged-in machine
+  the views carry their owner, on the local store too.
+
+### Changed
+
+- **`graph.check_writable` requires `actor`; `None` now means "no identity",
+  full stop.** It used to fall back to `identity.actor_id()`, which gave the
+  parameter two meanings depending on the caller. Every call site
+  (`indexer`, `bridge`, `ingest`) already resolves an actor — it needs one to
+  *name* the view it is about to write — so nothing relied on the fallback,
+  and for `ingest` it was wrong: a request arriving with no actor was judged
+  against the serving process's identity rather than being refused for having
+  none.
+
 ## [0.15.0] - 2026-08-21
 
 ### Changed

mcp/servers/witan-code/tests/conftest.py

@@ -41,21 +41,63 @@ def main():
 """
 
 
+# Every input `identity._resolve` reads. Isolating the file paths is not
+# enough on its own: each of these env vars short-circuits ahead of them.
+_IDENTITY_ENV = (
+    "WITAN_ACTOR",
+    "WITAN_REMOTE_URL",
+    "WITAN_OIDC_ISSUER",
+    "WITAN_TARGET",
+)
+
+
 @pytest.fixture(autouse=True)
-def _fresh_identity():
-    """Forget the process-lifetime actor between tests.
+def _fresh_identity(tmp_path, monkeypatch):
+    """Pin the identity these tests run under: nobody, unless one is asked for.
 
+    Two jobs. The first is to forget the process-lifetime actor between tests —
     ``identity.actor_id`` memoizes deliberately (a witan-code process writes as
     exactly one identity), which without this would let the first test that
     resolves one decide the branch-view names for every test after it.
+
+    The second is to stop that resolution reaching the machine. It reads the
+    real ``~/.config/witan/config.toml`` and the real OIDC token cache, so on a
+    developer's box — logged in, unlike a CI runner — ``actor_id()`` returns an
+    ``act-…`` and the branch views get namespaced under it. Four tests asserted
+    un-namespaced names and failed for everyone who had run ``witan login``,
+    green in CI the whole time. Pointing both at ``tmp_path`` makes logged-out
+    the deterministic default everywhere; ``logged_in_actor`` opts back in.
     """
     from witan_code import identity
 
+    monkeypatch.setenv("WITAN_CONFIG", str(tmp_path / "witan-config.toml"))
+    monkeypatch.setenv("WITAN_TOKEN_CACHE", str(tmp_path / "witan-tokens.json"))
+    for var in _IDENTITY_ENV:
+        monkeypatch.delenv(var, raising=False)
+
     identity.reset_cache()
     yield
     identity.reset_cache()
 
 
+@pytest.fixture
+def logged_in_actor(monkeypatch):
+    """Run the test as a specific actor — the half of the guard CI never sees.
+
+    Set through ``WITAN_ACTOR`` rather than by stubbing ``actor_id``, so the
+    resolution path a non-interactive writer (the CI indexer, a maintenance
+    job) actually takes is the one under test.
+    """
+    from witan_code import identity
+
+    def _login(actor: str = "act-alice") -> str:
+        monkeypatch.setenv(identity.ACTOR_ENV_VAR, actor)
+        identity.reset_cache()
+        return actor
+
+    return _login
+
+
 @pytest.fixture(autouse=True)
 def _fresh_git_context():
     """Forget the memoized git context between tests.

mcp/servers/witan-code/tests/test_branches.py

@@ -155,6 +155,51 @@ def test_feature_branch_indexes_to_own_branch(tmp_path, monkeypatch):
     assert not on_main, "main view must not see in-flight branch symbols"
 
 
+@requires_stack
+def test_a_logged_in_writer_owns_the_views_it_indexes(
+    tmp_path, monkeypatch, logged_in_actor
+):
+    """The other half of `witan_code.views`, which CI never reaches.
+
+    A GitHub runner has no witan identity, so every branch-view assertion above
+    is of the un-namespaced name. On a logged-in machine — every real user —
+    the views carry their owner, on the local store too (there is no second
+    naming rule for local stores). Asserting it here is what keeps the two
+    halves from drifting.
+    """
+    from witan_code import config as cfg_module
+    from witan_code import indexer
+    from witan_code.graph import OmnigraphClient
+    from witan_code.views import owner
+
+    actor = logged_in_actor("act-alice")
+    monkeypatch.setenv("WITAN_REPO", REPO)
+    monkeypatch.setenv("WITAN_CODE_DIR", str(tmp_path / "code"))
+    cfg = cfg_module.load()
+
+    base = _git_repo(tmp_path / "r")
+    (base / "svc.py").write_text(SAMPLE)
+    indexer.index_path(base, config=cfg)
+
+    _git(base, "checkout", "-q", "-b", "feature/new-api")
+    (base / "extra.py").write_text("def branch_only_symbol():\n    return 2\n")
+    indexer.index_path(base, config=cfg)
+
+    view = f"{actor}/feature_new-api"
+    store = str(cfg_module.store_path(REPO, cfg.code_dir))
+    main_client = OmnigraphClient(store, cfg.queries_dir)
+    assert view in main_client.list_branches()
+    assert "feature_new-api" not in main_client.list_branches(), (
+        "the un-namespaced name is the collision this scheme replaced"
+    )
+    assert owner(view) == actor
+
+    branch_client = OmnigraphClient(store, cfg.queries_dir, branch=view)
+    assert branch_client.read(
+        "code_read.gq", "find_by_name", {"name": "branch_only_symbol"}
+    )
+
+
 @requires_stack
 def test_branch_index_writes_to_bridge_overlay_not_main(tmp_path, monkeypatch):
     """A non-default branch's bridge writes land on its repo-qualified

mcp/servers/witan-code/tests/test_graph.py

@@ -118,6 +118,23 @@ def test_a_shared_branch_view_needs_an_identity_to_own_it():
         _check(remote=True, branch="feature-x", actor=None)
 
 
+def test_no_actor_means_logged_out_even_when_the_machine_has_one(logged_in_actor):
+    """`actor=None` is "nobody owns this write", never "go ask the process".
+
+    The guard used to fall back to `identity.actor_id()` here, which made the
+    refusal depend on whether whoever ran it had done a `witan login` — the
+    test above asserted the logged-out prose and CI, with no identity, was the
+    only place it held. It also meant a request arriving with no actor
+    (:mod:`witan_code.ingest`) was judged against the *server's* identity.
+    """
+    logged_in_actor("act-the-machine")
+
+    with pytest.raises(SharedGraphWriteRefused) as excinfo:
+        _check(remote=True, branch="feature-x", actor=None)
+    assert "witan login" in str(excinfo.value)
+    assert "act-the-machine" not in str(excinfo.value)
+
+
 def test_local_branch_views_need_no_actor():
     """A local store has one user, who owns every view in it — unchanged
     names, no migration, no login required to index offline."""

mcp/servers/witan-code/witan_code/graph.py

@@ -72,21 +72,25 @@ def check_writable(
     branch: str | None,
     cfg: cfg_module.Config,
     slug: str,
-    actor: str | None = None,
+    actor: str | None,
 ) -> None:
     """Raise :class:`SharedGraphWriteRefused` unless :func:`owns_view` allows it.
 
-    ``actor`` is the identity this process writes as; it defaults to the
-    resolved one, so a caller that does not construct view names itself does
-    not have to thread it through.
+    ``actor`` is the identity the write is being made as, and ``None`` means
+    exactly one thing: there is no identity to own the view. Required, with no
+    fallback to :func:`~witan_code.identity.actor_id` — every caller already
+    resolves an actor, because it needs one to *name* the view it is about to
+    write, and a caller serving somebody else's write resolves theirs, not this
+    process's (:mod:`witan_code.ingest`). A default would make ``None`` mean
+    "logged out" or "ask the machine" depending on who was calling, and the
+    second reading is the one that cannot be right here.
 
     ``is_remote`` is "is this graph shared", which for a client is a property
     of its store (``client.is_remote``) and for the MCP tier serving somebody
     else's write is true by construction — that is the only reason the request
     exists (:mod:`witan_code.ingest`). Taking the bit rather than the client is
     what lets both ask the same question.
     """
-    actor = actor if actor is not None else identity_module.actor_id()
     if owns_view(is_remote=is_remote, branch=branch, cfg=cfg, actor=actor):
         return
     if branch is None: