Skip to content

lore: lore/implementation-loop/issue-1624 - #1688

Open
lore-agent[bot] wants to merge 5 commits into
mainfrom
lore/implementation-loop/issue-1624
Open

lore: lore/implementation-loop/issue-1624#1688
lore-agent[bot] wants to merge 5 commits into
mainfrom
lore/implementation-loop/issue-1624

Conversation

@lore-agent

@lore-agent lore-agent Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

The cluster-agent was choosing which credential to use on every call via
process.env.LORE_INGEST_TOKEN ?? agentToken. That per-call fallback is what
caused the 2026-08-24 outage: a central-cluster agent that had LORE_INGEST_TOKEN
mounted would correctly pick it up most of the time, but any satellite that
somehow acquired the variable after boot would shadow its own per-agent token
and 401 on every report — both ends typechecked, nothing in the logs until the
reaper cleaned up the stuck nodes.

The fix is in apps/cluster-agent/src/claim/select-reporter-token.ts. The
function selectReporterToken(env, getAgentToken) reads env.LORE_INGEST_TOKEN
exactly once at call time and makes a permanent decision: if the token is
present the central path captures it in a static closure and returns that same
value forever, so later env mutations are irrelevant; if it is absent the
satellite path returns the getAgentToken thunk directly, so re-registration
rotations continue to be picked up per call. There is no fallback chain, and
the choice cannot change after boot.

apps/cluster-agent/src/index.ts calls selectReporterToken(process.env, () => agentToken) once during startup and binds the result to reporterToken. Both
the ClaimLoop reporter and the TelemetrySink now receive that single
reference instead of inlining the ?? expression independently. Keeping two
separate inlinings was an additional hazard: they could drift.

The three acceptance tests in
apps/cluster-agent/src/claim/select-reporter-token.test.ts cover the three
invariants the DoD names: the central capture survives an env delete after
selection, the satellite thunk reflects rotations made after selection, and a
satellite ignores LORE_INGEST_TOKEN that appears in the env after the choice
was made. All three were written red first, then made green by the implementation.

This is FR5 of specs/running-stations-in-any-k8s-cluster/spec.md. The
acceptedTokens relay route in index.ts (line 120) was deliberately left
unchanged — it accepts both tokens because agent run pods may present either,
and that multi-token accept path is covered by a separate requirement (FR8.1).

No deviation from the DoD strategy.

…er-agent-token

Acceptance tests for selectReporterToken, a boot-time credential
selector that replaces the per-call `LORE_INGEST_TOKEN ?? agentToken`
fallback chain in index.ts. Three tests pin the invariant:

- central cluster: LORE_INGEST_TOKEN is captured at construction, not
  read from env on every call, so a mid-run removal cannot fall back
  to agentToken
- satellite: the agentToken thunk is returned directly, so rotations
  are still picked up per-call
- satellite: LORE_INGEST_TOKEN appearing in the env after the token
  is selected is ignored — boot-time decision, not per-call

The function does not exist yet; all three tests fail with
"Cannot find module './select-reporter-token.js'".

Strategy: parallel-change — see .lore/dod.md for the facets.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

🔍 Lore Spec Impact — advisory

This PR touches 1 statement(s) across 1 spec(s), and 1 of them has validating tests this PR does not change.

Feature Specification: Running Stations in Any Kubernetes Cluster · 1 statement(s)

specs/running-stations-in-any-k8s-cluster/spec.md

FR5 — Reporting credentials for satellites
⚠ the tests that validate it are not touched by this PR

only its test links changed — the statement text itself is unchanged

Satellites report with their per-agent token; LORE_INGEST_TOKEN never leaves the central cluster — and a per-agent to…

validated by libs/shared/src/project/events/event-reporter-http.test.ts:119, apps/event-router/src/delivery/server-auth.test.ts:39

1 new statement(s) have no test link yet.

Deterministic · graph @ b079b26 (projected 2026-08-31) · no tests run by this check

Lore Agent and others added 4 commits August 31, 2026 07:44
…tial at boot

Replaces the two inline `() => process.env.LORE_INGEST_TOKEN ?? agentToken`
thunks (token and TelemetrySink) with a single `reporterToken` selected once
at boot via `selectReporterToken(process.env, () => agentToken)`.

Central clusters capture LORE_INGEST_TOKEN statically; satellites use the
per-agent thunk so re-registration rotations are still visible. Eliminates
the per-call fallback that caused the 2026-08-24 outage.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lore-agent
lore-agent Bot marked this pull request as ready for review August 31, 2026 08:10
@gedaiu

gedaiu commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

@lore review

@lore-agent

lore-agent Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Lore is reviewing this PR — b7652aae-31d7-444d-8bf2-85dbdbab0ad3.

Reply to any review comment to discuss or approve a fix (e.g. "ok, fix it"). Comment @lore review to re-run the review.

@lore-agent

lore-agent Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Inline placement was rejected by GitHub, so this review is posted as a single comment.

Lore review — Approved

Correct, well-tested fix for the 2026-08-24 credential-shadowing outage; only cosmetic convention gaps remain.

Must fix (0) · Consider (0) · Nits (3)

Reply to any review comment to discuss or approve a fix (e.g. "ok, fix it"). Comment @lore review to re-run the review.

specs/running-stations-in-any-k8s-cluster/spec.md:425nit: FR5 documents satellite path only — spec missing central-cluster capture behaviour

- The satellite's reporter RESOLVES that token per call rather than capturing it:
  a re-registration rotates it, and a captured value would 401 every report
  from then on — which is what the watch did silently until the credential
  was wired at all, leaving every node to the reaper instead. A central
  cluster captures LORE_INGEST_TOKEN once at boot instead: a stable credential
  that does not change with re-registrations. Which path is taken is decided
  at process start by `selectReporterToken` and does not change thereafter.

apps/cluster-agent/src/claim/select-reporter-token.ts:1nit: 9-line file-level docblock violates one-line-max convention from CLAUDE.md

// Chooses the reporter credential once at boot — central clusters capture LORE_INGEST_TOKEN, satellites return the per-agent thunk (FR5).

apps/cluster-agent/src/claim/select-reporter-token.test.ts:1nit: 16-line module-level comment block violates one-line-max convention from CLAUDE.md

// FR5 of specs/running-stations-in-any-k8s-cluster: one credential, chosen at boot, not a per-call fallback.

apps/cluster-agent/src/index.ts:120praise: acceptedTokens correctly left on live env reads rather than reporterToken

apps/cluster-agent/src/claim/select-reporter-token.ts:11praise: Pure, zero-dependency function — exactly right for boot-time credential selection

@lore-agent

lore-agent Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Lore implementation-loop run failed (failed — node "dod" failed: BackoffLimitExceeded: Job has reached the specified backoff limit — The pod died rather than the work failing — a crash, an OOM, an eviction, or a Job deadline. Re-running is the right response; check pod events if it repeats.) — e536b555-642f-4765-9b0a-39acbdd9c250.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant