refactor(slack): resolve the reviewer through the org directory (#26)

The CLI resolves runners and reviewers to stable directory ids, but the
Slack path recorded the raw Slack handle — the same person clicking in
Slack and typing 'flightdeck feedback' became two reviewers to the
KPIs (the asymmetry was even acknowledged in apply_interaction's
docstring). apply_interaction now resolves the Slack user through
org.directory with the same precedence as the CLI; no match keeps the
raw string, so orgs without a directory are byte-identical.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: sturlese

docs/governance.md

@@ -108,8 +108,9 @@ click back into the same measurement `flightdeck feedback` records.
 - **One feedback path.** The CLI command and the Slack handler both call a single
   `record_feedback(...)` function, so a button click lands the *identical* store row and the
   *identical* `feedback_recorded` ledger event (`{run_id, outcome, human_minutes, by}`) — the
-  Slack `by` is the reviewer's Slack handle, with a `via slack` note on the store row for
-  provenance. There is no second, weaker feedback API to keep in sync.
+  Slack `by` is resolved through the org directory exactly like the CLI reviewer (the stable
+  id when the handle matches, the raw Slack handle otherwise), with a `via slack` note on the
+  store row for provenance. There is no second, weaker feedback API to keep in sync.
 - **Offline-first, no new dependency.** `flightdeck slack post <run_id>` renders a Slack Block
   Kit message and, by default, **prints the JSON** — fully demoable and pipeable to any poster.
   Only when `FLIGHTDECK_SLACK_WEBHOOK` is set does it actually POST, via stdlib `urllib`

src/flightdeck/integrations/slack.py

@@ -263,20 +263,23 @@ def apply_interaction(
     """Parse a Slack interaction and record it through the shared feedback path.
 
     An explicit ``minutes`` wins; otherwise a modal's minutes are used; otherwise
-    ``None`` (metrics fall back to ``org.config.default_review_minutes``). ``org``
-    is accepted for parity with the CLI and future directory resolution — today
-    ``by`` is simply the Slack user string. Raises ``SlackError`` on a bad payload
-    and ``FeedbackError`` (from ``record_feedback``) on an unknown run.
+    ``None`` (metrics fall back to ``org.config.default_review_minutes``). The
+    Slack user is resolved through the org directory exactly like the CLI's
+    reviewer — the stable id when the handle matches, the raw Slack string
+    otherwise — so one person clicking in Slack and typing in a terminal stays
+    ONE reviewer to the KPIs. Raises ``SlackError`` on a bad payload and
+    ``FeedbackError`` (from ``record_feedback``) on an unknown run.
     """
     parsed = parse_interaction(payload)
     effective_minutes = minutes if minutes is not None else parsed.minutes
+    resolved = org.directory.resolve(parsed.user)
     return record_feedback(
         store,
         ledger,
         parsed.run_id,
         parsed.outcome,
         human_minutes=effective_minutes,
-        by=parsed.user,
+        by=resolved.id if resolved is not None else parsed.user,
         note="via slack",
     )
 

tests/test_slack.py

@@ -30,7 +30,7 @@
 from flightdeck.runner import execute
 from flightdeck.schemas import Run
 from flightdeck.store import Store
-from tests.conftest import NOW
+from tests.conftest import NOW, write_org
 
 runner = CliRunner()
 
@@ -231,6 +231,39 @@ def test_apply_interaction_uses_modal_minutes(org, store, ledger):
     assert fb.outcome == "edited" and fb.human_minutes == 9.0
 
 
+def test_apply_interaction_resolves_reviewer_through_the_directory(tmp_path):
+    # A click by Slack handle and a CLI review by alias must land on the SAME
+    # stable id — one person, one reviewer to the KPIs (same rule as `flightdeck
+    # feedback`, which already resolves --by through the directory).
+    directory = {
+        "provider": "azure_ad",
+        "users": [
+            {
+                "id": "AAD-42", "display_name": "Ana García",
+                "email": "ana.garcia@example.com", "department": "Support",
+                "aliases": ["ana", "ana.g"],
+            }
+        ],
+    }
+    org = load_org(write_org(tmp_path / "org", directory=directory))
+    with Store(org.db_path) as store:
+        ledger = Ledger(org.ledger_path)
+        run = _seed_run(org, store, ledger)
+        payload = _button_payload(json.dumps({"run_id": run.id, "outcome": "accepted"}), user="ana.g")
+        fb = apply_interaction(payload, store, ledger, org)
+
+    assert fb.by == "AAD-42"  # the stable id, not the raw Slack handle
+    assert ledger.entries()[-1]["data"]["by"] == "AAD-42"
+
+
+def test_apply_interaction_keeps_unknown_reviewers_raw(org, store, ledger):
+    # No directory (or no match) → the raw Slack string, exactly as before.
+    run = _seed_run(org, store, ledger)
+    payload = _button_payload(json.dumps({"run_id": run.id, "outcome": "accepted"}), user="stranger")
+    fb = apply_interaction(payload, store, ledger, org)
+    assert fb.by == "stranger"
+
+
 # ------------------------------------------------------------------------ transport