From 4ee1e422b311a1d4b8961fd76e8b62b945208623 Mon Sep 17 00:00:00 2001 From: jrtxio Date: Fri, 23 Oct 2026 11:55:32 +0800 Subject: [PATCH] doctor: resolve mediated pins and stop judging changelog history (#310) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two release-gate false findings from products adopting the gate: - A workflow install step that indirects through a variable (`rivet.git#${{ env.RIVET_PIN }}`, `${RIVET_PIN}`, `$env:RIVET_PIN`) read as an empty ref, because the ref position stopped at the dollar sign. The pin regex now matches the mediation forms, resolves the name against the same file's env block (top-level and job-level), and an unresolvable mediation stays a finding — fail safe, like the empty-ref incident class. - The tag-vs-installed comparison ran on markdown prose too, so a released changelog section describing its past pin failed forever. The comparison now runs only on configuration files (.yml/.yaml/.rkt/ .rktd); the shape-based empty-ref check still applies everywhere. Tests: mediated+resolved passes, undefined variable fails naming the variable, resolved wrong tag fails the comparison, changelog prose with a past tag passes, empty ref in prose still fails. --- rivet-cli/doctor.rkt | 85 +++++++++++++++++++++++++++++++++--------- rivet/tests/doctor.rkt | 63 +++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 17 deletions(-) diff --git a/rivet-cli/doctor.rkt b/rivet-cli/doctor.rkt index 7069ac96..ef350ddf 100644 --- a/rivet-cli/doctor.rkt +++ b/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) diff --git a/rivet/tests/doctor.rkt b/rivet/tests/doctor.rkt index ac8d96a2..585dfb15 100644 --- a/rivet/tests/doctor.rkt +++ b/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.