[ENG-3568] Use one credential for Pulse ingest and block-logs - #154
Merged
Conversation
apiKey and pulseAuth have always held the same value: there is one oauth_clients row per site, so there is one secret. Two fields isolated nothing — a bad credential broke both paths regardless — while obliging every write site to keep them in step. That obligation already caused a bug. `login` rotated the credential and wrote only pulseAuth, leaving apiKey holding a value the server had just invalidated, which would have stopped block-log delivery silently. Stops writing pulseAuth. Nothing else changes: Pulse resolution already falls back to apiKey in both the CLI (config.ts) and the guard (runtime.js), so a config carrying only apiKey authenticates both paths. The one hazard is a stale copy. pulseAuth resolves ahead of apiKey, so a config written by the previous version would keep authenticating Pulse with the replaced value after a rotation. persistApiKey now drops the field when it writes, which is correct in every case: a new apiKey invalidates any older pulseAuth by definition. Reading is unchanged. An existing pulseAuth is still honoured and PATCHSTACK_PULSE_AUTH still overrides, for deployments that do want Pulse ingest on a different credential from block-logs. persistPulseAuth stays exported for them. Four tests pin the compatibility surface: apiKey-only resolves for Pulse, an existing pulseAuth still wins, a stale one is dropped on rotation, and the env override still applies. Co-Authored-By: Claude <noreply@anthropic.com>
|
Unified credential logic simplifies config and removes redundant pulseAuth handling. 🎯 Quality: 100% Elite · 📦 Size: Medium 📈 This month: Your 59th PR — above team average · Averaging Excellent |
Contributor
Author
|
/review |
daniloradovic
approved these changes
Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
apiKeyandpulseAuthhave always held the same value — there is oneoauth_clientsrow per site, so there is one secret. Two fields isolated nothing (a bad credential breaks both paths regardless) while obliging every write site to keep them in step.That obligation already produced a bug:
loginrotated the credential and wrote onlypulseAuth, leavingapiKeyholding a value the server had just invalidated — block-log delivery would have stopped silently.No breaking changes
Reading is completely unchanged. Pulse resolution already falls back to
apiKeyin both paths:config.ts:83—fromEnv.pulseAuth ?? fromFile.pulseAuth ?? apiKeyRawruntime.js:694,704—resolvePulseAuth()→resolveApiKey()So every existing config keeps working, in every combination:
apiKeyonly (pre-ADR-0018 sites)apiKey+pulseAuth(current version)pulseAuthstill winsPATCHSTACK_PULSE_AUTHsetapiKeyonlypersistPulseAuthstays exported — removing it would break the package's public API — and is documented for deployments that genuinely want Pulse on a different credential from block-logs.The one hazard, and the fix
pulseAuthresolves ahead ofapiKey. So a config written by the current version would, after a rotation that wrote onlyapiKey, keep authenticating Pulse with the replaced value.persistApiKeynow drops the field when it writes. That is correct in every case: a newapiKeyinvalidates any olderpulseAuthby definition. Without it this change would have introduced exactly the class of bug it exists to remove.Tests
Four new tests pin the compatibility surface:
apiKeyalone resolves for PulsepulseAuthstill winsPATCHSTACK_PULSE_AUTHstill overrides1224 pass (up 4), typecheck clean. README and AGENT-INSTALL updated to describe one credential, with the
pulseAuthescape hatch noted as something an agent should not add by default.Ref ENG-3568
🤖 Generated with Claude Code