docs(eventing): Phase 2 — make §8's count checkable, and correct one mechanism

[mrsabath]

Phase / scope

Phase	Phase 2 — identity on the event path (DESIGN_PHASE2.md §8)
What this changes	That phase's design record only. No code, no manifests, no tests.
Why a separate PR	These are the three findings from @aslom's #890 review, which approved and merged before they were addressed. Follow-up rather than a reopen.
Phases it does not touch	Phase 0/1 and Phase 3 untouched. No overlap with #883.

Addresses the three findings in the #890 approval. All about §8 rather than the code it
describes, and all verified against the tree.

The count did not reconcile — in §8 of all places

"Thirteen" appears six times in DESIGN_PHASE2.md and twice in agentdocs/README.md,
and §8.1–§8.3 enumerated eleven. Thirteen was reachable only by knowing that two
bullets each carried two findings in one paragraph:

• §8.2's capability-URL entry covered both GET /v0/groups and GET /v0/groups/{id}/status
• §8.3's SPIRE entry covered both the algorithm and the keyset file format

Nothing in the text marked either as two, so a reader counting got 11 every time.

Both are split now, and §8 carries the tally explicitly: five in §8.1 (four rows plus
the fifth, promised nowhere), four in §8.2, four in §8.3.

@aslom's framing is why this was worth more than a wording tweak: §8 exists to argue that
unchecked claims survive review, so a count that does not survive counting is the same
failure in miniature — and it is the first thing a sceptical reader tests.

§8.1's group-event row named the wrong field and the wrong causal step

It said kafka_in.py "keeps final and groupid, so the rewritten event still reaches
on_group_event". Checked against the code:

• A group event carries no final at all — publish_group_event never sets it and
  on_group_event never reads it. final matters for the member-answer row two rows
  down, not this one.
• Routing is on ce.is_group_event(evt), computed on the original event, before and
  regardless of the rewrite. So the rewrite is not why it reaches the handler; type and
  groupid surviving is why the batch then ends.

Corrected to the reviewer's wording. Worth being exact because §8.5.1 is "Which code
path, by name?" and this row is the worked example a reader copies the style from.

One finding had no issue, in the section that says to file the issue

The ntfy group-suppression finding was the only one of the thirteen with no link, and
#887 does not cover it — that one is PUT /transcript and listable correlation ids.
Filed as #891 and linked, with a pointer to §8.5.4 as the general rule it
instantiates ("if the decision reads an attribute off the event, the forger controls that
attribute — including by leaving it out").

While checking that, §8.3's items turned out to have no links either — correctly,
because they are documentation drift rather than code bugs, so the correction is the
fix. §8.3 now says so, since §8.7 tells readers to file issues and an unlinked finding
otherwise reads as an oversight rather than a decision.

Verification

• Every §N.N reference in the file resolves to a heading that exists; the only
  unresolved one is the pre-existing, explicitly qualified "Phase 1 §16".
• The tally was checked by counting the rendered items, not by trusting the prose.
• 575 passing, 5 skipped; ruff check and ruff format --check clean. No code
  changed.

Not included

The fifth suggestion from that review — making render() fall back to kustomize build
so test_manifests.py's assertions run in CI without a cluster — is deliberately out of
scope. It is a change to test infrastructure that affects five assertions across two
phases, and @aslom's own note said it is bigger than a docs PR should carry. Worth its own
issue alongside #885–#891.

🤖 Generated with Claude Code

[aslom]

Summary

The tally this PR adds reconciles, and I checked it by counting rather than reading: §8.1 has four table rows plus the fifth at "A fifth, promised nowhere" (5), §8.2 has four bullets, §8.3 has four — 13. The §8.1 group-event correction is accurate against the code: kafka_in.py:136 routes on ce.is_group_event(evt), the original event, and the rewrite at kafka_in.py:122 is dict(d, phase="error", data={...}), so it replaces only phase/data and type/groupid survive. §8.3's new "no issue links, deliberately" note is also correct — those four bullets carry no links, and every item in §8.1 and §8.2 does carry one. #891 exists and matches the ntfy finding.

Two problems, both in the second commit's new kustomize comment, and both the shape §8 is about — a claim written from the change that introduced it. The warning says dropping the env list would "run the demo in MOCK MODE". It would not: envFrom is a sibling field that an env patch cannot touch, so anthropic-credentials still supplies ANTHROPIC_AUTH_TOKEN, and config.py:127 then auto-detects real mode. The actual loss is HOME/CLAUDE_CONFIG_DIR — the §16 Gap B transcript-durability failure the demo patch's own comment warns about. Separately, the copy of that warning in the eventbridge patch cites three env entries that ../demo only ever adds to eventrunner; ../demo patches eventbridge's volumes/0 and nothing else.

Neither blocks merge — no code, no behavior, CI green — but the mock-mode claim is the one I'd want corrected before this lands, since §8.5.1 is "Which code path, by name?" and this comment is now the counter-example.

Two notes too small for inline comments:

• The PR description says the only unresolved §N.N reference is the qualified "Phase 1 §16". There is a second, equally qualified one: "Phase 1 §1.1" at line 48. Every in-document reference does resolve, so the file is fine — only the description is incomplete.
• Line 450's scope line still reads "§8.1–§8.4 ... thirteen claims", while the new tally localises all thirteen to §8.1–§8.3 (§8.4 is commentary and enumerates nothing). Defensible as a range, slightly odd now that the count is explicit.

Author: mrsabath (MEMBER — maintainer)
Areas reviewed: Docs (DESIGN_PHASE2.md), K8s/kustomize (kind-signed/kustomization.yaml, comments only), plus code verification of the claims in kafka_in.py and config.py
Agent/IDE config (.claude/.vscode): none
Commits: 2 commits, all signed-off: yes
CI status: passing (11/11 — CodeQL, DCO, Trivy, codeql, dependency-review, eventing-test, hadolint, lint, test, test-startup, trivy-scan)

[aslom]

[suggestion] Mock mode is not what this would cause, and the mechanism is worth getting right because §8.5.1 is "Which code path, by name?".

op: add on …/env replaces the env list, but envFrom is a sibling field — a JSON patch on /spec/template/spec/containers/0/env cannot touch it. So in the hypothetical demo-signed, ../demo's envFrom.secretRef: anthropic-credentials (optional: false) survives and still supplies ANTHROPIC_AUTH_TOKEN. config.py then takes the auto-detect branch:

mock_env = (e("ER_MOCK_CLAUDE") or "").strip()
if mock_env: ...
elif has_api_credentials():          # ANTHROPIC_AUTH_TOKEN / ANTHROPIC_API_KEY
    cfg.mock_claude = False
    cfg.mock_reason = "auto: API credential present"

With ER_MOCK_CLAUDE dropped, has_api_credentials() is true, so the runner comes up in real mode, reason auto: API credential present — the opposite of the warning. The demo patch's own comment already says this ("Phase 0's auto-detection would pick real mode on its own once the Secret mounts").

What actually breaks is HOME and CLAUDE_CONFIG_DIR: claude writes session transcripts under $HOME/.claude, so losing them moves transcripts off the mounted volume onto the container's ephemeral layer and a restart loses every resumable session on the pod — the §16 Gap B failure that patch calls out. Quieter than mock mode, which is the better argument for the warning.

Suggested change

	# `../kind`, whose containers have no `env` — but `../demo` has three entries, so
	# a `demo-signed` overlay built by changing only the base would silently drop
	# `ER_MOCK_CLAUDE=false`, `HOME` and `CLAUDE_CONFIG_DIR` and run the demo in MOCK
	# MODE. `kustomize build` succeeds and nothing warns. Whoever writes that overlay
	# wants `op: add` on `env/-` per entry, against a base that already has the key.
	# `../kind`, whose containers have no `env` — but `../demo` has three entries, so
	# a `demo-signed` overlay built by changing only the base would silently drop
	# `ER_MOCK_CLAUDE=false`, `HOME` and `CLAUDE_CONFIG_DIR`. Mock mode is *not* the
	# failure: `envFrom` is a sibling field and survives, so the credential is still
	# present and config.py auto-detects **real** mode ("auto: API credential
	# present"). What is lost is `HOME`/`CLAUDE_CONFIG_DIR`, so claude writes session
	# transcripts to the container's ephemeral layer instead of the mounted volume and
	# a restart loses every resumable session on the pod — the §16 Gap B failure.
	# `kustomize build` succeeds and nothing warns. Whoever writes that overlay wants
	# `op: add` on `env/-` per entry, against a base that already has the key.

[mrsabath]

Both corrections taken in 3e1dfa1, and the mock-mode one was right — I checked it by running config.load() rather than reading it, which is the rule this PR is about:

A. ../demo as it stands (env list present)
    mock_claude = False
    mock_reason = 'explicit ER_MOCK_CLAUDE=false'

B. demo-signed: `op: add` on .../env replaced the list
    mock_claude = False
    mock_reason = 'auto: API credential present'

ER_MOCK_CLAUDE is set in exactly one place in the tree — eventrunner-demo-patch.yaml — so with the env list replaced it is unset, envFrom still carries ANTHROPIC_AUTH_TOKEN, and config.py:127 takes the auto-detect branch. Real mode, reason auto: API credential present. The warning claimed the opposite.

Your eventbridge point is confirmed too: ../demo's eventbridge target is op: replace on volumes/0 and nothing else, so there is no env list to drop there at all. That patch now carries the general rule and points here for the worked example, per your suggestion.

一针见血 (yī zhēn jiàn xiě — "one needle, and blood shows") — the §8.5.1 counter-example was sitting in the same PR that added the checklist. That is the second time on this document that a sentence written from the change that introduced it did not survive being run, which is the finding generalizing rather than the finding being wrong.

Both description notes are also correct and fixed in the file: "Phase 1 §1.1" at line 48 is a second qualified reference I missed, and line 450's scope line now reads §8.1–§8.3, with §8.4 named as commentary that enumerates nothing.

[aslom]

[suggestion] Same mock-mode correction as on the eventrunner patch below, plus one specific to this target: the three env entries cited here are never added to eventbridge.

../demo patches eventbridge only to replace volumes/0:

  - target:
      kind: Deployment
      name: eventbridge
    patch: |-
      - op: replace
        path: /spec/template/spec/volumes/0

ER_MOCK_CLAUDE/HOME/CLAUDE_CONFIG_DIR come from eventrunner-demo-patch.yaml, which targets eventrunner. So for this container a demo-signed overlay would hit exactly the same empty-env case as ../kind — there is nothing to drop. The hazard is real but belongs to the eventrunner patch; stating it here with eventrunner's variable names makes the eventbridge patch look riskier than it is.

Suggest keeping the general rule here and leaving the worked example on the eventrunner patch:

Suggested change

	# `../kind`, whose containers have no `env` — but `../demo` has three entries, so
	# a `demo-signed` overlay built by changing only the base would silently drop
	# `ER_MOCK_CLAUDE=false`, `HOME` and `CLAUDE_CONFIG_DIR` and run the demo in MOCK
	# MODE. `kustomize build` succeeds and nothing warns. Whoever writes that overlay
	# wants `op: add` on `env/-` per entry, against a base that already has the key.
	# `../kind`, whose containers have no `env`. That is a property of this base, not
	# a general guarantee — see the eventrunner patch below, where `../demo` does add
	# an `env` list that a `demo-signed` overlay would silently replace. `kustomize
	# build` succeeds and nothing warns either way. Whoever writes that overlay wants
	# `op: add` on `env/-` per entry, against a base that already has the key.

[mrsabath]

Taken as suggested in 3e1dfa1. Verified your specific point first: ../demo's eventbridge target is

  - target:
      kind: Deployment
      name: eventbridge
    patch: |-
      - op: replace
        path: /spec/template/spec/volumes/0

and nothing else, so there is no env list on this container to drop in a demo-signed overlay — the three variables I cited come from eventrunner-demo-patch.yaml, which targets eventrunner. You are right that naming them here made this patch look riskier than it is.

This one now carries the general rule and points at the eventrunner patch for the worked example; the mock-mode correction is applied there.

[aslom]

[nit] Removing the old ### 8.3 Three things about the keys heading left two consecutive blank lines between the new "No issue links in this subsection" paragraph and the §2.6 bullet. Renders the same; just an artifact of the edit. One blank line here.

[mrsabath]

Fixed in 3e1dfa1 — one blank line. Artifact of removing the old ### 8.3 Three things about the keys heading, as you diagnosed.