chore: add ticket references to TODOs and enforce via CI (#71)

prajwolrg · web-flow · commit 030639ae1d0c · 2026-04-14T09:20:10.000Z
* chore: add proper ticket references to TODOs and fix small TODOs

* Add a CI check and justfile task that ensures all TODO/FIXME/HACK
comments include a ticket reference

* fix: remove james bond ticket number with actual ticket number

* fix: allow hacks without ticket references

* fix: ref proper ticket
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -84,6 +84,21 @@ jobs:
         run: |
           taplo fmt --check
 
+  todo-tickets:
+    name: Check ticketed TODOs
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    permissions:
+      contents: read
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v4
+        with:
+          persist-credentials: false
+
+      - name: Check for ticketless TODOs
+        run: bash contrib/check_ticketless_todos.sh
+
   lint-success:
     name: Check that lints passed
     runs-on: ubuntu-latest
@@ -92,6 +107,7 @@ jobs:
       - clippy
       - fmt
       - taplo
+      - todo-tickets
     timeout-minutes: 30
     steps:
       - name: Decide whether the needed jobs succeeded or failed
diff --git a/.justfile b/.justfile
@@ -50,8 +50,12 @@ doctest:
 functional-test:
   cd functional-tests && ./run_test.sh
 
+# Check that all TODO/FIXME/HACK comments have ticket references
+check-todos:
+  bash contrib/check_ticketless_todos.sh
+
 # Run all lints and formatting checks
-lints: toml-check-fmt toml-lint check-fmt clippy functional-check-fmt functional-lint
+lints: toml-check-fmt toml-lint check-fmt clippy functional-check-fmt functional-lint check-todos
 
 # Rust all tests
 test: unit-test doctest
diff --git a/bin/asm-runner/src/config.rs b/bin/asm-runner/src/config.rs
@@ -55,7 +55,7 @@ pub(crate) struct BitcoinConfig {
     /// Optional retry interval
     pub retry_interval: Option<Duration>,
     /// Connection string used in `bitcoin.conf => zmqpubrawblock`.
-    // TODO: (@prajwolrg) We should be able to work with `hashblock_connection_string` since ASM
+    // TODO(STR-2662): We should be able to work with `hashblock_connection_string` since ASM
     // runner used btc-client to fetch the full block. We don't use it here since the BlockEvent is
     // emitted only on the rawblock connection. Fix that.
     pub rawblock_connection_string: String,
diff --git a/bin/asm-runner/src/prover/input.rs b/bin/asm-runner/src/prover/input.rs
@@ -79,7 +79,7 @@ impl InputBuilder {
             .context("anchor state not found")?;
         let anchor_state = asm_state.state();
 
-        // TODO: https://alpenlabs.atlassian.net/browse/STR-2958 Construct proper MohoState
+        // TODO(STR-2958): Construct proper MohoState
         let inner_state_commitment = AsmStfProgram::compute_state_commitment(anchor_state);
         let moho_state = moho_types::MohoState::new(
             inner_state_commitment,
diff --git a/contrib/check_ticketless_todos.sh b/contrib/check_ticketless_todos.sh
@@ -0,0 +1,34 @@
+#!/usr/bin/env bash
+
+# Check that all TODO/FIXME comments include a ticket reference.
+#
+# Valid:   TODO(STR-2105), FIXME(#123)
+# Invalid: TODO, TODO:, FIXME without a ticket
+#
+# Usage:
+#   ./contrib/check_ticketless_todos.sh
+
+set -euo pipefail
+
+# Scan all source files for TODO/FIXME without ticket references.
+violations=$(grep -rn \
+    --include='*.rs' --include='*.py' --include='*.sh' \
+    --include='*.toml' --include='*.yml' --include='*.yaml' \
+    --exclude-dir='target' --exclude-dir='.venv' \
+    -E '\b(TODO|FIXME)\b' . \
+    | grep -vE '(TODO|FIXME)\([A-Za-z]+-[0-9]+\)' \
+    | grep -vE '(TODO|FIXME)\(#[0-9]+\)' \
+    | grep -vE 'check_ticketless_todos\.sh' \
+    | grep -vE 'TODO_TICKETS\.md' \
+    || true)
+
+if [ -n "$violations" ]; then
+    echo "ERROR: Found TODO/FIXME comments without ticket references."
+    echo "       Use the format TODO(PROJ-123) or FIXME(#456) instead."
+    echo ""
+    echo "$violations"
+    exit 1
+fi
+
+echo "OK: All TODO/FIXME comments have ticket references."
+exit 0
diff --git a/crates/common/src/msg.rs b/crates/common/src/msg.rs
@@ -87,11 +87,6 @@ impl Message {
     }
 }
 
-// [PLACE_HOLDER] Update the names to align with the team’s new naming convention.
-// TODO: [PLACE_HOLDER] L2ToL1Msg are not expected to be used as inter-proto messages. (probably
-// it's gonna be part of Aux input) Temporary alias
-pub type L2ToL1Msg = Message;
-
 #[cfg(test)]
 mod tests {
     use std::any::Any;
diff --git a/crates/common/src/subprotocol.rs b/crates/common/src/subprotocol.rs
@@ -149,10 +149,8 @@ pub trait Subprotocol: 'static {
     /// * `msgs` - Slice of messages received from other subprotocols
     /// * `l1ref` - L1 block being processed
     ///
-    /// TODO:
-    /// Also generate the event logs that is later needed for other components
-    /// to read ASM activity. Return the commitment of the events. The actual
-    /// event is defined by the subprotocol and is not visible to the ASM.
+    /// TODO(STR-3028): Enable log emission from process_msgs to support multi-round
+    /// inter-subprotocol messaging
     fn process_msgs(state: &mut Self::State, msgs: &[Self::Msg], l1ref: &L1BlockCommitment);
 }
 
diff --git a/crates/manifest-types/src/lib.rs b/crates/manifest-types/src/lib.rs
@@ -32,5 +32,5 @@ pub use ssz_generated::ssz::{
 };
 
 /// Type alias for a 32-byte hash.
-// TODO use Buf32 from identifiers?
+// TODO(STR-3027): use Buf32 from identifiers or use proper newtype instead
 pub type Hash32 = [u8; 32];
diff --git a/crates/manifest-types/src/payloads.rs b/crates/manifest-types/src/payloads.rs
@@ -10,8 +10,8 @@ use strata_msg_fmt::TypeId;
 use crate::AsmLog;
 
 // Define type IDs for these log types
-pub const DEPOSIT_INTENT_ASM_LOG_TYPE_ID: TypeId = 100; // TODO: Confirm the actual type ID
-pub const CHECKPOINT_ACK_ASM_LOG_TYPE_ID: TypeId = 101; // TODO: Confirm the actual type ID
+pub const DEPOSIT_INTENT_ASM_LOG_TYPE_ID: TypeId = 100; // TODO(STR-3029): Consolidate logs together
+pub const CHECKPOINT_ACK_ASM_LOG_TYPE_ID: TypeId = 101; // TODO(STR-3029): Consolidate logs together
 
 /// Payload for a deposit intent ASM log.
 ///
diff --git a/crates/stf/src/lib.rs b/crates/stf/src/lib.rs
@@ -8,11 +8,11 @@
 mod manager;
 mod preprocess;
 mod stage;
-mod stf;
+mod transition;
 mod tx_filter;
 mod types;
 
 pub use preprocess::pre_process_asm;
-pub use stf::compute_asm_transition;
+pub use transition::compute_asm_transition;
 pub use tx_filter::group_txs_by_subprotocol;
 pub use types::{AsmStfInput, AsmStfOutput};
diff --git a/crates/stf/src/manager.rs b/crates/stf/src/manager.rs
@@ -40,7 +40,7 @@ impl<S: Subprotocol, R: MsgRelayer> SubprotoHandler for HandlerImpl<S, R> {
         self.interproto_msg_buf.push(m.clone());
     }
 
-    // TODO make this just return the aux request
+    // TODO(STR-3065): make this just return the aux request
     fn pre_process_txs(&mut self, txs: &[TxInputRef<'_>], collector: &mut AuxRequestCollector) {
         S::pre_process_txs(&self.state, txs, collector);
     }
@@ -61,7 +61,7 @@ impl<S: Subprotocol, R: MsgRelayer> SubprotoHandler for HandlerImpl<S, R> {
     }
 
     fn process_buffered_msgs(&mut self, l1ref: &L1BlockCommitment) {
-        // TODO probably will make this more sophisticated
+        // TODO(STR-2416): allow multi rounds of interproto msg passing
         S::process_msgs(&mut self.state, &self.interproto_msg_buf, l1ref)
     }
 
diff --git a/crates/stf/src/stage.rs b/crates/stf/src/stage.rs
@@ -1,5 +1,4 @@
 //! Loader infrastructure for setting up the context.
-// TODO maybe move (parts of) this module to common?
 
 use std::collections::BTreeMap;
 
diff --git a/crates/stf/src/transition.rs b/crates/stf/src/transition.rs
@@ -1,7 +1,6 @@
 //! The `asm_stf` crate implements the core Anchor State Machine state transition function (STF). It
 //! glues together block‐level validation, a set of pluggable subprotocols, and the global chain
 //! view into a single deterministic state transition.
-// TODO rename this module to `transition`
 
 use bitcoin::Block;
 use ssz_types::VariableList;
@@ -68,7 +67,7 @@ pub fn compute_asm_transition<S: AsmSpec>(
 
     // 6. FINISH: Allow each subprotocol to process buffered inter-protocol messages.
     // This stage handles cross-protocol communication and finalizes state changes.
-    // TODO probably will have change this to repeat the interproto message
+    // TODO(STR-2416): probably will have change this to repeat the interproto message
     // processing phase until we have no more messages to deliver, or some
     // bounded number of times
     let mut finish_stage = FinishStage::new(&mut manager, &pow_state.last_verified_block);
diff --git a/crates/stf/src/tx_filter.rs b/crates/stf/src/tx_filter.rs
@@ -1,4 +1,3 @@
-// TODO: Move the logic with other libraries to do all the L1 transaction parsing logic.
 use std::collections::BTreeMap;
 
 use bitcoin::Transaction;
diff --git a/crates/subprotocols/admin/src/handler.rs b/crates/subprotocols/admin/src/handler.rs
@@ -56,7 +56,7 @@ pub(crate) fn handle_pending_updates(
                 let (key, kind) = update.into_inner();
                 match kind {
                     ProofType::Asm => {
-                        // TODO: STR-1721 Emit ASM Log
+                        // TODO(STR-1721): Emit ASM Log
                     }
                     ProofType::OLStf => {
                         relay_checkpoint_predicate(relayer, key);
@@ -68,7 +68,7 @@ pub(crate) fn handle_pending_updates(
                 }
             }
             UpdateAction::OperatorSet(_update) => {
-                // TODO: STR-1721 Set an InterProtoMsg to the Bridge Subprotocol
+                // TODO(STR-1721): Set an InterProtoMsg to the Bridge Subprotocol
             }
             UpdateAction::Sequencer(update) => {
                 let new_key = update.into_inner();
diff --git a/crates/subprotocols/bridge-v1/src/handler.rs b/crates/subprotocols/bridge-v1/src/handler.rs
@@ -88,7 +88,10 @@ pub(crate) fn handle_parsed_tx(
             // Use SubprotocolId as the containerId.
             let withdrawal_processed_log =
                 NewExportEntry::new(BRIDGE_V1_SUBPROTOCOL_ID, unlock.compute_hash());
-            relayer.emit_log(AsmLogEntry::from_log(&withdrawal_processed_log).expect("FIXME:PG"));
+            relayer.emit_log(
+                AsmLogEntry::from_log(&withdrawal_processed_log)
+                    .expect("withdrawal processed log must not fail"),
+            );
 
             info!(
                 deposit_idx,
diff --git a/crates/subprotocols/bridge-v1/src/state/assignment.rs b/crates/subprotocols/bridge-v1/src/state/assignment.rs
@@ -70,7 +70,7 @@ impl Ord for AssignmentEntry {
 }
 
 impl AssignmentEntry {
-    // TODO: rename this function — it's no longer purely random, it honors user-selected
+    // TODO(STR-2356): rename this function — it's no longer purely random, it honors user-selected
     // operators when eligible and falls back to random.
     /// Creates a new assignment entry by randomly selecting an eligible operator.
     ///
diff --git a/crates/txs/admin/src/parser.rs b/crates/txs/admin/src/parser.rs
@@ -45,7 +45,7 @@ impl SignedPayload {
 /// - The transaction lacks a taproot leaf script in its witness
 /// - The envelope payload cannot be parsed
 /// - The signed payload cannot be deserialized
-// TODO: https://alpenlabs.atlassian.net/browse/STR-2366
+// TODO(STR-2366): Update L1Payload to minimize DA footprint
 pub fn parse_tx(tx: &TxInputRef<'_>) -> Result<SignedPayload, AdministrationTxParseError> {
     let tx_type = tx.tag().tx_type();
 
diff --git a/crates/worker/src/aux_resolver.rs b/crates/worker/src/aux_resolver.rs
@@ -262,7 +262,7 @@ impl<'a> fmt::Debug for AuxDataResolver<'a> {
 
 #[cfg(test)]
 mod tests {
-    // TODO: Add tests once we have mock implementations
+    // TODO(STR-3031): Add tests
     // - test_resolve_empty_requests
     // - test_resolve_bitcoin_txs
     // - test_resolve_manifest_hashes
diff --git a/crates/worker/src/service.rs b/crates/worker/src/service.rs
@@ -160,7 +160,8 @@ where
             pivot_block.height(),
             *pivot_block.blkid(),
             wtxids_root.into(),
-            vec![], // TODO: this is not supposed to be empty right?
+            vec![], /* TODO(STR-2771): we shouldn't require a genesis manifest. The manifest
+                     * should start from the block after genesis. */
         )
         .expect("empty genesis manifest is within capacity");
 
diff --git a/tests/asm/checkpoint.rs b/tests/asm/checkpoint.rs
@@ -9,7 +9,7 @@
     reason = "test dependencies shared across test suite"
 )]
 
-// TODO: Add checkpoint-specific tests here
+// TODO(STR-3033): Add checkpoint-specific tests here
 // - Checkpoint submission and verification
 // - Batch processing
 // - Proof verification

Original file line number	Diff line number	Diff line change
`@@ -87,11 +87,6 @@ impl Message {`
`87`	`87`	`}`
`88`	`88`	`}`
`89`	`89`
`90`		`-// [PLACE_HOLDER] Update the names to align with the team’s new naming convention.`
`91`		`-// TODO: [PLACE_HOLDER] L2ToL1Msg are not expected to be used as inter-proto messages. (probably`
`92`		`-// it's gonna be part of Aux input) Temporary alias`
`93`		`-pub type L2ToL1Msg = Message;`
`94`		`-`
`95`	`90`	`#[cfg(test)]`
`96`	`91`	`mod tests {`
`97`	`92`	`use std::any::Any;`