Skip to content

Audit staxly for drift after ~2 year deploy gap #119

Description

@TylerZeroMaster

Context

This app hadn't been redeployed to Heroku in roughly 2 years before #118. That deploy activated several years of accumulated dependency drift all at once — 4 separate bugs stacked in src/changelog.js alone (see #118), all invisible because nothing exercised that code path. Worth a broader look for similar rot elsewhere in the repo, tracked here rather than in the bugfix PR.

Known items to look at

  • Orphaned src/utils/ auto-merge cluster: addConnectedPRToIssue.js, getConnectedIssueForPR.js, getConnectedPRsForIssue.js, getPRBlock.js, pipeline.js, prIsReadyForAutoMerge.js, removeConnectedPRFromIssue.js, connectedPRRegexes.js, regexes.js are not imported by any robot.on(...) handler — unreachable from any real webhook event. They're fully unit-tested (100% coverage), which is misleading since nothing in production calls them. Left behind when the "Auto merge" feature was deliberately reverted in 2020 (02f0405/ca6c48c), which only unwired src/merge-bases.js and left the supporting utils + tests in place. Decide: delete, or re-wire if the feature is still wanted.
  • Dead env vars in app.json/.env.example: SECRET_PROJECT_EVENTS_PATH (marked required: true, leftover from Project Automation being extracted to a separate package in 2018 — extract Project Automation to separate package #49) and SENTRY_ENVIRONMENT (also required: true, but nothing reads it — only SENTRY_DSN is actually consumed, by Probot itself). Both just add friction to every deploy for no functional benefit.
  • Slack integration status: src/slack-api.js/src/slack-stuff.js currently have no credentials configured in the Heroku app, so they're inactive. Originally a small utility for cross-posting/reacting to messages; there was also an idea at some point to trigger book builds via a Slack slash command that was never built. Worth deciding whether to keep, finish, or remove rather than leave half-wired.
  • changelog: config staleness: only 2 repos in the org have a changelog: key in .github/config.yml — staxly itself (requires README.md updates for src/ changes) and cnx-press (requires docs/source/changes.rst updates for press/ changes). Neither convention has actually been followed in years (staxly's README.md last touched 2023-09-12). Worth deciding whether to keep enforcing this now that the check works again, update the conventions, or drop the config.
  • Probot v14 heads up: current code relies on Octokit's legacy top-level REST method access (octokit.pulls.foo() instead of octokit.rest.pulls.foo()), which Probot v14 removes. Not a problem on the current v13, but will need updating across changelog.js, merge-bases.js, and track-versions.js on the next major upgrade.
  • GitHub Dependabot alerts: pushing the fix branch surfaced 50 open vulnerability alerts on the default branch (5 critical, 19 high, 19 moderate, 7 low) — see the repo's Security tab. Not evaluated as part of this audit scope, but given how long this app went without a deploy, likely worth a look.

Why this matters

The pattern that let #118's bugs hide for years — dormant code path + no test coverage — likely applies to more than just changelog.js. Recommend treating "does this have real test coverage or real traffic" as the triage question for anything found here.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions