fix(dsl): derive action CPN vars from incoming arcs (ground truth)

The previous compile_transition_action/2 inferred which keys to
`Keyword.fetch!/2` out of `event.binding` from `ExpressionHelper.free_vars/1`
on the action body. That over-approximates: any locally-introduced name
inside the body (a literal `pid = options[:test_pid]` or a pattern-match
temporary inside an `if`/`case`) was treated as a CPN var, so
`Keyword.fetch!(event.binding, :pid)` raised `KeyError`, the spawned
`Task` crashed, and `LifecycleHooks.safe_invoke/3` swallowed it — the
action silently never ran.

Switch the injection set to ground truth: at `__close_transition__!`
time we already know which CPN variables this transition's incoming
arcs bind. Use that list, drop the body-free-var step from the action
macro entirely, and let the body's pattern-match locals stay local.

To keep the compiler quiet under `--warnings-as-errors` when the body
only references a subset of the bound vars, every fetched binding gets
a paired `_used = var` discard. The injection still happens uniformly
(simpler reasoning, matches "all CPN vars are available") but unused
vars never trigger the unused-variable lint.

Drive-by: rename the misleading `describe "lifecycle dispatch (bare
module hooks)"` block to `"lifecycle dispatch (hooks options carry
test_pid only)"` — the test does not exercise the bare-module path
(that's covered by `LifecycleHooks.validate!/1` unit tests). Revert the
two-step `pid = options[:test_pid]; if pid do` workaround in the e2e
test back to the natural inline `if pid = options[:test_pid] do …` form
now that local pattern-match names no longer collide with CPN-var
injection.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: fahchen

docs/tmp/pr-comments.md

@@ -0,0 +1,61 @@
+# PR #89 — Unresolved Comments
+
+Latest commit: `5475998`. 7 unresolved (5 stale copilot + 2 new fahchen).
+
+## Open
+
+### 1. `test/coloured_flow/dsl/listener_e2e_test.exs` (file-level) — @copilot
+> setup_flow!/0 not idempotent vs PR description.
+
+Thread: `PRRT_kwDOMZc9dM5_KOFJ` / Comment: `3177549638`
+**Status:** keep  **Decision:** stale — file renamed to `lifecycle_hooks_e2e_test.exs` and `setup_flow!` removed entirely in `5475998` (Q1).
+
+---
+
+### 2. `lib/coloured_flow/dsl/spec.md` (file-level) — @copilot
+> ActionHandler vs Listener naming mismatch.
+
+Thread: `PRRT_kwDOMZc9dM5_KOFW` / Comment: `3177549653`
+**Status:** keep  **Decision:** stale — module renamed `ActionHandler → Listener → LifecycleHooks` (final). 7 callbacks (`on_workitem_reoffered` dropped). spec.md aligned in `5475998`. PR title/description need manual update.
+
+---
+
+### 3. `lib/coloured_flow/dsl/builder.ex` (file-level) — @copilot
+> setup_flow!/0 doc says raise on missing name but impl doesn't validate.
+
+Thread: `PRRT_kwDOMZc9dM5_KOFh` / Comment: `3177549664`
+**Status:** keep  **Decision:** stale — `setup_flow!/0` removed entirely in `5475998`. DSL no longer inserts flows.
+
+---
+
+### 4. `lib/coloured_flow/runner/enactment/enactment.ex` #310 — @copilot
+> apply_calibration dispatches workitem callbacks before on_enactment_start during boot.
+
+Thread: `PRRT_kwDOMZc9dM5_KOFo` / Comment: `3177549671`
+**Status:** keep  **Decision:** principle is "each callback fires after its own telemetry event"; current code obeys that. Cross-callback ordering (workitem_enabled before enactment_start) is acceptable because each callback follows its own emit point.
+
+---
+
+### 5. `lib/coloured_flow/dsl/storage.ex` (file-level) — @copilot
+> Idempotency claim mismatch.
+
+Thread: `PRRT_kwDOMZc9dM5_KOFq` / Comment: `3177549673`
+**Status:** keep  **Decision:** stale — file `lib/coloured_flow/dsl/storage.ex` deleted entirely in `5475998` (Q1).
+
+---
+
+### 6. `lib/coloured_flow/runner/enactment/enactment.ex` (file-level) — @fahchen
+> 这个文件太大了，可以把新增的部分抽象出来
+
+Thread: `PRRT_kwDOMZc9dM5_Kedo` / Comment: `3177634901`
+**Status:** fix  **Decision:** extract `dispatch_lifecycle/3` + `dispatch_post_calibration/3` into new module `ColouredFlow.Runner.Enactment.LifecycleHooks.Dispatcher`. API takes `%Enactment{}` state struct directly (internal helper, coupling acceptable). `:abnormal_exit` line stays in `terminate/2`.
+
+---
+
+### 7. `test/coloured_flow/dsl/lifecycle_hooks_e2e_test.exs` #37 — @fahchen
+> 通过 options 传进来，而不是 persistent term
+
+Thread: `PRRT_kwDOMZc9dM5_KfCI` / Comment: `3177637783`
+**Status:** fix  **Decision:** drop `:persistent_term` test-pid hack. Pass `test_pid` via `lifecycle_hooks: {Workflow, [test_pid: self()]}` options. DSL bodies read `options[:test_pid]` and send. Apply same pattern to `lifecycle_hooks_dispatch_test.exs` (also uses persistent_term).
+
+---

lib/coloured_flow/dsl/builder.ex

@@ -383,20 +383,43 @@ defmodule ColouredFlow.DSL.Builder do
   #   * `event`   — `LifecycleHooks.workitem_completed_event()` map carrying
   #     `:enactment_id`, `:markings`, `:workitem`, `:occurrence`, `:binding`.
   #   * `options` — keyword list registered alongside the hook module.
-  #   * each free CPN variable (`:s`, `:x`, …) — plucked from `event.binding`.
+  #   * each CPN variable bound by the transition's incoming arcs (`:s`,
+  #     `:x`, …) — plucked from `event.binding`. Body-local pattern-match
+  #     names are NOT injected, so writing `pid = options[:pid]` (or any
+  #     local) inside `action do ... end` is safe.
   #
-  # The wrapper unpacks `event.binding` via `Keyword.fetch!/2` for each declared
-  # free var and runs the body inside a `Task.Supervisor.start_child/2` call
-  # (or unsupervised `Task.start/1` when no `:task_supervisor` was provided to
+  # The wrapper unpacks `event.binding` via `Keyword.fetch!/2` for each
+  # transition-bound var (using a `_`-prefixed alias when the body does not
+  # reference the var, to suppress unused-variable warnings) and runs the
+  # body inside a `Task.Supervisor.start_child/2` call (or unsupervised
+  # `Task.start/1` when no `:task_supervisor` was provided to
   # `use ColouredFlow.DSL`) so the runner never blocks on user side effects.
-  defp compile_transition_action({transition_name, body, cpn_vars}, task_supervisor) do
+  defp compile_transition_action(
+         {transition_name, body, incoming_vars},
+         task_supervisor
+       ) do
+    # `incoming_vars` is the ground-truth set of CPN variables this
+    # transition's incoming arcs bind — we always fetch each one from
+    # `event.binding` and immediately discard the binding via `_ = var`
+    # so a body that only references a subset of the bound vars does
+    # not trip `--warnings-as-errors`. Body-local pattern-match names
+    # (`pid = options[:pid]`, etc.) are NOT in this list and are never
+    # touched.
     var_assignments =
-      Enum.map(cpn_vars, fn var ->
+      Enum.flat_map(incoming_vars, fn var ->
         var_ast = Macro.var(var, nil)
 
-        quote do
-          unquote(var_ast) = Keyword.fetch!(var!(event).binding, unquote(var))
-        end
+        [
+          quote do
+            unquote(var_ast) = Keyword.fetch!(var!(event).binding, unquote(var))
+          end,
+          quote do
+            # Discard expression so a body that doesn't reference the var
+            # doesn't trip `--warnings-as-errors`. The underscore prefix
+            # satisfies the project's unused-variable naming convention.
+            _used = unquote(var_ast)
+          end
+        ]
       end)
 
     task_call = wrap_in_task(body, task_supervisor)

lib/coloured_flow/dsl/transition.ex

@@ -17,10 +17,6 @@ defmodule ColouredFlow.DSL.Transition do
 
   @scope_attr :__cf_transition_scope__
 
-  # Magic bindings made available inside an `action do ... end` body, in
-  # addition to any free CPN variables resolved from the transition's binding.
-  @action_magic_bindings [:event, :options]
-
   @doc """
   Declare a transition. The block accepts `guard/1`, `action/1`, `input/2,3` and
   `output/2,3`.
@@ -118,19 +114,12 @@ defmodule ColouredFlow.DSL.Transition do
     caller_line = __CALLER__.line
     expr_ast = ExpressionHelper.block_to_ast(expression)
     code = ExpressionHelper.ast_to_code(expr_ast)
-
-    raw_free = ExpressionHelper.free_vars(expr_ast)
-    cpn_vars = raw_free -- @action_magic_bindings
     escaped_ast = Macro.escape(expr_ast)
 
     quote do
       ColouredFlow.DSL.Transition.__set_action__!(
         __MODULE__,
-        %{
-          code: unquote(code),
-          body: unquote(escaped_ast),
-          cpn_vars: unquote(cpn_vars)
-        },
+        %{code: unquote(code), body: unquote(escaped_ast)},
         unquote(caller_file),
         unquote(caller_line)
       )
@@ -183,11 +172,22 @@ defmodule ColouredFlow.DSL.Transition do
     Module.put_attribute(module, :cf_transitions, transition)
 
     case scope.action do
-      %{body: body, cpn_vars: cpn_vars} ->
+      %{body: body} ->
+        # Ground truth: the variables that this transition's incoming arcs
+        # actually bind. The injection set comes from this list (not from
+        # body free-var inference), so locally-introduced names
+        # (`pid = options[:test_pid]`, match-pattern temporaries inside
+        # the body) never get `Keyword.fetch!`-ed out of `event.binding`.
+        incoming_vars =
+          arcs
+          |> Stream.filter(&match?(%Arc{orientation: :p_to_t}, &1))
+          |> Stream.flat_map(& &1.expression.vars)
+          |> Enum.uniq()
+
         Module.put_attribute(
           module,
           :cf_transition_actions,
-          {scope.name, body, cpn_vars}
+          {scope.name, body, incoming_vars}
         )
 
       _other ->

test/coloured_flow/dsl/lifecycle_hooks_e2e_test.exs

@@ -24,13 +24,7 @@ defmodule ColouredFlow.DSL.LifecycleHooksE2ETest do
       output :output, {1, x}
 
       action do
-        # NOTE: bind in two steps. The action macro's free-var analysis treats
-        # an inline `if pid = options[...] do ... end` as a CPN var (`:pid`)
-        # and tries to fetch it from `event.binding`, which crashes the Task
-        # silently. A separate assignment statement avoids that.
-        pid = options[:test_pid]
-
-        if pid do
+        if pid = options[:test_pid] do
           send(pid, {:action_fired, x, event.enactment_id, event.workitem.id, options})
         end
       end

test/coloured_flow/runner/enactment/lifecycle_hooks_dispatch_test.exs

@@ -63,7 +63,7 @@ defmodule ColouredFlow.Runner.Enactment.LifecycleHooksDispatchTest do
     %{cpnet: cpnet}
   end
 
-  describe "lifecycle dispatch (bare module hooks)" do
+  describe "lifecycle dispatch (hooks options carry test_pid only)" do
     setup :setup_flow
     setup :setup_enactment