doctor: resolve mediated pins and stop judging changelog history (#310)

rivet-cli/doctor.rkt

@@ -405,7 +405,34 @@
                   '(".git" ".rivet" "compiled" "dist" "node_modules" "stage")
                   #rx"[.]rkt$"))
 
-(define pin-regex #px"rivet\\.git#([A-Za-z0-9._-]*)")
+;; The ref position also matches the variable-mediated forms a workflow
+;; install step uses (`${{ env.RIVET_PIN }}`, `${RIVET_PIN}`, `$env:RIVET_PIN`);
+;; a plain character-class scan saw those as empty refs (issue #310).
+(define pin-regex
+  #px"rivet\\.git#(\\$\\{\\{ ?env\\.[A-Za-z_][A-Za-z0-9_]* ?\\}\\}|\\$\\{[A-Za-z_][A-Za-z0-9_]*\\}|\\$env:[A-Za-z_][A-Za-z0-9_]*|[A-Za-z0-9._-]*)")
+
+(define (mediated-var-name ref)
+  (define match
+    (regexp-match #px"^\\$\\{\\{ ?env\\.([A-Za-z_][A-Za-z0-9_]*) ?\\}\\}$|^\\$\\{([A-Za-z_][A-Za-z0-9_]*)\\}$|^\\$env:([A-Za-z_][A-Za-z0-9_]*)$"
+                  ref))
+  (and match (or (cadr match) (caddr match) (cadddr match))))
+
+;; NAME: value pairs from every `env:` mapping in a workflow-style file —
+;; top-level and job-level blocks both bind the names installs reference.
+(define (env-definitions text)
+  (for/fold ([env (hash)] [in-env? #f] #:result env)
+            ([line (in-list (string-split text "\n"))])
+    (define block (regexp-match #px"^env:[\t ]*$" line))
+    (define entry (regexp-match #px"^[\t ]+([A-Za-z_][A-Za-z0-9_]*):[\t ]*(.*?)[\t ]*$" line))
+    (cond
+      [block (values env #t)]
+      [(and in-env? entry)
+       (define value (string-trim (caddr entry) "\""))
+       (values (hash-set env (cadr entry) value) #t)]
+      [else (values env (and in-env? (regexp-match? #px"^[\t ]" line)))])))
+
+(define (config-file? path)
+  (regexp-match? #rx"[.](yml|yaml|rkt|rktd)$" (path->string path)))
 
 (define (text-files-under root)
   (collect-files! root
@@ -460,32 +487,56 @@
    (for/list ([path (in-list (source-files-under root))])
      (update-api-contract-violations procedures path))))
 
+(define (classify-pin ref path installed-version expected-tag)
+  (cond
+    [(string=? ref "")
+     (list 'finding
+           (format "~a: dependency pin `rivet.git#` has an empty ref and silently tracks the default branch; pin a tag or commit SHA"
+                   path))]
+    ;; Tag-shaped refs must name the installed release; commit SHAs cannot
+    ;; be compared and are accepted as-is. The comparison runs only on
+    ;; configuration files: markdown prose documents history — a changelog
+    ;; section describing a past release legitimately names its past pin
+    ;; (issue #310), so it is not judged against the installed version.
+    [(and expected-tag
+          (config-file? path)
+          (regexp-match? #rx"^v[0-9]" ref)
+          (not (string=? ref expected-tag)))
+     (list 'finding
+           (format "~a: dependency pin `rivet.git#~a` does not match the installed rivet release ~a"
+                   path ref installed-version))]
+    [else (list 'ok ref)]))
+
 (define (repository-pin-findings root installed-version)
   ;; Empty refs are the autarx v0.4.1 incident: `URL#` resolves to the
   ;; default branch, so CI ran a moving rivet while the release believed it
-  ;; was pinned. Tag-shaped refs must name the installed release; commit
-  ;; SHAs cannot be compared and are accepted as-is.
+  ;; was pinned. A variable-mediated ref is resolved against the same
+  ;; file's env block before classification, and a mediation this file
+  ;; cannot resolve stays a finding — fail safe, like the incident class
+  ;; this gate exists for (issue #310).
   (define expected-tag (and installed-version (string-append "v" installed-version)))
   (define findings '())
   (for ([path (in-list (text-files-under root))])
     (define text
       (with-handlers ([exn:fail? (lambda (_) "")])
         (file->string path)))
+    (define env (env-definitions text))
     (for ([pin-match (in-list (regexp-match* pin-regex text #:match-select cdr))])
-      (define ref (list-ref pin-match 0))
-      (cond
-        [(string=? ref "")
-         (set! findings
-               (cons (format "~a: dependency pin `rivet.git#` has an empty ref and silently tracks the default branch; pin a tag or commit SHA"
-                             path)
-                     findings))]
-        [(and expected-tag
-              (regexp-match? #rx"^v[0-9]" ref)
-              (not (string=? ref expected-tag)))
-         (set! findings
-               (cons (format "~a: dependency pin `rivet.git#~a` does not match the installed rivet release ~a"
-                             path ref installed-version)
-                     findings))])))
+      (define raw-ref (list-ref pin-match 0))
+      (define resolved
+        (if (string-prefix? raw-ref "$")
+            (let ([name (mediated-var-name raw-ref)])
+              (or (and name (hash-ref env name #f))
+                  (list 'finding
+                        (format "~a: dependency pin `rivet.git#~a` reads an env variable this file does not define; inline the pin or add it to this file's env block"
+                                path raw-ref))))
+            raw-ref))
+      (define verdict
+        (if (and (pair? resolved) (eq? (car resolved) 'finding))
+            resolved
+            (classify-pin resolved path installed-version expected-tag)))
+      (when (eq? (car verdict) 'finding)
+        (set! findings (cons (cadr verdict) findings)))))
   (reverse findings))
 
 (define (release-check name thunk)

rivet/tests/doctor.rkt

@@ -229,6 +229,69 @@ RKT
                              "dependency pins are non-empty and match the installed release"
                              #rx"empty ref"))
 
+    ;; Issue #310: a variable-mediated pin is resolved against this file's
+    ;; env block; an undefined variable and a resolved wrong tag both fail;
+    ;; a past pin in changelog prose is history, not a live pin, while the
+    ;; shape-based empty-ref check still applies to prose.
+    (let ([workflows (build-path project-root ".github" "workflows")])
+      (when (directory-exists? workflows)
+        (define (write-ci content)
+          (call-with-output-file
+           (build-path workflows "ci.yml")
+           #:exists 'truncate
+           (lambda (out) (displayln content out))))
+        ;; mediated and resolved to the installed release -> passes
+        (write-ci
+         (format "env:\n  RIVET_PIN: v~a\npackages: \"https://github.com/turinglambdaai/rivet.git#${{ env.RIVET_PIN }}\""
+                 installed))
+        (define mediated-report (release-doctor-report project))
+        (release-check-passed?
+         mediated-report
+         "dependency pins are non-empty and match the installed release")
+        ;; mediated but undefined -> fails naming the variable
+        (write-ci
+         "packages: \"https://github.com/turinglambdaai/rivet.git#${{ env.RIVET_PIN }}\"")
+        (define unresolved-report (release-doctor-report project))
+        (release-check-failed?
+         unresolved-report
+         "dependency pins are non-empty and match the installed release"
+         #rx"reads an env variable this file does not define")
+        ;; mediated and resolved to a wrong tag -> fails the comparison
+        (write-ci
+         "env:\n  RIVET_PIN: v0.0.1\npackages: \"https://github.com/turinglambdaai/rivet.git#${{ env.RIVET_PIN }}\"")
+        (define wrong-tag-report (release-doctor-report project))
+        (release-check-failed?
+         wrong-tag-report
+         "dependency pins are non-empty and match the installed release"
+         #rx"does not match the installed rivet release")
+        ;; restore the live workflow to the installed release, then check
+        ;; that changelog prose naming a past pin is exempt from the comparison
+        (write-ci
+         (format "env:\n  RIVET_PIN: v~a\npackages: \"https://github.com/turinglambdaai/rivet.git#${{ env.RIVET_PIN }}\""
+                 installed))
+        (call-with-output-file
+         (build-path project-root "CHANGELOG.md")
+         #:exists 'truncate
+         (lambda (out)
+           (displayln "installed Rivet from `rivet.git#v0.0.1` instead of floating"
+                      out)))
+        (define history-report (release-doctor-report project))
+        (release-check-passed?
+         history-report
+         "dependency pins are non-empty and match the installed release")
+        ;; but an empty ref in prose is still the autarx shape
+        (call-with-output-file
+         (build-path project-root "CHANGELOG.md")
+         #:exists 'truncate
+         (lambda (out)
+           (displayln "installed Rivet from `rivet.git#` instead of floating"
+                      out)))
+        (define prose-empty-report (release-doctor-report project))
+        (release-check-failed?
+         prose-empty-report
+         "dependency pins are non-empty and match the installed release"
+         #rx"empty ref")))
+
     ;; The scanner itself is exported so products can run just this check.
     ;; updater-usage.rkt still holds the red-2 drifted call here, so the
     ;; direct scan must report the violation against the live contract.