feat: clean up TODOs and add CI check (#12005)

* maint: clean up TODO tags

Cleans up existing TODO tags, particularly those that referenced
old Linear issues. Updates the todo-checker script to allow
<#issue> as well as <issue>.

* feat(ci): add todo-issues check to main CI

Updates the main CI workflow to run the todo-issues check. For
now this check will ignore any bare TODOs. We can update the
script later to reject bare TODOs.

.circleci/config.yml

@@ -1763,6 +1763,8 @@ workflows:
           skip_slow_tests: true
           notify: true
       - cannon-build-test-vectors
+      - todo-issues:
+          name: todo-issues-check
       - shellcheck/check:
           name: shell-check
           # We don't need the `exclude` key as the orb detects the `.shellcheckrc`

cannon/mipsevm/multithreaded/instrumented_test.go

@@ -20,7 +20,7 @@ func vmFactory(state *State, po mipsevm.PreimageOracle, stdOut, stdErr io.Writer
 
 func TestInstrumentedState_OpenMips(t *testing.T) {
 	t.Parallel()
-	// TODO(cp-903): Add mt-specific tests here
+	// TODO: Add mt-specific tests here
 	testutil.RunVMTests_OpenMips(t, CreateEmptyState, vmFactory, "clone.bin")
 }
 

op-e2e/opgeth/op_geth_test.go

@@ -129,7 +129,7 @@ func TestGethOnlyPendingBlockIsLatest(t *testing.T) {
 	defer opGeth.Close()
 
 	checkPending := func(stage string, number uint64) {
-		// TODO(CLI-4044): pending-block ID change
+		// TODO: pending-block ID change
 		pendingBlock, err := opGeth.L2Client.BlockByNumber(ctx, big.NewInt(-1))
 		require.NoError(t, err, "failed to fetch pending block at stage "+stage)
 		require.Equal(t, number, pendingBlock.NumberU64(), "pending block must have expected number")

op-node/flags/flags.go

@@ -259,7 +259,7 @@ var (
 	MetricsAddrFlag = &cli.StringFlag{
 		Name:     "metrics.addr",
 		Usage:    "Metrics listening address",
-		Value:    "0.0.0.0", // TODO(CLI-4159): Switch to 127.0.0.1
+		Value:    "0.0.0.0", // TODO: Switch to 127.0.0.1
 		EnvVars:  prefixEnvVars("METRICS_ADDR"),
 		Category: OperationsCategory,
 	}

op-node/node/node.go

@@ -490,7 +490,7 @@ func (n *OpNode) initP2P(cfg *Config) (err error) {
 		panic("p2p node already initialized")
 	}
 	if n.p2pEnabled() {
-		// TODO(protocol-quest/97): Use EL Sync instead of CL Alt sync for fetching missing blocks in the payload queue.
+		// TODO(protocol-quest#97): Use EL Sync instead of CL Alt sync for fetching missing blocks in the payload queue.
 		n.p2pNode, err = p2p.NewNodeP2P(n.resourcesCtx, &cfg.Rollup, n.log, cfg.P2P, n, n.l2Source, n.runCfg, n.metrics, false)
 		if err != nil {
 			return

op-node/p2p/node.go

@@ -79,7 +79,7 @@ func NewNodeP2P(
 	}
 	if n.host == nil {
 		// See prior comment about n.host optionality:
-		// TODO(CLI-4016): host is not optional, NodeP2P as a whole is.
+		// TODO: host is not optional, NodeP2P as a whole is.
 		panic("host is not optional if p2p is enabled")
 	}
 	return &n, nil

op-node/p2p/sync.go

@@ -57,7 +57,7 @@ const (
 	// If the client hits a request error, it counts as a lot of rate-limit tokens for syncing from that peer:
 	// we rather sync from other servers. We'll try again later,
 	// and eventually kick the peer based on degraded scoring if it's really not serving us well.
-	// TODO(CLI-4009): Use a backoff rather than this mechanism.
+	// TODO: Use a backoff rather than this mechanism.
 	clientErrRateCost = peerServerBlocksBurst
 )
 
@@ -310,7 +310,7 @@ func NewSyncClient(log log.Logger, cfg *rollup.Config, host HostNewStream, rcv r
 	}
 
 	// never errors with positive LRU cache size
-	// TODO(CLI-3733): if we had an LRU based on on total payloads size, instead of payload count,
+	// TODO: if we had an LRU based on on total payloads size, instead of payload count,
 	//  we can safely buffer more data in the happy case.
 	q, _ := simplelru.NewLRU[common.Hash, syncResult](100, c.onQuarantineEvict)
 	c.quarantine = q

op-node/rollup/engine/engine_controller.go

@@ -25,7 +25,7 @@ const (
 	// We transition between the 4 EL states linearly. We spend the majority of the time in the second & fourth.
 	// We only want to EL sync if there is no finalized block & once we finish EL sync we need to mark the last block
 	// as finalized so we can switch to consolidation
-	// TODO(protocol-quest/91): We can restart EL sync & still consolidate if there finalized blocks on the execution client if the
+	// TODO(protocol-quest#91): We can restart EL sync & still consolidate if there finalized blocks on the execution client if the
 	// execution client is running in archive mode. In some cases we may want to switch back from CL to EL sync, but that is complicated.
 	syncStatusWillStartEL               // First if we are directed to EL sync, check that nothing has been finalized yet
 	syncStatusStartedEL                 // Perform our EL sync

op-node/rollup/engine/engine_update.go

@@ -104,7 +104,7 @@ func startPayload(ctx context.Context, eng ExecEngine, fc eth.ForkchoiceState, a
 	}
 
 	switch fcRes.PayloadStatus.Status {
-	// TODO(proto): snap sync - specify explicit different error type if node is syncing
+	// TODO: snap sync - specify explicit different error type if node is syncing
 	case eth.ExecutionInvalid, eth.ExecutionInvalidBlockHash:
 		return eth.PayloadID{}, BlockInsertPayloadErr, eth.ForkchoiceUpdateErr(fcRes.PayloadStatus)
 	case eth.ExecutionValid:

op-service/metrics/cli.go

@@ -37,7 +37,7 @@ func CLIFlags(envPrefix string) []cli.Flag {
 		&cli.StringFlag{
 			Name:    ListenAddrFlagName,
 			Usage:   "Metrics listening address",
-			Value:   defaultListenAddr, // TODO(CLI-4159): Switch to 127.0.0.1
+			Value:   defaultListenAddr, // TODO: Switch to 127.0.0.1
 			EnvVars: opservice.PrefixEnvVar(envPrefix, "METRICS_ADDR"),
 		},
 		&cli.IntFlag{

op-service/metrics/http.go

@@ -65,7 +65,7 @@ var LatencyBuckets = []float64{.025, .05, .1, .25, .5, 1, 2.5, 5, 10, 25, 50, 10
 
 func NewPromHTTPRecorder(r *prometheus.Registry, ns string) HTTPRecorder {
 	return &PromHTTPRecorder{
-		// TODO(INF-509): remove this in the future when services opted in to HTTPRequestLatency
+		// TODO: remove this in the future when services opted in to HTTPRequestLatency
 		HTTPRequestDuration: promauto.With(r).NewHistogramVec(prometheus.HistogramOpts{
 			Namespace: ns,
 			Name:      "http_request_duration_ms",
@@ -104,7 +104,7 @@ func NewPromHTTPRecorder(r *prometheus.Registry, ns string) HTTPRecorder {
 }
 
 func (p *PromHTTPRecorder) RecordHTTPRequestDuration(params *HTTPParams, dur time.Duration) {
-	// TODO(INF-509): remove this in the future when services opted in to new metric
+	// TODO: remove this in the future when services opted in to new metric
 	p.HTTPRequestDuration.WithLabelValues(params.Method, strconv.Itoa(params.StatusCode)).
 		Observe(float64(dur.Milliseconds()))
 

op-service/oppprof/cli.go

@@ -76,7 +76,7 @@ func CLIFlagsWithCategory(envPrefix string, category string) []cli.Flag {
 		&cli.StringFlag{
 			Name:     ListenAddrFlagName,
 			Usage:    "pprof listening address",
-			Value:    defaultListenAddr, // TODO(CLI-4159): Switch to 127.0.0.1
+			Value:    defaultListenAddr, // TODO: Switch to 127.0.0.1
 			EnvVars:  opservice.PrefixEnvVar(envPrefix, "PPROF_ADDR"),
 			Category: category,
 		},

op-service/rpc/cli.go

@@ -21,7 +21,7 @@ func CLIFlags(envPrefix string) []cli.Flag {
 		&cli.StringFlag{
 			Name:    ListenAddrFlagName,
 			Usage:   "rpc listening address",
-			Value:   "0.0.0.0", // TODO(CLI-4159): Switch to 127.0.0.1
+			Value:   "0.0.0.0", // TODO: Switch to 127.0.0.1
 			EnvVars: opservice.PrefixEnvVar(envPrefix, "RPC_ADDR"),
 		},
 		&cli.IntFlag{

op-service/txmgr/txmgr.go

@@ -110,7 +110,7 @@ type ETHBackend interface {
 	SendTransaction(ctx context.Context, tx *types.Transaction) error
 
 	// These functions are used to estimate what the base fee & priority fee should be set to.
-	// TODO(CLI-3318): Maybe need a generic interface to support different RPC providers
+	// TODO: Maybe need a generic interface to support different RPC providers
 	HeaderByNumber(ctx context.Context, number *big.Int) (*types.Header, error)
 	SuggestGasTipCap(ctx context.Context) (*big.Int, error)
 	// NonceAt returns the account nonce of the given account.
@@ -983,7 +983,7 @@ func updateFees(oldTip, oldFeeCap, newTip, newBaseFee *big.Int, isBlobTx bool, l
 		return newTip, newFeeCap
 	} else if newTip.Cmp(thresholdTip) >= 0 && newFeeCap.Cmp(thresholdFeeCap) < 0 {
 		// Tip has gone up, but base fee is flat or down.
-		// TODO(CLI-3714): Do we need to recalculate the FC here?
+		// TODO: Do we need to recalculate the FC here?
 		lgr.Debug("Using new tip and threshold feecap")
 		return newTip, thresholdFeeCap
 	} else if newTip.Cmp(thresholdTip) < 0 && newFeeCap.Cmp(thresholdFeeCap) >= 0 {
@@ -993,7 +993,7 @@ func updateFees(oldTip, oldFeeCap, newTip, newBaseFee *big.Int, isBlobTx bool, l
 		return thresholdTip, calcGasFeeCap(newBaseFee, thresholdTip)
 
 	} else {
-		// TODO(CLI-3713): Should we skip the bump in this case?
+		// TODO: Should we skip the bump in this case?
 		lgr.Debug("Using threshold tip and threshold feecap")
 		return thresholdTip, thresholdFeeCap
 	}

ops/scripts/todo-checker.sh

@@ -65,10 +65,10 @@ for todo in $todos; do
     # * TODO(repo#<issue_number>): <description> (Default org "ethereum-optimism")
     # * TODO(org/repo#<issue_number>): <description>
     #
-    # Check if it's just a number
-    if [[ $ISSUE_REFERENCE =~ ^[0-9]+$ ]]; then
+    # Check if it's just a number or a number with a leading #
+    if [[ $ISSUE_REFERENCE =~ ^[0-9]+$ ]] || [[ $ISSUE_REFERENCE =~ ^#([0-9]+)$ ]]; then
         REPO_FULL="$ORG/$REPO"
-        ISSUE_NUM="$ISSUE_REFERENCE"
+        ISSUE_NUM="${ISSUE_REFERENCE#\#}"  # Remove leading # if present
     # Check for org_name/repo_name#number format
     elif [[ $ISSUE_REFERENCE =~ ^([^/]+)/([^#]+)#([0-9]+)$ ]]; then
         REPO_FULL="${BASH_REMATCH[1]}/${BASH_REMATCH[2]}"