feat: Corpus.default_agent + C8 consumer agent — make a pack's agent wiring installable

[JSv4]

The problem

A domain pack can install a corpus group and a GLOBAL orchestrator. It cannot install the thing that actually answers a user's question: an agent scoped to the corpus holding their documents, carrying both the corpus persona and reach across the group.

That configuration was reachable only by running a script by hand against one database. Anyone installing the pack got the orchestrator-only shape.

Why it wasn't just a missing field

Two independent mechanisms had to change. Neither is obvious from the outside, and the second is the one nobody had noticed:

#	Gate	Location
1	Selection — priority 3 resolves the hardcoded GLOBAL slug default-corpus-agent, so a CORPUS-scoped config is never chosen	_resolve_agent_config
2	Application — if self.agent_config and self.agent_config_id: — fields apply only on the explicit ?agent_id= path	_initialize_agent

Gate 2 means that even a config resolved by fallback had its system_instructions and available_tools silently discarded. Fixing selection alone would have produced a default that resolves and then does nothing.

Both were written when REPLACE was the only mode, and gate 2's own comment gives the reason: a generic default's instructions "would clobber the richer per-corpus persona." That is true of REPLACE and false of EXTEND (#2262), which appends. So the relaxation is narrow:

if self.agent_config and (
    self.agent_config_id
    or self.agent_config.system_instructions_mode == "EXTEND"
):

REPLACE keeps its original fallback-path behaviour. The existing test asserting that (test_default_agent_keeps_factory_prompt_and_tools) is unchanged and still passes — I only made its mode explicit and added the EXTEND counterpart.

Design notes worth reviewing

Why an explicit FK, not "find the corpus-scoped agent". Corpora already carry scoped agents for unrelated purposes — corpus_mutations.py creates an inline moderator. Choosing positionally among them would silently hand corpus chat to whichever sorted first.

Why Corpus.save validates. default_agent is the fallback, so a pointer at an agent scoped to a different corpus would serve that corpus's private instructions to this one's users. Checked in save() rather than as a DB constraint because the condition spans two tables.

Why EXTEND is required for consumer_agent, not merely preferred. REPLACE would overwrite the consuming corpus's persona with text shipped by a third party — precisely the coupling DOMAIN_PACKS.md forbids. Requiring EXTEND is what makes the feature compatible with that prohibition rather than an exception to it.

Why the split. The pack authors the instructions because the group slug they must name is the pack's own invention. The operator names the corpus because which corpus consumes a domain is not knowable when the pack is written. Supplying one without the other is a mistake in both directions, and each gets its own diagnostic — declaring consumer_agent without --consumer-corpus reports that it was not applied (C5) rather than passing silently.

Companion PR

The domain.yaml schema, validator and contract text land in the registry: Open-Source-Legal/authority-packs#8. This PR is the platform half; that one is the schema half. Neither is useful alone.

⚠️ Verification status — please read

Lint and mypy pass against a freshly-resolved pre-commit env (pre-commit clean first, since a cached env is not the env CI builds).

I did not run pytest locally. This machine's postgres credentials are mismatched — django authenticates as doadmin while the running container was initialised with oc_user — and running test.yml risks evicting a live 46-hour stack through the shared compose project name. I judged that not worth the risk on someone else's machine. CI is the verification for the 11 new tests, and I'd rather say so than imply coverage I didn't observe.

New tests:

• test_corpus_default_agent.py — the guard (3 cases: GLOBAL ok, same-corpus ok, other-corpus refused) and the preference (3 cases: unset falls back, corpus default wins, inactive falls back)
• test_unified_agent_consumer.py — EXTEND applies on the fallback path as extra_system_context (not system_prompt, which would consume the "caller supplied nothing" signal and discard the persona — the Corpus/document agent instructions are silently discarded: temporal grounding consumes the system_prompt is None signal #2247 bug)
• test_domain_pack_install.py — 8 C8 cases: binds, unbound-is-reported, corpus-without-spec errors, REPLACE refused, missing group slug refused, ungrantable tool refused, idempotent, missing file refused

🤖 Generated with Claude Code

[claude]

Review: Corpus.default_agent + C8 consumer agent

Solid piece of work — the two-gate root-cause analysis (selection and application both silently dropping a corpus-scoped default) is exactly right, the Corpus.save() cross-corpus guard is well-reasoned, and the EXTEND-vs-REPLACE distinction is applied consistently with #2247's fix. Comments below are in order of importance.

1. --check doesn't validate (or even mention) the new consumer_agent/C8 contract

handle() runs _preflight() for both --check and a real install, on the stated principle that "everything decidable from the FILES is decided before anything is written, for --check and for a real install alike" (install_domain_pack.py:267-269). All of the C8 checks in _wire_consumer_agent (mode must be EXTEND, instructions_file must exist/be non-empty, tools must be grantable, search_across_corpora requires the group slug in the instructions text) are file-decidable — none of them depend on --consumer-corpus. But they only run inside _wire, which handle() never calls under --check (it returns right after _report_plan at line ~285-291).

Net effect: manage.py install_domain_pack foo --check reports "0 violations, no changes written" even when consumer_agent.mode: REPLACE, or the instructions_file is missing, or a declared tool isn't grantable, or the instructions never mention the group slug — all of which will hard-fail (CommandError) on the real install. _report_plan doesn't mention consumer_agent at all, so a pack author gets zero signal from --check about this half of the pack. This is the same asymmetry the codebase explicitly guards against for C1–C7 ("a preflight that only runs under --check is a preflight the install path never gets") — here it's the mirror problem, and it's untested (no test in the diff exercises --check together with consumer_agent).

Worth moving the file-decidable subset of the C8 checks into _preflight (or a helper it calls) so --check actually previews C8 the way it previews C1-C7.

2. @pytest.mark.asyncio on CorpusDefaultAgentResolutionTestCase is inconsistent with the codebase (and a no-op)

opencontractserver/tests/test_corpus_default_agent.py:559 decorates the class with @pytest.mark.asyncio, but pytest-asyncio isn't in requirements/*.txt — every other async TestCase/TransactionTestCase in this repo (including the sibling test_unified_agent_consumer.py, test_agent_factory.py, test_websocket_auth.py, etc.) relies on Django's native async-test-method support and carries no such marker. Since the mark isn't registered and --strict-markers isn't set, it's silently ignored today, but it's misleading (implies a dependency that isn't there) and would start failing collection if --strict-markers is ever turned on. Suggest dropping it for consistency.

3. Model-level default_agent guard checks scope, not visibility — fine today, worth flagging for the future

Corpus.save()'s new guard (corpuses/models.py:477-501) only checks that a CORPUS-scoped default_agent belongs to this corpus; it doesn't check is_public/visible_to_user the way CorpusGroup.default_agent's GraphQL mutation path does (corpus_groups.py::_resolve_default_agent). That's not a live issue — there's no GraphQL mutation exposing Corpus.default_agent yet, only the trusted install_domain_pack management-command path writes it — but if a Corpus.default_agent mutation is added later (mirroring the group one), it should route through the same visibility-checked resolution rather than relying on the model guard alone, since a corpus editor could otherwise point default_agent at a private GLOBAL agent they don't own.

4. Minor: extra query in _corpus_default_agent()

config/websocket/consumers/unified_agent_conversation.py:404-420 issues its own Corpus.objects.filter(pk=self.corpus_id).select_related("default_agent").first() rather than reusing self.corpus, which is already fetched earlier in the connect flow (line ~167). One extra query per corpus-chat session when no explicit agent_id is passed — not a correctness issue, just worth noting if this path is ever perf-sensitive. (It may be intentional, to keep _resolve_agent_config testable without requiring self.corpus to be populated — the tests do construct a bare UnifiedAgentConsumer() with only corpus_id set.)

Nice touches

• The mypy.ini addition documents why (manager introspection limitation) rather than just silencing the error, and even notes why the sibling CorpusGroup.default_agent doesn't need it — good trail for the next person.
• _wire_consumer_agent's split diagnostics (declared-but-unbound vs. bound-but-undeclared) match the PR description's stated intent well, and are both tested.
• test_extend_default_agent_is_applied_on_the_fallback_path correctly asserts extra_system_context (not system_prompt) is used, directly guarding against the Corpus/document agent instructions are silently discarded: temporal grounding consumes the system_prompt is None signal #2247 regression class.

Test coverage

Good breadth on the new behavior (guard, resolution priority, inactive fallback, all 8 C8 install cases). The one gap is #1 above — --check + consumer_agent.

[codecov]

Codecov Report

❌ Patch coverage is 97.95918% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...orpuses/management/commands/install_domain_pack.py	97.40%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[claude]

Review

Solid, well-scoped feature. The two-gate diagnosis in the PR description (selection vs. application) is correct, and the EXTEND-only relaxation is the right fix: it does not touch REPLACE behavior on the fallback path, which is exactly the guard the original comment was protecting. The Corpus.save() cross-corpus leak guard, the permission ordering (corpus READ is checked in _validate_resource_permissions before _resolve_agent_config runs, so the new _corpus_default_agent() query needs no extra visibility check of its own), and the install_domain_pack C8 split (pack owns text, operator owns binding, each half reports its own diagnostic) all check out. The consumer-agent wiring mirrors the existing orchestrator wiring block almost exactly (slug validation, search_across_corpora/group-slug check, GRANTABLE_TOOLS gate) -- good, since it is a variant of an already-reviewed pattern rather than a new one.

Bug: dead code in _corpus_default_agent

In config/websocket/consumers/unified_agent_conversation.py, at the end of the new _corpus_default_agent method:

return await database_sync_to_async(_lookup)()

return None

The second "return None" is unreachable. It looks like a leftover from moving the trailing return out of _resolve_agent_config when this helper was split out. Harmless at runtime, but it is dead code (CLAUDE.md flags this explicitly) and reads as a mistake to the next person touching the file. Worth a quick follow-up deletion.

Minor / non-blocking observations:

• The getattr(self.agent_config, "system_instructions_mode", None) fallback in _initialize_agent is deliberate (some call sites still build agent_config as a partial SimpleNamespace in tests), and the second-commit fix is exactly right -- turning an AttributeError regression into a getattr default rather than a broader try/except.
• install_domain_pack._wire_consumer_agent: when mode != "EXTEND", the method returns immediately without validating instructions_file/tools/group-slug-in-instructions. A pack with both a bad mode and an ungrantable tool only ever reports the mode error on a given run. Not a correctness bug -- _wire is transaction.atomic, so nothing partial is ever persisted -- just a minor diagnostics-completeness gap versus accumulating all unmet violations in one pass.
• Corpus.default_agent is not yet exposed through GraphQL (no config/graphql/ or schema.graphql changes here). Looks like a deliberate scope decision -- the field is set via install_domain_pack today -- but it means there is currently no UI/API path to point an existing corpus at a different default agent without the management command or a shell. Worth confirming that is intended as a follow-up rather than a gap.
• The Corpus.save() guard adds one extra query whenever default_agent_id is set, i.e. on essentially every save once a default is assigned. Right tradeoff for the leak prevention it buys, and explicitly called out in the comment -- just flagging as expected, not a regression.

Test coverage:

The 11 new tests cover the right matrix: the FK guard (GLOBAL ok / same-corpus ok / other-corpus refused), the resolution preference (unset falls back / corpus default wins / inactive falls back), and the 8 C8 install-command paths (bind, unbound-reported, corpus-without-spec, REPLACE-refused, missing-slug-reference refused, ungrantable-tool refused, idempotent, missing-file refused). The second commit moving the resolution tests to a TransactionTestCase-backed base (because the consumer resolves the corpus on its own connection) is the correct diagnosis for a TestCase-wrapped-transaction visibility problem, not a workaround.

One gap: no test exercises Corpus.save()s "agent is None" branch (a default_agent_id pointing at a since-deleted row). Low priority given on_delete=SET_NULL makes it hard to construct organically, but it is the one branch in the new guard without direct coverage.

Overall: looks ready to merge modulo deleting the dead return None.

[claude]

Review

Solid, well-reasoned change — the writeup on the two independent gates (selection vs. application) is accurate and the test coverage for the guard/resolution/install-contract paths is thorough. Two findings from reading the diff closely, plus a couple of minor nits.

1. consumer_agent.preferred_llm skips the C3-style validation its sibling path documents (medium)

opencontractserver/corpuses/management/commands/install_domain_pack.py, in _wire_consumer_agent (~line 933-934):

if spec.get("preferred_llm"):
    consumer.preferred_llm = str(spec["preferred_llm"])
consumer.save()

The orchestrator path in the same file (~line 544-561) pre-validates preferred_llm with validate_model_spec specifically because AgentConfiguration.save() raises Django's ValidationError, which "is NOT a CommandError and so is caught nowhere." There's even a regression test for it: test_unusable_preferred_llm_fails_before_any_write, whose docstring calls this out as "the one remaining path that produced a stack trace instead of a diagnosis."

The consumer-agent path reintroduces exactly that path: a domain pack declaring an invalid consumer_agent.preferred_llm (e.g. not-a-registered-provider:nope) will hit consumer.save() (or get_or_create, which also calls save()) and raise a bare, uncaught ValidationError instead of accumulating into unmet and failing cleanly with a C8-prefixed CommandError. Because _wire is @transaction.atomic, the DB state still rolls back correctly — this isn't a data-integrity bug — but it's an inconsistent, worse error-handling experience for exactly the failure mode the orchestrator code was written to avoid. There's also no test for it on the consumer path (only test_consumer_agent_preferred_llm_is_threaded, which uses a valid value).

Suggest mirroring the orchestrator's validate_model_spec call before assigning consumer.preferred_llm, appending to unmet on failure.

2. Unreachable return None in _corpus_default_agent (nit — dead code)

config/websocket/consumers/unified_agent_conversation.py, end of the new _corpus_default_agent method:

        return await database_sync_to_async(_lookup)()

        return None

The trailing return None is unreachable (the method already always returns via the line above). Harmless at runtime, but flake8/pyflakes won't catch this particular shape, and CLAUDE.md calls out "no dead code" — worth deleting before merge.

Other notes (no action needed, just flagging what I checked)

• The gate-2 relaxation (self.agent_config_id or ... == "EXTEND") is correctly scoped: REPLACE keeps the old fallback behavior, EXTEND rides extra_system_context rather than system_prompt, avoiding the Corpus/document agent instructions are silently discarded: temporal grounding consumes the system_prompt is None signal #2247 regression. Confirmed by test_extend_default_agent_is_applied_on_the_fallback_path.
• Corpus.save()'s cross-corpus guard for default_agent looks correct, including the not-yet-persisted-corpus edge case (self.pk is None for a brand-new corpus, so a CORPUS-scoped agent pointing elsewhere is correctly refused) and the nonexistent-pk case (test_pointer_at_a_nonexistent_agent_is_refused).
• In _wire_consumer_agent, the corpus = Corpus.objects.filter(pk=corpus_pk).first(); if corpus is None: raise CommandError(...) check runs after the instructions/tools validation but raises immediately rather than joining unmet. If both a bad --consumer-corpus and a manifest content problem are present simultaneously, only the "does not exist" error surfaces instead of the aggregated report. Very low severity (the two problems can't really compound in normal use — a bad corpus pk is an operator-CLI mistake, a bad manifest is a pack-author mistake) but worth a glance if you want every C8 violation reported in one pass like the orchestrator's C1-C4 checks are.
• Migration (0060_corpus_default_agent.py), mypy.ini addition, and the changelog fragment all look correct and consistent with existing conventions in this file/repo.

Nice test naming and comments throughout — the "why" is unusually well documented for a Django management command.

[claude]

Review

Overall this is a careful, well-documented change: the two-gate diagnosis (selection vs. application) is correct, the Corpus.save() cross-corpus guard closes a real leak vector, and the C8 consumer-agent contract is properly shared between --check and the real install path so the preview can't lie. Test coverage is thorough, including the "unreachable but kept for defense-in-depth" case and the codecov-flagged guard paths.

Policy issue (please fix before merge)

CLAUDE.md's Baseline Commit Rule #3 is violated: "Never credit Claude or Claude Code in commit messages, PR messages, comments, or any other artifacts. This includes Co-Authored-By lines, 'Generated by Claude', etc."

• The PR description ends with 🤖 Generated with [Claude Code](https://claude.com/claude-code).
• Several commits carry Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> and list "Claude Opus 5" as a commit author.

Please strip the attribution line from the PR body and, if feasible, the co-author trailers from the commit messages (or squash/reword on merge) to comply with the repo's own rule.

Correctness

• config/websocket/consumers/unified_agent_conversation.py — the relaxed fallback-path gate (self.agent_config_id or system_instructions_mode == "EXTEND") plus the getattr(..., None) defensiveness is correct and well-justified by the second commit's postmortem (SimpleNamespace test doubles that don't carry the field). The EXTEND path correctly routes through extra_system_context rather than system_prompt, avoiding the Corpus/document agent instructions are silently discarded: temporal grounding consumes the system_prompt is None signal #2247 regression.
• Corpus.save()'s new guard correctly handles all values of AgentConfiguration.scope (only GLOBAL/CORPUS exist, no third case to miss) and catches a stale/nonexistent default_agent_id set without loading the row.
• install_domain_pack.py's _consumer_agent_violations being shared between _preflight and _wire_consumer_agent is the right move — it's what makes --check trustworthy for C8, matching the existing C1-C7 guarantee.

Minor observations (non-blocking)

1. Silent overwrite of an existing default_agent. _wire_consumer_agent unconditionally does corpus.default_agent = consumer; corpus.save(...) with no check for (and no warning about) a corpus that already has a different default_agent set — e.g. from a previously installed domain pack, or set by hand. Given the C5 "declared but not bound" case already gets an explicit warning, it might be worth a similar heads-up here (stdout.write a WARNING when corpus.default_agent_id is non-null and about to change) so an operator running a second domain pack against the same corpus doesn't silently lose the first one's binding.

2. Corpus.default_agent isn't exposed anywhere outside the ORM/CLI. CorpusType in config/graphql/corpus_types.py doesn't resolve this new field (the existing default_agent resolver there is on CorpusGroupType, for the unrelated CorpusGroup.default_agent), and it's not registered in corpuses/admin.py either. That may well be intentional/out-of-scope for this PR (the companion authority-packs#8 PR is the schema half), but as it stands the only way to inspect or change a corpus's default agent post-install is direct DB/ORM access. Worth a tracking note if a GraphQL/admin surface is expected later.

3. Extra query on every Corpus.save() once default_agent_id is set. The new validation block in Corpus.save() re-queries AgentConfiguration on every save of a corpus that has a default_agent, not just when that field changes (mirrors the existing preferred_llm validation pattern, so it's consistent with prior art, but worth flagging if corpus saves are ever on a hot path).

4. Nit: _wire_consumer_agent's get_or_create only sets is_public via defaults=, so re-running the install with a different --public value won't update is_public on an already-existing consumer agent (the orchestrator's get_or_create at line ~805 has the identical characteristic, so this is consistent with existing behavior rather than a new gap).

Test coverage

Good breadth — guard tests (same-corpus/other-corpus/nonexistent), resolution-priority tests correctly moved to a TransactionTestCase-backed base once the "connection already closed" issue was diagnosed, and all 8 C8 install-path branches plus their --check preview counterparts. Nothing missing that I'd block on.