feat!: Collect review trailers, speak kernel review, open commands

Breaking-Users: -by lines and "Not"/"No" retractions no longer
require being a maintainer or the original setter.

Author: bzg

docs/bone-manual.org

@@ -124,6 +124,12 @@
  :report/last-activity-address {:db/valueType :db.type/string}
  :report/has-ics               {:db/valueType :db.type/boolean}
  :report/has-text-attachments  {:db/valueType :db.type/boolean}
+ ;; Git person trailers (Acked-by:, Reviewed-by:, ...) collected from
+ ;; replies to a patch report, verbatim "Key: Name <addr>" strings.
+ ;; Clients applying the patch fold them into the commit message
+ ;; (b4-style).  Set semantics: duplicates and replays collapse.
+ :report/trailers              {:db/valueType   :db.type/string
+                                :db/cardinality :db.cardinality/many}
  :report/updated-at            {:db/valueType :db.type/instant}
  ;; Wall-clock instant the report was first created (ingested), as opposed
  ;; to the originating email's date (`:report/email` -> :email/date-sent),

resources/bone-schema.edn

@@ -1093,7 +1093,7 @@
   :email/body-text  "Owned-by: fixer@test.org\n"}
 
  ;; =========================================================================
- ;; 84 -- Regular user tries Closed-by directive (should be denied)
+ ;; 84 -- Regular user uses Closed-by directive (open to any user)
  ;; =========================================================================
  {
   :email/id        "84"

resources/emails.edn

@@ -449,16 +449,24 @@
    [:report/urgent    :urgent    :urgent-proxy]
    [:report/important :important :important-proxy]])
 
-(defn- assoc-owned-name
-  "Attach :owned-name when the owner's display name is known (looked up
-  via the global author-names map built once per export)."
-  [m author-names]
-  (if-let [owned (:owned m)]
-    (if-let [nm (get author-names (str/lower-case owned))]
-      (assoc m :owned-name nm)
+(defn- assoc-state-name
+  "Attach `name-k` when the display name of the address under `addr-k`
+  is known (looked up via the global author-names map built once per
+  export)."
+  [m addr-k name-k author-names]
+  (if-let [addr (get m addr-k)]
+    (if-let [nm (get author-names (str/lower-case addr))]
+      (assoc m name-k nm)
       m)
     m))
 
+(defn- assoc-state-names
+  "Attach :owned-name and :acked-name when known."
+  [m author-names]
+  (-> m
+      (assoc-state-name :owned :owned-name author-names)
+      (assoc-state-name :acked :acked-name author-names)))
+
 (defn- assoc-from-addresses
   "Extract addresses from report: direct string attrs and author-address of ref attrs.
   Omits -proxy keys when their value equals the corresponding address key."
@@ -717,7 +725,7 @@
          :priority (report-priority report)
          :replies  (report-descendant-count report)}
         (assoc-from-addresses report)
-        (assoc-owned-name (ctx-author-names))
+        (assoc-state-names (ctx-author-names))
         (cond->
             from-name                       (assoc :from-name from-name)
             role                            (assoc :role role)
@@ -726,6 +734,8 @@
             (:report/topic-value report)    (assoc :topic (:report/topic-value report))
             (:report/patch-seq report)      (assoc :patch-seq (:report/patch-seq report))
             (:report/patch-source report)   (assoc :patch-source (mapv name (sort (:report/patch-source report))))
+            ;; Sorted for reproducible exports (cardinality-many = set).
+            (seq (:report/trailers report)) (assoc :trailers (vec (sort (:report/trailers report))))
             arch                            (assoc :archived-at arch)
             (:report/deadline-value report) (assoc :deadline (format-date-iso (:report/deadline-value report)))
             (:report/last-activity report)  (assoc :last-activity (format-date-iso (:report/last-activity report)))

scripts/bone-export.clj

@@ -42,8 +42,13 @@
         "(" (str/join "|" (map #(java.util.regex.Pattern/quote %) words))
         ")(?:" trailing-punct (when-not strict-punct? "|\\s") "|$)")))
 
-(defn- line-pattern [strict-syntax? {:keys [syntax param]}]
-  (let [qs       (java.util.regex.Pattern/quote syntax)
+(defn- line-pattern [strict-syntax? {:keys [syntax param]} syntaxes]
+  (let [qs       (if (seq syntaxes)
+                   (str "(?:"
+                        (str/join "|" (map #(java.util.regex.Pattern/quote %)
+                                           syntaxes))
+                        ")")
+                   (java.util.regex.Pattern/quote syntax))
         prefix   (common/bang-prefix strict-syntax?)
         addr     "[^@<>\\s]+@[^@<>\\s]+\\.[^@<>\\s]+"   ; email address (dot required)
         mid      "[^<>\\s]+@[^<>\\s]+"                  ; bracketed message-id
@@ -69,11 +74,24 @@
                             words)]))
            action-map))))
 
+(defn- command-syntaxes
+  "Alternative syntaxes of a line command, nil for the single default.
+  Explicit :syntaxes win; unset commands carrying :not-words accept
+  the \"Not <word>\" negation of every word in the state's resolved
+  vocabulary."
+  [cmd commands-map]
+  (or (:syntaxes cmd)
+      (when-let [words (some->> (:not-words cmd) (get commands-map) seq)]
+        (mapv #(str "Not " (str/lower-case %)) words))))
+
 (def ^:private compile-line-patterns
   "Compile colon-line command patterns for a syntax mode."
   (memoize
-   (fn [strict-syntax?]
-     (mapv (fn [cmd] [cmd (line-pattern strict-syntax? cmd)]) line-commands))))
+   (fn [strict-syntax? commands-map]
+     (mapv (fn [cmd]
+             [cmd (line-pattern strict-syntax? cmd
+                                (command-syntaxes cmd commands-map))])
+           line-commands))))
 
 (defn build-source-commands
   "Return a source-commands descriptor with keys:
@@ -87,7 +105,7 @@
         strict-syntax? (= :strict (common/resolve-command-syntax source-cfg))]
     {:commands       commands
      :word-patterns  (compile-word-patterns strict-syntax? commands)
-     :line-patterns  (compile-line-patterns strict-syntax?)
+     :line-patterns  (compile-line-patterns strict-syntax? commands)
      :strict-syntax? strict-syntax?
      :overrides      (common/resolve-command-overrides source-cfg)}))
 
@@ -150,7 +168,8 @@
                      (assoc :report/close-reason reason))]
       (when (seq result) result))))
 
-(def ^:private compiled-lines-loose (compile-line-patterns false))
+(def ^:private compiled-lines-loose
+  (compile-line-patterns false common/default-commands))
 
 (defn detect-lines
   "Detect colon-line commands in `body-text`. The optional `compiled`

src/bone/commands.clj

@@ -24,13 +24,23 @@
 ;; prefixed line (`:syntax` key + optional `:param`).
 ;;
 ;; :scope values:
-;;   :user                 -- anyone
+;;   :user                 -- anyone.  The default everywhere: updates
+;;                            are auditable (each records the email
+;;                            that posed it) and reversible, and
+;;                            sources that are not self-moderating
+;;                            channels can tighten commands via the
+;;                            per-command :scope override.  Role
+;;                            controls (Add maintainer: ...) are gated
+;;                            separately, in bone.roles.
 ;;   :maintainer           -- any maintainer
 ;;   :setter-or-maintainer -- the address that set the attribute, plus
 ;;                            any maintainer.  Only meaningful for unset
 ;;                            commands whose target has a recoverable
 ;;                            setter (`setter-ref-attrs` /
 ;;                            `setter-scoped-command-ids`).
+;; No default entry uses the last two anymore; they remain available to
+;; sources that tighten a command via the per-command :scope override
+;; (config.edn `:commands {<cmd-id> {:scope ...}}`).
 ;; ---------------------------------------------------------------------------
 
 (def commands
@@ -41,19 +51,34 @@
    {:id :owned    :kind :trigger :action :set :attr :report/owned  :scope :user
     :words :owned  :report-types #{:bug :patch :request}}
    {:id :closed   :kind :trigger :action :set :attr :report/closed :scope :user :words :closed}
-   ;; -by lines (maintainer credits a third party)
-   {:id :acked-by  :kind :trigger :action :set :attr :report/acked  :scope :maintainer
-    :syntax "Acked-by"  :param :email-address :report-types #{:bug :patch :request}}
-   {:id :owned-by  :kind :trigger :action :set :attr :report/owned  :scope :maintainer
+   ;; -by lines (credit the update to a third party)
+   ;; Reviewed-by is the kernel-style strong-review line, accepted as
+   ;; a plain syntax synonym: BONE's acked state is the strong
+   ;; approval (Confirmed, Approved, Reviewed-by:).  Any participant's
+   ;; Reviewed-by both acks the report and is collected as a review
+   ;; trailer (:report/trailers); on a source that tightens :acked-by
+   ;; via a :scope override, denied lines are still collected as
+   ;; trailers.
+   {:id :acked-by  :kind :trigger :action :set :attr :report/acked  :scope :user
+    :syntax "Acked-by" :syntaxes ["Acked-by" "Reviewed-by"]
+    :param :email-address :report-types #{:bug :patch :request}}
+   {:id :owned-by  :kind :trigger :action :set :attr :report/owned  :scope :user
     :syntax "Owned-by"  :param :email-address :report-types #{:bug :patch :request}}
-   {:id :closed-by :kind :trigger :action :set :attr :report/closed :scope :maintainer
+   {:id :closed-by :kind :trigger :action :set :attr :report/closed :scope :user
     :syntax "Closed-by" :param :email-address}
-   ;; Unset lines
-   {:id :unacked  :kind :trigger :action :unset :attr :report/acked  :scope :setter-or-maintainer
-    :syntax "Not acked"  :report-types #{:bug :patch :request}}
-   {:id :unowned  :kind :trigger :action :unset :attr :report/owned  :scope :setter-or-maintainer
-    :syntax "Not owned"  :report-types #{:bug :patch :request}}
-   {:id :unclosed :kind :trigger :action :unset :attr :report/closed :scope :setter-or-maintainer
+   ;; Unset lines.  :not-words derives the accepted syntaxes from the
+   ;; state's resolved vocabulary -- every word that can set the state
+   ;; (defaults and per-source synonyms alike) gets its "Not <word>"
+   ;; negation; :syntax remains the canonical form for display.
+   {:id :unacked  :kind :trigger :action :unset :attr :report/acked  :scope :user
+    :syntax "Not acked"  :not-words :acked :report-types #{:bug :patch :request}}
+   {:id :unowned  :kind :trigger :action :unset :attr :report/owned  :scope :user
+    :syntax "Not owned"  :not-words :owned :report-types #{:bug :patch :request}}
+   ;; Deliberately no :not-words here: closing words carry a close
+   ;; reason ("Not canceled"/"Not fixed" would not), and in loose mode
+   ;; a prose "Not fixed." would reopen reports.  "Not closed" is the
+   ;; only reopening form.
+   {:id :unclosed :kind :trigger :action :unset :attr :report/closed :scope :user
     :syntax "Not closed"}
    ;; Closure relations -- backed by :rel/supersedes / :rel/duplicates;
    ;; :attr kept for registry shape.
@@ -64,40 +89,40 @@
    ;; pull-map key must distinguish them.
    {:id :superseded-by    :kind :trigger :action :set-superseded   :attr :rel/supersedes      :scope :user
     :syntax "Superseded-by"     :param :message-id :report-types #{:bug :patch :request}}
-   {:id :unsuperseded-by  :kind :trigger :action :unset-superseded :attr :rel/supersedes-from :scope :setter-or-maintainer
+   {:id :unsuperseded-by  :kind :trigger :action :unset-superseded :attr :rel/supersedes-from :scope :user
     :syntax "Not superseded-by" :param :message-id :report-types #{:bug :patch :request}}
    {:id :duplicate-of    :kind :trigger :action :set-duplicate   :attr :rel/duplicates      :scope :user
     :syntax "Duplicate-of"     :param :message-id :report-types #{:bug :patch :request}}
-   {:id :unduplicate-of  :kind :trigger :action :unset-duplicate :attr :rel/duplicates-from :scope :setter-or-maintainer
+   {:id :unduplicate-of  :kind :trigger :action :unset-duplicate :attr :rel/duplicates-from :scope :user
     :syntax "Not duplicate-of" :param :message-id :report-types #{:bug :patch :request}}
 
    ;; --- Annotations: property-set -------------------------------------------
    ;; Bareword
    {:id :urgent    :kind :annotation :action :set :attr :report/urgent    :scope :user :words :urgent}
    {:id :important :kind :annotation :action :set :attr :report/important :scope :user :words :important}
    ;; Unset lines for bareword annotations
-   {:id :unurgent    :kind :annotation :action :unset :attr :report/urgent    :scope :setter-or-maintainer
-    :syntax "Not urgent"}
-   {:id :unimportant :kind :annotation :action :unset :attr :report/important :scope :setter-or-maintainer
-    :syntax "Not important"}
+   {:id :unurgent    :kind :annotation :action :unset :attr :report/urgent    :scope :user
+    :syntax "Not urgent" :not-words :urgent}
+   {:id :unimportant :kind :annotation :action :unset :attr :report/important :scope :user
+    :syntax "Not important" :not-words :important}
    ;; Deadline / expiry / topic
    {:id :deadline    :kind :annotation :action :set-deadline   :attr :report/deadline :scope :user
     :syntax "Deadline" :param :date-or-duration :report-types #{:bug :patch :request}}
-   {:id :undeadline  :kind :annotation :action :unset-deadline :attr :report/deadline :scope :setter-or-maintainer
+   {:id :undeadline  :kind :annotation :action :unset-deadline :attr :report/deadline :scope :user
     :syntax "No deadline" :report-types #{:bug :patch :request}}
    {:id :expiry      :kind :annotation :action :set-expiry   :attr :report/expiry :scope :user
     :syntax "Expiry" :param :date-or-duration}
-   {:id :unexpiry    :kind :annotation :action :unset-expiry :attr :report/expiry :scope :setter-or-maintainer
+   {:id :unexpiry    :kind :annotation :action :unset-expiry :attr :report/expiry :scope :user
     :syntax "No expiry"}
    {:id :topic       :kind :annotation :action :set-topic   :attr :report/topic :scope :user
     :syntax "Topic" :param :word}
-   {:id :untopic     :kind :annotation :action :unset-topic :attr :report/topic :scope :setter-or-maintainer
+   {:id :untopic     :kind :annotation :action :unset-topic :attr :report/topic :scope :user
     :syntax "No topic"}
    ;; Supersedes -- inverse role of Superseded-by (posed on the replacement;
    ;; current = :rel/to, target = :rel/from = the closed report).
    {:id :supersedes   :kind :annotation :action :set-supersedes   :attr :rel/supersedes    :scope :user
     :syntax "Supersedes"     :param :message-id :report-types #{:bug :patch :request}}
-   {:id :unsupersedes :kind :annotation :action :unset-supersedes :attr :rel/supersedes-to :scope :setter-or-maintainer
+   {:id :unsupersedes :kind :annotation :action :unset-supersedes :attr :rel/supersedes-to :scope :user
     :syntax "Not supersedes" :param :message-id :report-types #{:bug :patch :request}}
    ;; Related-to -- neutral cross-reference (no closure, multi-target,
    ;; symmetric canonicalised by :rel/id).  Multi-target makes a clean
@@ -175,10 +200,12 @@
    :report/deadline      :report/deadline-value
    :report/expiry        :report/expiry-value})
 
-;; Commands accepting :scope :setter-or-maintainer:
+;; Commands that support a :setter-or-maintainer scope override:
 ;; - unset lines keyed on a setter-ref attribute, plus
 ;; - explicit relation-backed unsets (:unsuperseded-by, :unsupersedes,
 ;;   :unduplicate-of).
+;; All of them default to :scope :user; this set only matters when a
+;; source tightens one back via config.
 (def setter-scoped-command-ids
   (into #{:unsuperseded-by :unsupersedes :unduplicate-of}
         (comp (filter #(str/starts-with? (name (:action %)) "unset"))

src/bone/commands/registry.clj

@@ -24,7 +24,7 @@
 
 (def bone-format
   "BONE export format version. Bump when the JSON/Org export shape changes."
-  "0.9.4")
+  "0.9.5")
 
 (def bone-schema
   (edn/read-string (slurp (io/resource "bone-schema.edn"))))
@@ -329,6 +329,47 @@
   (some-> (or (:email/body-text email) (:email/body-text-from-html email))
           strip-signature))
 
+(def trailer-keys
+  "Person trailers collected from replies to a patch, b4-style:
+  lowercase key -> canonical capitalization.  Distinct from the BONE
+  command layer (`Acked-by:` as a maintainer proxy command): trailers
+  are collected verbatim from anyone, for clients to fold into the
+  commit message when applying the patch."
+  {"acked-by"        "Acked-by"
+   "reviewed-by"     "Reviewed-by"
+   "tested-by"       "Tested-by"
+   "reported-by"     "Reported-by"
+   "suggested-by"    "Suggested-by"
+   "signed-off-by"   "Signed-off-by"
+   "co-developed-by" "Co-developed-by"})
+
+(def ^:private patch-start-line-re
+  "First line of a patch pasted inline in a reply (mirrors detect's
+  inline-patch-start-patterns, which bone.detect cannot share without
+  inverting the dependency).  extract-trailers stops there, so a fixup
+  pasted below the reply text cannot leak its own Signed-off-by."
+  #"^(?:From [0-9a-f]{40} |diff --git |--- a/)")
+
+(defn extract-trailers
+  "Collect git person trailers from a reply body.
+  Returns a distinct vector of \"Key: Name <addr>\" strings for the
+  lines whose key starts at column 0 and is in `trailer-keys`, and
+  whose value contains an @ (so prose like \"Tested-by: nobody yet\"
+  is skipped).  Quoted lines never match, their key sitting behind
+  \">\".  Scanning stops at the first line of a pasted patch, the way
+  b4 does.  Keys are canonicalized, values trimmed.  Nil-safe on body."
+  [body]
+  (when body
+    (->> (str/split-lines body)
+         (take-while #(not (re-find patch-start-line-re %)))
+         (keep (fn [line]
+                 (when-let [[_ k v] (re-matches #"([A-Za-z][A-Za-z-]*):\s*(\S.*?)\s*" line)]
+                   (when-let [ck (get trailer-keys (str/lower-case k))]
+                     (when (str/includes? v "@")
+                       (str ck ": " v))))))
+         distinct
+         vec)))
+
 ;; ---------------------------------------------------------------------------
 ;; Date formatting
 ;; ---------------------------------------------------------------------------
@@ -696,6 +737,12 @@
 (def type->plural (into {} (map (juxt :type :plural)) report-type-spec))
 
 (def default-commands
+  ;; No bare "Reviewed" in :acked -- in loose mode a reply opener like
+  ;; "Reviewed v2 and found problems." would ack with inverse polarity
+  ;; (a review is not an approval, unlike Confirmed/Approved).  The
+  ;; kernel idiom is the full Reviewed-by: line, a syntax synonym of
+  ;; Acked-by: in the registry; sources that want the bareword can add
+  ;; it via :words.
   {:acked     ["Acked" "Confirmed" "Approved"]
    :owned     ["Owned"]
    :closed    ["Canceled" "Cancelled" "Closed" "Expired"
@@ -1104,6 +1151,7 @@
     {:report/expiry [:email/author-address :email/date-sent]}
     :report/expiry-value
     :report/has-ics :report/has-text-attachments
+    :report/trailers
     :report/last-activity :report/last-activity-address :report/descendants :report/updated-at
     {:rel/_from [:rel/kind :rel/active? :rel/setter :rel/posed-at :rel/value
                  {:rel/to [:db/id :report/type :report/message-id