Skip to content

docs(ADR-004): conformance pass — three divergences, and a field that never existed - #825

Merged
lilyshen0722 merged 1 commit into
mainfrom
docs/adr-004-conformance-pass
Aug 4, 2026
Merged

docs(ADR-004): conformance pass — three divergences, and a field that never existed#825
lilyshen0722 merged 1 commit into
mainfrom
docs/adr-004-conformance-pass

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

ADR-004 was frozen on 2026-04-14 and, as far as the git history shows, has not been read against the code since. Four seats found three separate divergences in one afternoon, independently, while working on unrelated PRs. Three findings in one day is a fact about the document, not about the code — and every driver author is being taught the divergences as fact.

C1 — createdBy is not a field, and installedBy cannot become one

docs/adr/ADR-004  ×3   Auth · invariant 5 · install lifecycle step 2
docs/adr/ADR-006  ×6   including its own audit claim
models/AgentRegistry.ts   grep -c createdBy → 0

Not a typo — a term of art that spread between two ADRs while never existing in the schema. The field is installedBy. ADR-006 is the one an external webhook-driver author reads first, so it's corrected here too.

The naming is the small half. On agent-initiated installs the value is the agent's User id:

agentsRuntime.ts:2593 · :2650 · :2740      installedBy: agentUser._id
agentAutoJoinService.ts:80                 installedBy: agentUser._id

Every other write site stores a human. So "every agent action traces back to a human" is false for self-installed agents, and invariant 5 does not hold.

The obvious fix is wrong, which is why this is documented rather than patched. installedBy is also a live authorization predicate:

// controllers/reactionController.ts:50-55 — gates whether an agent may react
AgentInstallation.findOne({ podId, installedBy: req.agentUser._id, status: 'active' })

That only matches rows where installedBy is the calling agent. Rewriting the four write sites to store a human would silently drop every agent through this gate to its Pod.members fallback. The field carries two incompatible meanings — who authorized this and whose row is this — and one live gate depends on the second. Restoring the invariant needs a separate field, not a repurposing. Filed as a design question.

C2 — attempts was a frozen 0 (fixed in #822)

Recorded with the new semantics — counts deliveries, incremented at the pending → delivered claim — plus the note that invariant 8's "the event re-delivers" is now bounded by a 3-attempt cap rather than cycling until the retention delete.

C3 — "re-deliver on next poll" is a 10–20 minute sweep

Still open. schedulerService.ts:151 runs */10 against a 10-minute threshold, so effective latency is uniform over [T, T+P). Against a spec that guides drivers to "3–10s for interactive agents" that is a 60–400× divergence, and it's the one a driver author would most reasonably design against. Closing it means a lease or short-TTL claim — a design change that wants its own PR, not a quiet behaviour edit.

Why the markers are inline

Each divergent bullet carries a ⚠️ and a link; the section carries the detail. A conformance block at the bottom is invisible to a reader who jumps to ### Auth or greps for attempts — which is exactly how ADR-012's rolled-back heartbeat cue survived three months with its own correction already written forty lines further down (#818). Applying that lesson to this document was the whole design of the change.

The section also carries its own maintenance rule: when a divergence closes, edit the bullet and leave the entry — the inline ⚠️ must not outlive the defect.

One thing this PR did to itself

C1 originally read "### Auth and invariant 5 both name createdBy." The grep run to verify that citation found a third instance in the same file a minute later, and the sweep across sibling ADRs found six more in ADR-006. A cardinality word inside a correction is the same defect the correction is about. That's recorded in C1, because the one-second probe that fixes it — grep the identifier across every document, not the sections you were reading — is the transferable part.

Not verified: whether any agent-initiated install has actually landed somewhere its human owner didn't intend — no DB read, so C1 is a reachable-state finding, not an observed one. I did not audit ADR-001/003/005 for other phantom field names beyond the createdBy sweep. C3's arithmetic is from the two constants; I have not measured a real redelivery interval end-to-end.

🤖 Generated with Claude Code

… never existed

ADR-004 was frozen 2026-04-14 and has not been read against the code since.
Four seats found three separate divergences in one afternoon, independently,
while working on unrelated PRs. Three findings in one day is a fact about the
document, not about the code.

C1 — `createdBy` is not a field. ADR-004 named it three times (Auth,
invariant 5, install lifecycle step 2) and ADR-006 six more times including
its own audit claim. `grep -c createdBy models/AgentRegistry.ts` → 0. The
field is `installedBy`. Not a typo: a term of art that spread between
documents while never existing in the schema.

The naming is the small half. On agent-initiated installs the value is the
AGENT's User id (agentsRuntime.ts:2593/:2650/:2740,
agentAutoJoinService.ts:80), so "every agent action traces back to a human"
is false there and invariant 5 does not hold.

And the obvious fix is wrong, which is the part worth recording.
`installedBy` is also a live authorization predicate —
reactionController.ts:50-55 gates agent reactions on
`findOne({podId, installedBy: req.agentUser._id})`, which only matches rows
where the field IS the calling agent. Rewriting the four write sites to store
a human would silently drop every agent to its Pod.members fallback. The
field carries two incompatible meanings and one gate depends on the second;
restoring the invariant needs a separate field. Filed, not fixed.

C2 — `attempts` was a frozen 0 until today. Fixed in #822; recorded here with
the new semantics (counts deliveries, incremented at the claim) and the fact
that invariant 8's "re-delivers" is now bounded by a 3-attempt cap.

C3 — "re-deliver on next poll" is a 10-20 minute server-side sweep. Cron
`*/10` against a 10-min threshold gives [T, T+P); against a spec that guides
drivers to 3-10s that is a 60-400x divergence. Still open, wants a lease
design rather than a quiet behaviour edit.

Markers are INLINE at each divergent bullet, not only in the section. A
conformance block at the bottom is invisible to a reader who jumps to
`### Auth` or greps for `attempts` — which is exactly how ADR-012's
rolled-back heartbeat cue survived three months with its correction already
written forty lines below it (PR #818).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lilyshen0722
lilyshen0722 merged commit 8af9581 into main Aug 4, 2026
10 checks passed
@lilyshen0722
lilyshen0722 deleted the docs/adr-004-conformance-pass branch August 4, 2026 20:23
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.

1 participant