Skip to content

docs: capture push-arc learnings in docs/solutions#1166

Merged
marcusrbrown merged 1 commit into
mainfrom
docs/compound-push-arc-learnings
Jul 11, 2026
Merged

docs: capture push-arc learnings in docs/solutions#1166
marcusrbrown merged 1 commit into
mainfrom
docs/compound-push-arc-learnings

Conversation

@marcusrbrown

Copy link
Copy Markdown
Collaborator

Captures four learnings from the operator push notification feature as docs/solutions/ entries, so the next time these traps appear they take minutes instead of a review cycle.

  • logic-errors/node-24-happy-eyeballs-... — Node 20+ enables autoSelectFamily (Happy Eyeballs) by default, so a custom https.Agent DNS-lookup SSRF guard must return an array and classify every candidate. A guard that called back a single address broke every send with Invalid IP address: undefined; the tests used the {family: 0} shape the real runtime never takes, so they stayed green while it was broken.
  • workflow-issues/gh-auth-login-refuses-to-persist-...gh auth login --with-token refuses to store credentials when GH_TOKEN is in the environment, which silently stopped the CI review bot from posting reviews repo-wide. The CI job stayed green because the job succeeds even when the review never posts.
  • best-practices/broadcast-notifications-when-operator-identity-is-absent — when a repo-neutral background notification fires from an event seam that carries no per-recipient identity, broadcast to all subscribers rather than plumbing identity through shared state. The payload being content-free is what makes broadcast safe.
  • workflow-issues/rebase-without-bun-install-stale-dist (updated) — folds in that adding a runtime dependency to any workspace package regenerates the workspace-wide dist/THIRD_PARTY_NOTICES.txt and fails the committed-dist gate, even for packages not bundled into dist/.

All cross-links resolve (link checker green). AGENTS.md already surfaces docs/solutions/.

…EN, broadcast targeting, dep-add dist notices)

@fro-bot fro-bot left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: PASS

Documentation-only PR capturing four post-incident learnings as docs/solutions/ entries. I verified every technical claim against the actual codebase — the docs are accurate, not just plausible.

Blocking issues

None

Non-blocking concerns

None. Spot-checks all confirmed the docs match reality:

  • Happy Eyeballs SSRFcreateGuardedLookup in packages/gateway/src/web/operator-push/push-sender.ts resolves with all: true and classifies every candidate via isBlockedResolvedAddress, matching the doc's before/after and the "check every candidate" claim.
  • gh-auth GH_TOKEN — the snippet matches src/services/setup/gh-auth.ts:54-57 (delete loginEnv.GH_TOKEN/GITHUB_TOKEN scoped to the login exec only; harness keeps process.env.GH_TOKEN).
  • Broadcast targetingdispatcher.ts lists all active subscriptions and fans out a neutral payload, reaping dead records per-send, as described.
  • Workspace-wide THIRD_PARTY_NOTICESscripts/third-party-notices.ts:181-182 confirms collection from ALL workspace packages, so the "adding a dep to any package regenerates the notices file" claim holds.

All four cross-links resolve to existing files (authenticated-sse-run-observation, gateway-control-surface-spine, same-job-phase-split-not-a-security-boundary, isolate-ci-credential-via-oidc-broker). Frontmatter (including last_updated on the modified entry) follows the established convention used across docs/solutions/.

Missing tests

None — documentation-only change with no executable behavior. The link checker (reported green) is the appropriate gate.

Risk assessment (LOW): No source, config, or workflow behavior changes. Zero regression surface, no security exposure, blast radius limited to Markdown under docs/solutions/. The captured learnings themselves are high-value (a silent SSRF-guard break and a repo-wide silent review-bot outage), which improves future incident response.


Run Summary
Field Value
Event pull_request
Repository fro-bot/agent
Run ID 29132793315
Cache hit
Session ses_0b1682eaaffepHc4RYACDIYloF

@marcusrbrown marcusrbrown merged commit 2c2be89 into main Jul 11, 2026
12 checks passed
@marcusrbrown marcusrbrown deleted the docs/compound-push-arc-learnings branch July 11, 2026 00:38
@fro-bot fro-bot mentioned this pull request Jul 11, 2026
76 tasks
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.

2 participants