feat: add orphaned-stages rule (v0.1.1)

CONTRIBUTING.md

@@ -0,0 +1,105 @@
+# Contributing to spinlint
+
+## Adding a Rule
+
+All lint rules implement the two-method [`Rule` interface](pkg/rules/rule.go):
+
+```go
+type Rule interface {
+    Name() string
+    Check(pipeline schema.Pipeline) []Violation
+}
+```
+
+`Violation` carries the rule name, the affected stage's `refId`, a human-readable message, and an optional severity:
+
+```go
+type Violation struct {
+    Rule     string `json:"rule"`
+    Stage    string `json:"stage,omitempty"`
+    Message  string `json:"message"`
+    Severity string `json:"severity,omitempty"` // "warning" or "" (treated as "error")
+}
+```
+
+Follow these steps to add a new rule. For more context, see the [Adding a Rule](README.md#adding-a-rule) section of the README.
+
+### 1. Create the rule file
+
+Create `pkg/rules/<your_rule>.go`:
+
+```go
+package rules
+
+import (
+    "fmt"
+    "github.com/jakeva/spinlint/pkg/schema"
+)
+
+type YourRule struct{}
+
+func (r YourRule) Name() string { return "your-rule-name" }
+
+func (r YourRule) Check(pipeline schema.Pipeline) []Violation {
+    var violations []Violation
+    for _, stage := range pipeline.Stages {
+        if /* condition */ {
+            violations = append(violations, Violation{
+                Rule:    r.Name(),
+                Stage:   stage.RefID,
+                Message: fmt.Sprintf("..."),
+                // Severity: "warning", // omit for error (the default)
+            })
+        }
+    }
+    return violations
+}
+```
+
+Set `Severity: "warning"` for advisory checks; omit it for hard errors. Errors cause a non-zero exit code and map to `"error"` level in SARIF output; warnings map to `"warning"` level.
+
+### 2. Register the rule
+
+Add your rule to `pkg/rules/registry.go`:
+
+```go
+var All = []Rule{
+    RequiredStageFields{},
+    BrokenRequisiteRefs{},
+    DuplicateRefIDs{},
+    CircularDependencies{},
+    OrphanedStages{},
+    YourRule{}, // add here
+}
+```
+
+Rules run in the order listed.
+
+### 3. Add a schema field (if needed)
+
+If your rule needs data not yet modelled, add it to `pkg/schema/pipeline.go`:
+
+```go
+type Stage struct {
+    // ...existing fields...
+    YourNewField bool `json:"yourNewField"`
+}
+```
+
+### 4. Write tests
+
+Create `pkg/rules/<your_rule>_test.go` using the `rules_test` package. See existing `*_test.go` files for the pattern. At minimum test the happy path (no violations) and one violation case.
+
+### 5. Update the README
+
+Add your rule to the **Rules** section of `README.md` following the same format as existing rules.
+
+---
+
+## Development workflow
+
+```bash
+make lint && make test
+```
+
+Requirements: Go 1.22+, `golangci-lint` (for the `lint` target only).

README.md

@@ -232,6 +232,26 @@ Self-loops (`requisiteStageRefIds: ["1"]` on stage `"1"`) are also detected.
 
 ---
 
+### `orphaned-stages` _(warning)_
+
+Flags stages that are completely disconnected from the pipeline graph: they have no prerequisites of their own (`requisiteStageRefIds` is empty) **and** no other stage depends on them. These are isolated islands that Spinnaker will run unconditionally in parallel with the rest of the pipeline, which is usually unintentional.
+
+This rule does **not** flag valid terminal stages (stages at the end of a dependency chain that nothing else depends on) — those have prerequisites and are a normal part of pipeline design.
+
+Stages with an empty `refId` are skipped (caught by `required-stage-fields` instead).
+
+**Checks:** every stage with an empty `requisiteStageRefIds` is depended on by at least one other stage.
+
+**Severity:** warning — violations appear as `[warn: orphaned-stages]` in text output and as `"level": "warning"` in SARIF.
+
+**Example violation:**
+
+```
+pipelines/deploy.json: [warn: orphaned-stages] stage "20" is an orphaned island: nothing depends on it and it has no prerequisites
+```
+
+---
+
 ## Exit Codes
 
 | Code | Meaning |
@@ -251,6 +271,9 @@ spinlint validate 'pipelines/*.json' || exit 1
 
 ```
 spinlint/
+├── .github/
+│   └── workflows/
+│       └── ci.yml           # CI: lint + test + SARIF upload on push/PR
 ├── cmd/
 │   └── spinlint/
 │       └── main.go          # CLI entry point (cobra root + validate command)
@@ -263,16 +286,19 @@ spinlint/
 │   │   ├── required_fields.go
 │   │   ├── requisite_refs.go
 │   │   ├── duplicate_ref_ids.go
-│   │   └── circular_deps.go
+│   │   ├── circular_deps.go
+│   │   └── orphaned_stages.go
 │   ├── loader/
 │   │   └── loader.go        # Glob expansion and JSON file loading
 │   └── reporter/
-│       └── reporter.go      # Text and JSON output formatting
+│       └── reporter.go      # Text, JSON, and SARIF output formatting
 ├── testdata/
 │   ├── valid.json           # A well-formed 3-stage pipeline
-│   └── invalid.json         # Pipeline with violations across multiple rules
+│   ├── invalid.json         # Pipeline with violations across multiple rules
+│   └── complex.json         # 20-stage pipeline hitting all 5 rules
 ├── go.mod
 ├── Makefile
+├── CONTRIBUTING.md
 └── README.md
 ```
 

pkg/reporter/reporter.go

@@ -36,7 +36,11 @@ func (r *Reporter) Add(file string, violations []rules.Violation) {
 			return
 		}
 		for _, v := range violations {
-			fmt.Fprintf(r.out, "%s: [%s] %s\n", file, v.Rule, v.Message)
+			if v.Severity == "warning" {
+				fmt.Fprintf(r.out, "%s: [warn: %s] %s\n", file, v.Rule, v.Message)
+			} else {
+				fmt.Fprintf(r.out, "%s: [%s] %s\n", file, v.Rule, v.Message)
+			}
 		}
 		return
 	}
@@ -130,9 +134,13 @@ func (r *Reporter) flushSARIF() error {
 					ShortDescription: sarifMessage{Text: v.Rule},
 				})
 			}
+			level := v.Severity
+			if level == "" {
+				level = "error"
+			}
 			results = append(results, sarifResult{
 				RuleID:  v.Rule,
-				Level:   "error",
+				Level:   level,
 				Message: sarifMessage{Text: v.Message},
 				Locations: []sarifLocation{
 					{

pkg/rules/orphaned_stages.go

@@ -0,0 +1,43 @@
+package rules
+
+import (
+	"fmt"
+
+	"github.com/jakeva/spinlint/pkg/schema"
+)
+
+// OrphanedStages flags stages that are completely disconnected from the pipeline
+// graph: no other stage depends on them AND they have no prerequisites of their own.
+// These are isolated islands that Spinnaker will execute independently, which is
+// usually unintentional. Severity: warning.
+type OrphanedStages struct{}
+
+func (r OrphanedStages) Name() string { return "orphaned-stages" }
+
+func (r OrphanedStages) Check(pipeline schema.Pipeline) []Violation {
+	// Build the set of refIds that at least one other stage depends on.
+	dependedOn := make(map[string]bool)
+	for _, stage := range pipeline.Stages {
+		for _, dep := range stage.RequisiteStageRefIds {
+			dependedOn[dep] = true
+		}
+	}
+
+	var violations []Violation
+	for _, stage := range pipeline.Stages {
+		if stage.RefID == "" {
+			continue // empty refId is caught by required-stage-fields
+		}
+		// A stage is an orphaned island if nothing depends on it AND it has no
+		// prerequisites of its own — i.e. it is completely disconnected.
+		if !dependedOn[stage.RefID] && len(stage.RequisiteStageRefIds) == 0 {
+			violations = append(violations, Violation{
+				Rule:     r.Name(),
+				Stage:    stage.RefID,
+				Severity: "warning",
+				Message:  fmt.Sprintf("stage %q is an orphaned island: nothing depends on it and it has no prerequisites", stage.RefID),
+			})
+		}
+	}
+	return violations
+}

pkg/rules/orphaned_stages_test.go

@@ -0,0 +1,141 @@
+package rules_test
+
+import (
+	"testing"
+
+	"github.com/jakeva/spinlint/pkg/rules"
+	"github.com/jakeva/spinlint/pkg/schema"
+)
+
+func TestOrphanedStages_NoOrphans(t *testing.T) {
+	// Linear chain: 1 ← 2. Stage 1 is a root (depended on), stage 2 is a leaf (has prereqs).
+	pipeline := schema.Pipeline{
+		Stages: []schema.Stage{
+			{Type: "wait", Name: "A", RefID: "1", RequisiteStageRefIds: []string{}},
+			{Type: "wait", Name: "B", RefID: "2", RequisiteStageRefIds: []string{"1"}},
+		},
+	}
+	v := rules.OrphanedStages{}.Check(pipeline)
+	if len(v) != 0 {
+		t.Errorf("expected no violations, got %d: %v", len(v), v)
+	}
+}
+
+func TestOrphanedStages_OrphanedIsland(t *testing.T) {
+	// Stage 3 has no prerequisites and nothing depends on it — orphaned island.
+	pipeline := schema.Pipeline{
+		Stages: []schema.Stage{
+			{Type: "wait", Name: "A", RefID: "1", RequisiteStageRefIds: []string{}},
+			{Type: "wait", Name: "B", RefID: "2", RequisiteStageRefIds: []string{"1"}},
+			{Type: "wait", Name: "C", RefID: "3", RequisiteStageRefIds: []string{}},
+		},
+	}
+	v := rules.OrphanedStages{}.Check(pipeline)
+	if len(v) != 1 {
+		t.Fatalf("expected 1 violation, got %d: %v", len(v), v)
+	}
+	if v[0].Stage != "3" {
+		t.Errorf("expected Stage %q, got %q", "3", v[0].Stage)
+	}
+	if v[0].Severity != "warning" {
+		t.Errorf("expected Severity %q, got %q", "warning", v[0].Severity)
+	}
+}
+
+func TestOrphanedStages_TerminalStageNotFlagged(t *testing.T) {
+	// Stage 2 is a leaf node (nothing depends on it) but has prerequisites — valid terminal stage.
+	pipeline := schema.Pipeline{
+		Stages: []schema.Stage{
+			{Type: "wait", Name: "A", RefID: "1", RequisiteStageRefIds: []string{}},
+			{Type: "wait", Name: "B", RefID: "2", RequisiteStageRefIds: []string{"1"}},
+		},
+	}
+	v := rules.OrphanedStages{}.Check(pipeline)
+	if len(v) != 0 {
+		t.Errorf("terminal stage should not be flagged, got %d: %v", len(v), v)
+	}
+}
+
+func TestOrphanedStages_SkipsEmptyRefID(t *testing.T) {
+	// A stage with no refId is skipped — caught by required-stage-fields, not us.
+	pipeline := schema.Pipeline{
+		Stages: []schema.Stage{
+			{Type: "wait", Name: "A", RefID: "", RequisiteStageRefIds: []string{}},
+		},
+	}
+	v := rules.OrphanedStages{}.Check(pipeline)
+	if len(v) != 0 {
+		t.Errorf("stage with empty refId should be skipped, got %d: %v", len(v), v)
+	}
+}
+
+func TestOrphanedStages_MultipleOrphans(t *testing.T) {
+	// Two disconnected islands alongside a valid chain.
+	pipeline := schema.Pipeline{
+		Stages: []schema.Stage{
+			{Type: "wait", Name: "Chain start", RefID: "1", RequisiteStageRefIds: []string{}},
+			{Type: "wait", Name: "Chain end", RefID: "2", RequisiteStageRefIds: []string{"1"}},
+			{Type: "wait", Name: "Orphan A", RefID: "3", RequisiteStageRefIds: []string{}},
+			{Type: "wait", Name: "Orphan B", RefID: "4", RequisiteStageRefIds: []string{}},
+		},
+	}
+	v := rules.OrphanedStages{}.Check(pipeline)
+	if len(v) != 2 {
+		t.Errorf("expected 2 violations, got %d: %v", len(v), v)
+	}
+}
+
+func TestOrphanedStages_ComplexTopology(t *testing.T) {
+	// Pipeline layout:
+	//
+	//   Diamond subgraph:
+	//     "1" ──► "2" ──► "4" ──► "5"  (terminal — has prereqs, not orphaned)
+	//     "1" ──► "3" ──► "4"
+	//
+	//   Separate independent chain:
+	//     "6" ──► "7"  (root "6" is depended-on; terminal "7" has prereqs)
+	//
+	//   Orphaned islands (no prereqs, nothing depends on them):
+	//     "8", "9"
+	//
+	//   Stage with empty refId — must be skipped.
+	//
+	// Expected: exactly 2 warnings, for stages "8" and "9".
+	pipeline := schema.Pipeline{
+		Stages: []schema.Stage{
+			{Type: "jenkins", Name: "Build",          RefID: "1", RequisiteStageRefIds: []string{}},
+			{Type: "deploy",  Name: "Deploy canary",  RefID: "2", RequisiteStageRefIds: []string{"1"}},
+			{Type: "deploy",  Name: "Deploy baseline", RefID: "3", RequisiteStageRefIds: []string{"1"}},
+			{Type: "manualJudgment", Name: "Approve", RefID: "4", RequisiteStageRefIds: []string{"2", "3"}},
+			{Type: "deploy",  Name: "Deploy prod",    RefID: "5", RequisiteStageRefIds: []string{"4"}},
+			{Type: "bake",    Name: "Bake image",     RefID: "6", RequisiteStageRefIds: []string{}},
+			{Type: "deploy",  Name: "Deploy staging", RefID: "7", RequisiteStageRefIds: []string{"6"}},
+			{Type: "wait",    Name: "Stale wait",     RefID: "8", RequisiteStageRefIds: []string{}},
+			{Type: "webhook", Name: "Dead webhook",   RefID: "9", RequisiteStageRefIds: []string{}},
+			{Type: "wait",    Name: "No refId",       RefID: "",  RequisiteStageRefIds: []string{}},
+		},
+	}
+
+	v := rules.OrphanedStages{}.Check(pipeline)
+	if len(v) != 2 {
+		t.Fatalf("expected 2 violations, got %d: %v", len(v), v)
+	}
+
+	orphaned := map[string]bool{}
+	for _, viol := range v {
+		orphaned[viol.Stage] = true
+		if viol.Severity != "warning" {
+			t.Errorf("stage %q: expected severity %q, got %q", viol.Stage, "warning", viol.Severity)
+		}
+	}
+	for _, id := range []string{"8", "9"} {
+		if !orphaned[id] {
+			t.Errorf("expected stage %q to be flagged as orphaned", id)
+		}
+	}
+	for _, id := range []string{"1", "2", "3", "4", "5", "6", "7"} {
+		if orphaned[id] {
+			t.Errorf("stage %q should not be flagged as orphaned", id)
+		}
+	}
+}

pkg/rules/registry.go

@@ -6,4 +6,5 @@ var All = []Rule{
 	BrokenRequisiteRefs{},
 	DuplicateRefIDs{},
 	CircularDependencies{},
+	OrphanedStages{},
 }

pkg/rules/rule.go

@@ -4,9 +4,10 @@ import "github.com/jakeva/spinlint/pkg/schema"
 
 // Violation describes a single rule failure found in a pipeline.
 type Violation struct {
-	Rule    string `json:"rule"`
-	Stage   string `json:"stage,omitempty"` // refId or "index N" if refId is absent
-	Message string `json:"message"`
+	Rule     string `json:"rule"`
+	Stage    string `json:"stage,omitempty"`    // refId or "index N" if refId is absent
+	Message  string `json:"message"`
+	Severity string `json:"severity,omitempty"` // "warning" or "" (treated as "error")
 }
 
 // Rule is implemented by every lint check.