From 1de0623168d13c51fb9a7c8a87f89030642ae4d0 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Mon, 25 May 2026 21:30:34 -0500 Subject: refactor(save): remove the immediate-state-push org-sync machinery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The opt-in org-sync mode pushed Linear state immediately — on every TODO-keyword cycle (the org-after-todo-state-change hook) and on every buffer save (the after-save hook scanning the whole file). That's the last immediate-push path, and it contradicts the one-write-path model the rest of v2 builds, so I removed it: pearl-enable-org-sync / -disable-org-sync, pearl-org-hook-function, pearl-sync-org-to-linear, pearl-sync-current-heading-to-linear, and the now-orphaned pearl--process-heading-at-point, pearl--map-org-state-to-linear, and pearl--update-issue-state-async. This finishes the "make org-sync private" task — there was nothing left to make private. Cycling a TODO keyword now marks state dirty (reconciled at the next save) instead of pushing. Saving the buffer pushes nothing. Re-introducing an auto-push on save is the separate "Automatic sync on save" task, which would call the save engine rather than this whole-file scanner. I kept pearl--extract-org-heading-properties and pearl--get-todo-states-pattern. They're general heading/keyword readers, not org-sync-specific, and the keyword-cycle save (the keyword-derivation follow-up) needs exactly them. The obsolete sync tests are replaced by two that assert the contract: the sync commands are gone, and a keyword cycle pushes nothing. --- pearl.el | 126 +------------------------------ tests/test-pearl-mapping.el | 20 +---- tests/test-pearl-org-parse.el | 35 ++------- tests/test-pearl-states.el | 36 +-------- tests/test-pearl-sync-hooks.el | 168 ++++++----------------------------------- 5 files changed, 39 insertions(+), 346 deletions(-) diff --git a/pearl.el b/pearl.el index 370ccb2..3b53556 100644 --- a/pearl.el +++ b/pearl.el @@ -1403,49 +1403,6 @@ Use after renaming things in Linear, or to force the next lookup to refetch." (message "Failed to update issue %s state" issue-id))) (message "Failed to update issue %s state: API error" issue-id)))))) -(defun pearl--update-issue-state-async (issue-id state-name team-id) - "Asynchronously update a Linear issue's state. -ISSUE-ID is the Linear issue ID. -STATE-NAME is the target state name to set. -TEAM-ID is the team ID of the issue. -Gives immediate feedback and performs the API update in the background." - (pearl--log "Asynchronously updating issue %s state to %s for team %s" issue-id state-name team-id) - - ;; Resolve the state name to an ID first; bail out clearly if the team has - ;; no such state rather than firing a mutation with a null stateId. - (let ((state-id (pearl--get-state-id-by-name state-name team-id))) - (if (null state-id) - (message "Cannot update issue %s: no Linear state named %s in team %s" - issue-id state-name team-id) - (message "Updating issue state to %s... (in background)" state-name) - (let* ((query "mutation UpdateIssueState($issueId: String!, $stateId: String!) { - issueUpdate(id: $issueId, input: {stateId: $stateId}) { - success - issue { - id - identifier - state { - id - name - } - } - } - }") - (variables `(("issueId" . ,issue-id) - ("stateId" . ,state-id))) - (success-handler (lambda (data) - (let ((success (and (assoc 'data data) - (assoc 'issueUpdate (assoc 'data data)) - (cdr (assoc 'success (assoc 'issueUpdate (assoc 'data data))))))) - (if success - (message "Successfully updated issue %s state to %s" issue-id state-name) - (pearl--log "Failed to update issue state asynchronously: %s" (prin1-to-string data)) - (message "Failed to update issue %s state in Linear" issue-id))))) - (error-handler (lambda (error-thrown _response _data) - (message "Error updating issue %s state in Linear: %s" issue-id error-thrown)))) - - (pearl--graphql-request-async query variables success-handler error-handler))))) - ;;; Team Member and Project Management (defun pearl-get-team-members (team-id) @@ -1552,17 +1509,6 @@ Gives immediate feedback and performs the API update in the background." ;;; Org Mode Integration -(defun pearl-org-hook-function () - "Sync to Linear when the configured Linear org file is saved. -Fires only for the buffer visiting `pearl-org-file-path', so a -custom output path is honored instead of a hardcoded \"linear.org\" name." - (when (and buffer-file-name - pearl-org-file-path - (string-equal (file-truename buffer-file-name) - (file-truename pearl-org-file-path))) - (pearl--log "Linear org file saved, syncing changes to Linear API") - (pearl-sync-org-to-linear))) - (defun pearl--extract-org-heading-properties () "Extract Linear issue properties from the org entry at point. Returns a plist with :todo-state, :issue-id, :issue-identifier, and :team-id, @@ -1580,49 +1526,6 @@ there is no network lookup here." :team-id (org-entry-get nil "LINEAR-TEAM-ID"))))) -(defun pearl--process-heading-at-point () - "Process the Linear issue at the current org heading." - (let* ((props (pearl--extract-org-heading-properties)) - (todo-state (plist-get props :todo-state)) - (issue-id (plist-get props :issue-id)) - (issue-identifier (plist-get props :issue-identifier)) - (team-id (plist-get props :team-id))) - - ;; Only sync when this heading is a Linear issue (has id, identifier, team). - (when (and issue-id issue-identifier team-id) - (let ((linear-state (pearl--map-org-state-to-linear todo-state))) - (when linear-state - (pearl--update-issue-state-async issue-id linear-state team-id)))))) - -(defun pearl-sync-org-to-linear () - "Syncs change from linear.org to Linear API." - (interactive) - ;; If called from org-after-todo-state-change-hook, just process the current heading - (if (eq this-command 'org-todo) - (pearl-sync-current-heading-to-linear) - ;; Otherwise, scan the entire file - (save-excursion - (goto-char (point-min)) - (let ((todo-states-pattern (pearl--get-todo-states-pattern))) - (while (re-search-forward (format "^\\*+ \\(%s\\)" todo-states-pattern) nil t) - ;; Process at the heading start, but keep the outer point at the end - ;; of this match so the next search advances past it. Without the - ;; save-excursion, `beginning-of-line' rewinds point and the search - ;; re-matches the same heading forever. - (save-excursion - (beginning-of-line) - (pearl--process-heading-at-point))))))) - -(defun pearl-sync-current-heading-to-linear () - "Sync the current org heading's TODO state to the Linear API. -Used when directly changing a TODO state in the org buffer." - (save-excursion - ;; Move up to the enclosing heading. `org-back-to-heading' signals - ;; "before first heading" in the preamble; guard so the sync entry point - ;; degrades to a no-op there instead of erroring. - (when (ignore-errors (org-back-to-heading t) t) - (pearl--process-heading-at-point)))) - ;;; Mapping Functions (defun pearl--map-linear-state-to-org (state) @@ -1631,13 +1534,6 @@ STATE is the Linear state string." (or (cdr (assoc state pearl-state-to-todo-mapping)) "TODO")) ; Default fallback -(defun pearl--map-org-state-to-linear (todo-state) - "Map an Org TODO-STATE keyword to a Linear state name. -TODO-STATE is the Org keyword string." - (or (car (rassoc todo-state pearl-state-to-todo-mapping)) - nil)) - - (defun pearl--get-todo-states-pattern () "Return the regex pattern matching the Org TODO states. Built from the org keywords in `pearl-state-to-todo-mapping' and @@ -4395,25 +4291,6 @@ resolves to a single label." (message "Loaded Linear API key from LINEAR_API_KEY environment variable")) (message "LINEAR_API_KEY environment variable not found or empty")))) -;;; Org Mode Sync Hooks - -;;;###autoload -(defun pearl-enable-org-sync () - "Enable synchronization between org mode and Linear." - (interactive) - (add-hook 'after-save-hook #'pearl-org-hook-function nil t) - (add-hook 'org-after-todo-state-change-hook #'pearl-sync-org-to-linear nil t) - (pearl-highlight-comments) - (message "Linear-org synchronization enabled")) - -;;;###autoload -(defun pearl-disable-org-sync () - "Disable synchronization between org mode and Linear." - (interactive) - (remove-hook 'after-save-hook #'pearl-org-hook-function t) - (remove-hook 'org-after-todo-state-change-hook #'pearl-sync-org-to-linear t) - (message "Linear-org synchronization disabled")) - ;;; Comment Editing (defface pearl-editable-comment @@ -4522,8 +4399,7 @@ The viewer's own comments get `pearl-editable-comment'; all others get (defun pearl-highlight-comments () "Color comment headings in the current buffer by who can edit them. The viewer's own comments render green (editable); others render greyed. Runs -after a fetch/refresh and from `pearl-enable-org-sync', and is safe to -invoke by hand." +after a fetch/refresh, and is safe to invoke by hand." (interactive) (let ((buffer (current-buffer))) ;; Best-effort: highlighting is a display nicety and must never abort the diff --git a/tests/test-pearl-mapping.el b/tests/test-pearl-mapping.el index 17eff82..2a96163 100644 --- a/tests/test-pearl-mapping.el +++ b/tests/test-pearl-mapping.el @@ -54,22 +54,10 @@ (test-pearl--with-mapping '(("Todo" . "TODO")) (should (string-equal "TODO" (pearl--map-linear-state-to-org nil))))) -;;; pearl--map-org-state-to-linear - -(ert-deftest test-pearl-map-org-state-to-linear-mapped-returns-state () - "An org keyword present in the mapping returns its Linear state." - (test-pearl--with-mapping '(("Todo" . "TODO") ("In Progress" . "DOING")) - (should (string-equal "In Progress" (pearl--map-org-state-to-linear "DOING"))))) - -(ert-deftest test-pearl-map-org-state-to-linear-unmapped-returns-nil () - "An org keyword not in the mapping returns nil." - (test-pearl--with-mapping '(("Todo" . "TODO") ("Done" . "DONE")) - (should (null (pearl--map-org-state-to-linear "WAITING"))))) - -(ert-deftest test-pearl-map-org-state-to-linear-nil-returns-nil () - "A nil org keyword returns nil." - (test-pearl--with-mapping '(("Todo" . "TODO")) - (should (null (pearl--map-org-state-to-linear nil))))) +;; pearl--map-org-state-to-linear (the keyword->state reverse map) was removed +;; with the org-sync push path in save-model-v2; the keyword-cycle save will +;; resolve through the team's states (the keyword-derivation task), not a flat +;; reverse mapping. ;;; pearl--get-todo-states-pattern diff --git a/tests/test-pearl-org-parse.el b/tests/test-pearl-org-parse.el index 87f4a02..e0bb967 100644 --- a/tests/test-pearl-org-parse.el +++ b/tests/test-pearl-org-parse.el @@ -19,12 +19,10 @@ ;;; Commentary: -;; Tests for the org readers `pearl--extract-org-heading-properties' and -;; `pearl--process-heading-at-point'. The reader uses org APIs over the -;; LINEAR-* property drawer, so it works from anywhere in the entry, at any -;; heading depth, and is unbothered by body text or nested sub-entries. The one -;; network boundary reached during processing (`--update-issue-state-async') is -;; stubbed. +;; Tests for the org reader `pearl--extract-org-heading-properties'. It uses +;; org APIs over the LINEAR-* property drawer, so it works from anywhere in the +;; entry, at any heading depth, and is unbothered by body text or nested +;; sub-entries. ;;; Code: @@ -100,29 +98,8 @@ (goto-char (point-min)) (should (null (pearl--extract-org-heading-properties))))) -;;; process-heading-at-point - -(ert-deftest test-pearl-process-heading-updates-when-complete () - "A complete entry triggers an async state update with the mapped state." - (let ((captured nil)) - (cl-letf (((symbol-function 'pearl--update-issue-state-async) - (lambda (id state team) (setq captured (list id state team))))) - (test-pearl--in-org - "*** IN-PROGRESS x\n:PROPERTIES:\n:LINEAR-ID: i-1\n:LINEAR-IDENTIFIER: ENG-2\n:LINEAR-TEAM-ID: t-1\n:END:\n" - (re-search-forward "x") - (pearl--process-heading-at-point) - (should (equal '("i-1" "In Progress" "t-1") captured)))))) - -(ert-deftest test-pearl-process-heading-skips-without-team () - "An entry missing its team id makes no API call." - (let ((called nil)) - (cl-letf (((symbol-function 'pearl--update-issue-state-async) - (lambda (&rest _) (setq called t)))) - (test-pearl--in-org - "*** TODO x\n:PROPERTIES:\n:LINEAR-ID: i-1\n:LINEAR-IDENTIFIER: ENG-2\n:END:\n" - (re-search-forward "x") - (pearl--process-heading-at-point) - (should-not called))))) +;; process-heading-at-point (the org-sync per-heading state push) was removed +;; in save-model-v2; the extractor above is retained for the keyword-cycle save. ;;; render -> parse round trip diff --git a/tests/test-pearl-states.el b/tests/test-pearl-states.el index 713cb78..ae8ef63 100644 --- a/tests/test-pearl-states.el +++ b/tests/test-pearl-states.el @@ -97,38 +97,10 @@ (pearl-update-issue-state "i-1" "Done" "team-1") (should requested)))) -;;; pearl--update-issue-state-async -- nil-state-id guard - -(ert-deftest test-pearl-update-issue-state-async-nil-state-id-skips-request () - "The async update also short-circuits when the state name doesn't resolve." - (let ((requested nil) - (pearl-api-key "test-key")) - (cl-letf (((symbol-function 'pearl--get-state-id-by-name) (lambda (_s _t) nil)) - ((symbol-function 'request) (lambda (&rest _) (setq requested t)))) - (pearl--update-issue-state-async "i-1" "Nonexistent" "team-1") - (should-not requested)))) - -(ert-deftest test-pearl-update-issue-state-async-success-runs () - "The async success handler runs on a successful update." - (let ((pearl-api-key "test-key")) - (cl-letf (((symbol-function 'pearl--get-state-id-by-name) (lambda (_s _t) "s2")) - ((symbol-function 'request) - (lambda (_url &rest args) - (funcall (plist-get args :success) - :data '((data (issueUpdate (success . t)))))))) - (should (progn (pearl--update-issue-state-async "i-1" "Done" "team-1") t))))) - -(ert-deftest test-pearl-update-issue-state-async-error-runs () - "The async error handler runs on a transport error." - (let ((pearl-api-key "test-key")) - (cl-letf (((symbol-function 'pearl--get-state-id-by-name) (lambda (_s _t) "s2")) - ((symbol-function 'request) - (lambda (_url &rest args) - (funcall (plist-get args :error) - :error-thrown "boom" - :response (make-request-response :status-code 500) - :data nil)))) - (should (progn (pearl--update-issue-state-async "i-1" "Done" "team-1") t))))) +;; pearl--update-issue-state-async (the by-name state push) was removed with +;; the org-sync path in save-model-v2. State now pushes through the save +;; engine's pearl--save-state-field -> pearl--update-issue-async (stateId), +;; covered in test-pearl-save. (provide 'test-pearl-states) ;;; test-pearl-states.el ends here diff --git a/tests/test-pearl-sync-hooks.el b/tests/test-pearl-sync-hooks.el index 05864e7..0ee7d82 100644 --- a/tests/test-pearl-sync-hooks.el +++ b/tests/test-pearl-sync-hooks.el @@ -19,157 +19,37 @@ ;;; Commentary: -;; Tests for the org sync hook wiring: enable/disable add and remove -;; buffer-local hooks; the after-save hook only fires for linear.org buffers; -;; and per-heading sync degrades gracefully when point is before any heading. +;; Save-model-v2 removed the opt-in org-sync mode. The after-save and +;; TODO-cycle hooks that pushed Linear state immediately are gone -- every push +;; now goes through the save engine (pearl-save-issue / pearl-save-all). These +;; tests assert the removal: the sync commands no longer exist, and cycling a +;; TODO keyword pushes nothing. ;;; Code: (require 'test-bootstrap (expand-file-name "test-bootstrap.el")) (require 'cl-lib) -;;; enable / disable - -(ert-deftest test-pearl-enable-org-sync-adds-buffer-local-hooks () - "Enabling sync adds both hook functions buffer-locally." - (with-temp-buffer - (pearl-enable-org-sync) - (should (memq 'pearl-org-hook-function after-save-hook)) - (should (memq 'pearl-sync-org-to-linear org-after-todo-state-change-hook)))) - -(ert-deftest test-pearl-disable-org-sync-removes-hooks () - "Disabling sync removes both hook functions." - (with-temp-buffer - (pearl-enable-org-sync) - (pearl-disable-org-sync) - (should-not (memq 'pearl-org-hook-function after-save-hook)) - (should-not (memq 'pearl-sync-org-to-linear org-after-todo-state-change-hook)))) - -;;; org-hook-function buffer guard - -(ert-deftest test-pearl-org-hook-function-skips-other-buffer () - "The after-save hook does nothing in a buffer that isn't the configured file." - (let ((called nil) - (pearl-org-file-path "/tmp/linear.org")) - (cl-letf (((symbol-function 'pearl-sync-org-to-linear) (lambda () (setq called t)))) - (with-temp-buffer - (setq buffer-file-name "/tmp/scratch.org") - (pearl-org-hook-function) - (should-not called))))) - -(ert-deftest test-pearl-org-hook-function-syncs-configured-buffer () - "The after-save hook syncs when the buffer visits `pearl-org-file-path'." - (let ((called nil) - (pearl-org-file-path "/tmp/linear.org")) - (cl-letf (((symbol-function 'pearl-sync-org-to-linear) (lambda () (setq called t)))) - (with-temp-buffer - (setq buffer-file-name "/tmp/linear.org") - (pearl-org-hook-function) - (should called))))) - -(ert-deftest test-pearl-org-hook-function-honors-custom-path () - "A non-default `pearl-org-file-path' is what the hook matches on. -Regression: the hook used to hardcode a \"linear.org$\" regex, so a buffer -named linear.org fired even when the configured file was elsewhere, and a -custom-named configured file never fired." - (let ((called nil) - (pearl-org-file-path "/tmp/my-linear-issues.org")) - (cl-letf (((symbol-function 'pearl-sync-org-to-linear) (lambda () (setq called t)))) - ;; A buffer literally named linear.org must NOT fire when the configured - ;; file is something else. - (with-temp-buffer - (setq buffer-file-name "/tmp/linear.org") - (pearl-org-hook-function) - (should-not called)) - ;; The configured custom-named file DOES fire. - (with-temp-buffer - (setq buffer-file-name "/tmp/my-linear-issues.org") - (pearl-org-hook-function) - (should called))))) - -(ert-deftest test-pearl-org-hook-function-matches-through-symlink () - "A configured path and a visited symlink to the same file match via truename. -The hook resolves both sides with `file-truename', so a symlink to the -configured file still syncs -- this guards the choice of truename over a raw -string compare." - (let ((real (make-temp-file "linear-real-" nil ".org")) - (link (make-temp-file "linear-link-" nil ".org")) - (called nil)) - (unwind-protect - (progn - (delete-file link) - (make-symbolic-link real link) - (let ((pearl-org-file-path real)) - (cl-letf (((symbol-function 'pearl-sync-org-to-linear) - (lambda () (setq called t)))) - (with-temp-buffer - (setq buffer-file-name link) - (pearl-org-hook-function) - (should called))))) - (when (file-exists-p link) (delete-file link)) - (when (file-exists-p real) (delete-file real))))) - -(ert-deftest test-pearl-org-hook-function-nil-path-no-op () - "With `pearl-org-file-path' nil, the hook is a no-op and does not error." - (let ((called nil) - (pearl-org-file-path nil)) - (cl-letf (((symbol-function 'pearl-sync-org-to-linear) (lambda () (setq called t)))) - (with-temp-buffer - (setq buffer-file-name "/tmp/linear.org") - (should (progn (pearl-org-hook-function) t)) - (should-not called))))) - -;;; sync-current-heading-to-linear - -(ert-deftest test-pearl-sync-current-heading-before-first-heading-no-error () - "Syncing with point before the first heading degrades gracefully. - -Regression: `org-back-to-heading' signals \"before first heading\" in the -preamble, which must not propagate out of the sync entry point." - (cl-letf (((symbol-function 'pearl--process-heading-at-point) (lambda () nil))) - (with-temp-buffer - (insert "#+TITLE: x\n\npreamble line\n") - (org-mode) - (goto-char (point-min)) - (should (progn (pearl-sync-current-heading-to-linear) t))))) - -(ert-deftest test-pearl-sync-current-heading-processes-on-heading () - "Syncing from within an entry processes that heading." - (let ((called nil)) - (cl-letf (((symbol-function 'pearl--process-heading-at-point) - (lambda () (setq called t)))) - (with-temp-buffer - (insert "* Top\n*** TODO x\n") - (org-mode) - (goto-char (point-max)) - (pearl-sync-current-heading-to-linear) - (should called))))) - -;;; sync-org-to-linear dispatcher - -(ert-deftest test-pearl-sync-org-to-linear-org-todo-syncs-current-heading () - "When invoked from `org-todo', only the current heading is synced." - (let ((called nil) - (this-command 'org-todo)) - (cl-letf (((symbol-function 'pearl-sync-current-heading-to-linear) - (lambda () (setq called t)))) - (pearl-sync-org-to-linear) - (should called)))) - -(ert-deftest test-pearl-sync-org-to-linear-otherwise-scans-whole-file () - "Outside `org-todo', every matching heading in the buffer is processed." - (let ((count 0) - (this-command 'some-other-command) - (pearl-state-to-todo-mapping '(("Todo" . "TODO") ("Done" . "DONE"))) - (pearl-todo-states-pattern nil) - (pearl--todo-states-pattern-source nil)) - (cl-letf (((symbol-function 'pearl--process-heading-at-point) - (lambda () (setq count (1+ count))))) +(ert-deftest test-pearl-org-sync-commands-removed () + "The opt-in org-sync mode and its push machinery no longer exist." + (should-not (fboundp 'pearl-enable-org-sync)) + (should-not (fboundp 'pearl-disable-org-sync)) + (should-not (fboundp 'pearl-org-hook-function)) + (should-not (fboundp 'pearl-sync-org-to-linear)) + (should-not (fboundp 'pearl-sync-current-heading-to-linear))) + +(ert-deftest test-pearl-keyword-cycle-pushes-nothing () + "Cycling a TODO keyword pushes nothing -- pearl wires no TODO-cycle sync hook." + (let ((pushed nil)) + (cl-letf (((symbol-function 'pearl--update-issue-async) + (lambda (&rest _) (setq pushed t)))) (with-temp-buffer - (insert "*** TODO a\n*** DONE b\n") - (org-mode) - (pearl-sync-org-to-linear) - (should (= 2 count)))))) + (let ((org-todo-keywords '((sequence "TODO" "DONE")))) + (insert "*** TODO Title\n:PROPERTIES:\n:LINEAR-ID: a\n:END:\n") + (org-mode) + (goto-char (point-min)) + (org-todo "DONE")) + (should-not pushed))))) (provide 'test-pearl-sync-hooks) ;;; test-pearl-sync-hooks.el ends here -- cgit v1.2.3