v1: stable public API

[myzie]

Summary

Takes the workflow library from its pre-v1 shape to a stable v1 surface.

Based on the public API review and implementation plan.

What changed (13 PRs)

• Path → Branch rename — every public identifier using "path" for an execution thread is now "branch" (PR1: rename Path → Branch #15)
• Surface shrink — removed types/methods with a single internal caller or no test coverage (PR2: surface shrink #16)
• Validation at construction — workflow.New now rejects invalid step kinds, bad modifiers, and structural errors upfront instead of at runtime (PR3: validation phase 1 + step kinds + StartAt #22)
• ActivityRegistry + functional options + single Execute — one entry point (Execute), NewExecution(wf, reg, ...ExecutionOption), deleted Run/Resume/RunOrResume/ExecuteOrResume (PR4: ActivityRegistry, functional options, single Execute #23)
• Binding validation — NewExecution validates activity bindings before execution starts (PR5: binding validation in NewExecution #24)
• Idiomatic Context — dropped Get prefixes, moved Wait/History/ReportProgress onto the interface, added FakeContext for testing (PR6: Context becomes idiomatic Go #25)
• Checkpoint stable wire format — Checkpoint is opaque; accessor methods replace direct field access (PR7: checkpoint stable wire format #26)
• Single template syntax — ${...} only; deleted $(...) (PR8: single template syntax #27)
• Activities tier split — core (activities/), contrib (activities/contrib/), http (activities/httpx/) (PR9: activities tier split + naming #28)
• Child workflow fixes — cleaned up child workflow activity and state handling (PR10: child workflow fixes #29)
• Error model + completion hook godoc — clarified error types and hook documentation (PR11: error model + completion hook godoc #30)
• ExecutionResult helpers — convenience methods on *ExecutionResult (PR12: ExecutionResult helpers #31)
• Documentation rewrite — README, llms.txt, examples, MIGRATION.md, suspension docs, production checklist (PR13: documentation rewrite #32)

Stats

113 files changed, +8853 / −5270 lines

Migration

See MIGRATION.md for every breaking change with before/after snippets.

Test plan

• make test-all passes (tests + vet)
• All examples compile and run
• Review MIGRATION.md against any internal consumers before tagging

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Branch-based execution model and explicit step kinds (Join, WaitSignal, Sleep, Pause)
  • Activity registry and option-style execution/runner configuration
  • Checkpoint schema versioning; richer execution result helpers (typed output, suspension queries)

• Breaking Changes

  • Template syntax standardized to ${...}; parameter/Store keys now branch-local (no "state." prefix)
  • Child workflow calls are synchronous by default; timeouts use time.Duration
  • Activities reorganized into core vs contrib tiers; legacy wait activity removed

• Documentation

  • Added production checklist, suspension/resume, migration guide, and examples updated to the new model

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a918ac70-d68a-4c4c-b505-1c91249cdc33

📥 Commits

Reviewing files that changed from the base of the PR and between a46278d and f1fd4b5.

📒 Files selected for processing (20)

• README.md
• activity_registry.go
• branch_join_test.go
• branch_local_state.go
• checkpoint.go
• checkpoint_test.go
• checkpointer_file.go
• child_workflow.go
• cmd/workflow/main.go
• coverage_test.go
• docs/production_checklist.md
• examples/branching/main.go
• examples/error_handling/main.go
• examples/join_branches/main.go
• examples/retry_simple/main.go
• execution.go
• pause.go
• script/eval.go
• workflowtest/memory_checkpointer.go
• workflowtest/workflowtest_test.go

---

📝 Walkthrough

Walkthrough

This PR migrates the runtime model from "paths" to "branches", consolidates execution APIs to registry+functional options, adds checkpoint schema versioning, moves activities into httpx/contrib, standardizes durations and ${...} templates, and updates docs, examples, and tests for the v1 API surface and semantics.

Changes

Cohort / File(s)	Summary
Docs & Guides CLAUDE.md, MIGRATION.md, llms.txt, docs/production_checklist.md, docs/suspension.md, planning/review/*	Added v1 migration guide, production checklist, suspension docs, planning notes; updated CLAUDE/llms to reflect branch model, functional options, and template rules.
Readme & Examples README.md, examples/*, examples/expr/*	README rewritten; examples converted to ActivityRegistry + NewExecution(wf, reg, ...) + Execute(...); many expr example files removed.
Core execution surface execution.go, execution_adapter.go, execution_state.go, execution_result.go, execution_result_test.go, execution_test.go	Replaced Run/Resume APIs with Execute(ctx, ...ExecuteOption), resume as option, internal state renamed path→branch, added ExecutionResult helpers and suspension fields, checkpoint schema enforcement.
Branch runtime & evaluation branch.go, branch_local_state.go, branch_test.go, branch_join_test.go, step.go	Renamed path concepts to branch, introduced BranchLocalState and BranchID plumbing, refactored branching/join/wait logic, and moved parameter evaluation to ${...} template semantics.
Activity registry & helpers activity.go, activity_registry.go, activity_functions.go, activity_functions_test.go	Added concrete ActivityRegistry type with Register/MustRegister/Get/Names; replaced NewActivityFunction/NewTypedActivityFunction with ActivityFunc/TypedActivityFunc and internal wrappers.
Activities reorg & timeouts activities/*, activities/httpx/*, activities/contrib/*	Split activities into httpx/contrib, removed WaitActivity, changed timeout fields from float64→time.Duration, PrintActivity now accepts io.Writer, child workflow activity sync behavior simplified.
Child workflows child_workflow.go, activities/child_workflow_activity.go, tests	Removed Sync flags from specs/inputs, made sync vs async choice caller-side, removed Error field from results, added cleanup timeout behavior for async retention.
Checkpoint & checkpointer checkpoint.go, checkpointer.go, checkpointer_file.go, checkpoint_test.go	Introduced CheckpointSchemaVersion and SchemaVersion on checkpoints, renamed persisted PathStates→BranchStates and PathCounter→BranchCounter, added AtomicCheckpointer interface and load-time schema validation.
Context & history context.go, activity_history.go, activity_history_test.go, activity_logger.go	Context API changed to Inputs()/Set/Get/Delete/Keys, Logger/Compiler/BranchID accessors, ctx.Wait/History/ReportProgress; ActivityHistory moved onto Context; log entries use BranchID.
Runner & options runner.go, runner_test.go, runner_suspension_test.go	Runner switched to functional options (WithRunnerLogger, WithDefaultTimeout); Run accepts variadic RunOption (WithResumeFrom, WithHeartbeat, WithCompletionHook, WithRunTimeout); completion hook semantics adjusted.
Errors & validation errors.go, errors_test.go	Sentinel error strings prefixed with workflow: ; timeout classification tightened to context.DeadlineExceeded; added validation sentinel errors for structural validation.
Script/template eval script/eval.go, script/eval_test.go	Template parsing detects single ${...} expression to preserve typed values (Eval returns any); added EvalString; removed $(...) special-case handling for templates.
Pause & suspension helpers pause.go, pause_test.go, docs/suspension.md	Pause APIs renamed to branch-scoped variants (PauseBranch/UnpauseBranch), checkpoint mutation supports atomic update, suspension reasons and resume semantics documented and tests updated.
Tests & infra updates many *_test.go (coverage, execution, pause, sleep, progress, examples tests)	Massive test updates converting to ActivityRegistry, branch-local state, new context methods, new execution wiring, and added tests for ExecutionResult accessors and checkpoint schema handling.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Runner
  participant Execution
  participant ActivityRegistry
  participant Checkpointer

  User->>Runner: Run(ctx, exec, WithResumeFrom?/options...)
  Runner->>Execution: Execute(ctx, execOptions...)
  Execution->>ActivityRegistry: Lookup activity by name
  ActivityRegistry-->>Execution: Activity
  Execution->>Activity: Execute activity with BranchLocalState
  Activity-->>Execution: Activity result
  Execution->>Checkpointer: SaveCheckpoint(SchemaVersion=CheckpointSchemaVersion)
  Checkpointer-->>Execution: ack
  Execution-->>Runner: ExecutionResult
  Runner-->>User: result/status/outputs

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

• Reduce consumer boilerplate: structured results, Runner, validation, progress tracking #10 — Core API redesign introducing ActivityRegistry, NewExecution functional options, Runner/Run option changes and ExecutionResult helpers (strong overlap with this migration).
• Signals, waits, pausing, and durable sleep #14 — Durable suspension, SignalStore/WaitSignal, Pause/Resume and history memoization work that this PR restructures and integrates into the branch model.
• Isolate scripting engines behind interfaces and nested modules #13 — Script/compiler refactor (ScriptCompiler abstraction and template behavior) that this PR extends with ${...} single-expression preservation and template API changes.

Poem

🐇 I hopped from paths to branches wide,
With registries and options by my side,
Durable sleeps and checkpoints new,
Templates snug in ${...} view,
A tidy hop — v1 comes through!

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch v1

[coderabbitai]

Actionable comments posted: 17

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (8)

cmd/workflow/main.go (2)

134-139: ⚠️ Potential issue | 🔴 Critical

Propagate ExecutionResult, not just err.

Execute can return err == nil with result.Status == failed. Dropping the result here means showExecutionResults prints "Execution successful!" and exits 0 for failed workflows.

Suggested fix

-	_, err = execution.Execute(ctx)
+	result, err := execution.Execute(ctx)
 	duration := time.Since(startTime)

 	// Show execution results
-	showExecutionResults(execution, err, duration, config)
+	showExecutionResults(execution, result, err, duration, config)

-func showExecutionResults(execution *workflow.Execution, err error, duration time.Duration, config *Config) {
+func showExecutionResults(execution *workflow.Execution, result *workflow.ExecutionResult, err error, duration time.Duration, config *Config) {
 	status := execution.Status()
@@
-	if err != nil {
+	if err != nil {
 		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
 		if status != workflow.ExecutionStatusCompleted {
 			os.Exit(1)
 		}
-	} else {
+	} else if result != nil && result.Status != workflow.ExecutionStatusCompleted {
+		if result.Error != nil {
+			fmt.Fprintf(os.Stderr, "Error: %v\n", result.Error)
+		}
+		os.Exit(1)
+	} else {
 		info("Execution successful!")
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/workflow/main.go` around lines 134 - 139, The code currently discards the
ExecutionResult returned by execution.Execute and only checks err, which can be
nil while result.Status indicates failure; capture the returned ExecutionResult
(e.g., result := ...) from Execute and pass that result into
showExecutionResults instead of only passing execution and err so
showExecutionResults can inspect result.Status; update the call sites and any
function signature of showExecutionResults if needed to accept the
ExecutionResult (or add an overload) and use result for final success/failure
determination.

---

279-289: ⚠️ Potential issue | 🟠 Major

CLI help and registered built-ins drifted apart.

createActivityRegistry no longer registers a wait activity, but -help still advertises wait as supported. That sends users toward a runtime failure from the first example they copy. Either re-add the activity here or remove it from the usage text.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/workflow/main.go` around lines 279 - 289, The CLI help advertises a
"wait" activity but createActivityRegistry no longer registers it; fix by
restoring the built-in or removing it from the usage text: either add the Wait
activity back into createActivityRegistry (e.g., insert the wait activity
constructor into the activityList alongside activities.NewPrintActivity(),
activities.NewTimeActivity(), etc.) or remove "wait" from the CLI usage/help
string where activities are listed (search for the help/usage text that mentions
"wait"); ensure the symbol names createActivityRegistry and the wait activity
constructor you add/remove are updated consistently so help and the registered
activities no longer drift.

child_workflow.go (1)

285-305: ⚠️ Potential issue | 🟠 Major

Don't drop the terminal async execution result.

This goroutine ignores the values returned by execution.Execute(execCtx). Downstream, GetResult() can only synthesize a generic "child workflow execution failed" and cannot surface the real failure cause or terminal timing for async callers. Store the terminal ExecutionResult/error alongside the handle and read that back in GetResult().

Also applies to: 343-355

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@child_workflow.go` around lines 285 - 305, The goroutine that calls
execution.Execute(execCtx) currently discards its returned
ExecutionResult/error; capture that terminal result and store it together with
the handle in the asyncExecutions map so GetResult() can return the real outcome
and timing. Specifically, change the asyncExecutions value to include a field
for the terminal result/error (or add a separate results map keyed by
execution.ID()), assign the result of execution.Execute(execCtx) into that
storage before deleting the entry, and ensure the existing cleanup logic (the
time.Sleep cleanup goroutine) does not delete the stored result before
GetResult() reads it; update GetResult() to consult the stored
ExecutionResult/error for the given execution.ID() instead of synthesizing a
generic message. Ensure uses of asyncExecutionsMtx protect both writing the
result and deleting the entry.

context.go (1)

264-306: ⚠️ Potential issue | 🟠 Major

Preserve arbitrary Context implementations when wrapping timeout/cancel.

For any Context that isn't *executionContext, these helpers return a brand-new executionContext with none of the parent's state, inputs, logger, or Wait behavior. That breaks the Context contract as soon as callers wrap a custom implementation. Delegate to the parent instead of replacing it.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@context.go` around lines 264 - 306, The helpers WithTimeout and WithCancel
currently replace non-*executionContext parents with a fresh executionContext
losing the parent's state; instead, after creating ctx and cancel (ctx, cancel
:= context.WithTimeout(parent, timeout) / context.WithCancel(parent)), return
the original parent along with cancel when parent is not a *executionContext so
arbitrary Context implementations are preserved (i.e., in WithTimeout and
WithCancel, change the final return from &executionContext{Context: ctx} to
return parent, cancel).

examples/error_handling/main.go (1)

118-126: ⚠️ Potential issue | 🟡 Minor

Check the returned execution result before reporting success.

Execute() separates setup/runtime errors from workflow outcome. If the workflow ends in Failed, err can still be nil, so this path can print “completed successfully” for a failed run.

💡 Proposed fix

-	_, err = execution.Execute(context.Background())
+	result, err := execution.Execute(context.Background())
 	if err != nil {
 		fmt.Printf("Workflow failed: %v\n", err)
 		os.Exit(1)
 	}
+	if result.Failed() {
+		fmt.Printf("Workflow failed: %v\n", result.Error)
+		os.Exit(1)
+	}
 
 	fmt.Printf("Workflow completed successfully!\n")
-	fmt.Printf("Status: %s\n", execution.Status())
-	fmt.Printf("Final outputs: %+v\n", execution.GetOutputs())
+	fmt.Printf("Status: %s\n", result.Status)
+	fmt.Printf("Final outputs: %+v\n", result.Outputs)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@examples/error_handling/main.go` around lines 118 - 126, The current code
treats a nil error from execution.Execute(...) as success, but Execute can
return nil err while the workflow finished with a failed status; check
execution.Status() after Execute returns and treat non-success (e.g., "Failed"
or not "Completed") as an error case. Update the post-Execute logic around
execution.Status() (and optionally execution.GetOutputs()) to print a failure
message and call os.Exit(1) when the status indicates failure, otherwise print
the success messages as currently done.

README.md (1)

55-73: ⚠️ Potential issue | 🟠 Major

Declare result as an output before reading it from ExecutionResult.

The example stores the activity result in branch state, but result.OutputString("result") only reads declared workflow outputs. As written, this branch prints nothing even on success.

🛠️ Proposed fix

 	wf, err := workflow.New(workflow.Options{
 		Name: "demo",
 		Steps: []*workflow.Step{
 			{
 				Name:     "Call My Operation",
 				Activity: "my_operation",
 				Store:    "result",
 				Retry:    []*workflow.RetryConfig{{MaxRetries: 2}},
 				Next:     []*workflow.Edge{{Step: "Finish"}},
 			},
 			{
 				Name:     "Finish",
 				Activity: "print",
 				Parameters: map[string]any{
 					"message": "Workflow completed. Result: ${state.result}",
 				},
 			},
 		},
+		Outputs: []*workflow.Output{
+			{Name: "result", Variable: "result"},
+		},
 	})

Also applies to: 98-100

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 55 - 73, The example assigns the activity result to
branch state via Step.Store but never declares a workflow output, so calls like
result.OutputString("result") read nothing; update the workflow definition
returned by workflow.New (workflow.Options) to include an Outputs entry
declaring "result" and map it to the stored state key (e.g., state.result or the
framework's output path) so that ExecutionResult.OutputString("result") can read
the value; adjust the same pattern used in the other occurrence (lines ~98-100)
to declare the output there as well.

examples/branching/main.go (1)

243-248: ⚠️ Potential issue | 🟡 Minor

Update the example summary text.

Item 4 still says “Script activities for calculations”, but this example now uses Go activities throughout. The stale console output is misleading in a v1 migration example.

🛠️ Proposed fix

-	fmt.Println("4. Script activities for calculations")
+	fmt.Println("4. Go activities for calculations")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@examples/branching/main.go` around lines 243 - 248, Update the example
summary strings printed in main (the fmt.Println calls) so item 4 no longer says
"Script activities for calculations" and instead reflects that the example uses
Go activities (e.g., change to "Go activities for calculations" or similar);
locate the fmt.Println lines in main() that print the numbered list (the five
fmt.Println calls shown) and modify the fourth entry text accordingly to remove
the stale "Script activities" wording.

execution_callbacks_test.go (1)

209-217: ⚠️ Potential issue | 🟡 Minor

Assert the failure hooks explicitly.

This test says it verifies failure callbacks, but the expected event set omits OnWorkflowExecutionFailure, OnBranchFailure, and OnActivityFailure. It can pass even if those hooks stop firing.

🛠️ Proposed fix

-	require.Equal(t, 6, len(eventTypes), "Should have 6 callbacks")
-	require.Equal(t, map[string]bool{
-		"BeforeWorkflowExecution": true,
-		"AfterWorkflowExecution":  true,
-		"BeforeBranchExecution":   true,
-		"AfterBranchExecution":    true,
-		"BeforeActivityExecution": true,
-		"AfterActivityExecution":  true,
-	}, eventTypes)
+	require.True(t, eventTypes["BeforeWorkflowExecution"])
+	require.True(t, eventTypes["AfterWorkflowExecution"])
+	require.True(t, eventTypes["BeforeBranchExecution"])
+	require.True(t, eventTypes["AfterBranchExecution"])
+	require.True(t, eventTypes["BeforeActivityExecution"])
+	require.True(t, eventTypes["AfterActivityExecution"])
+	require.True(t, eventTypes["OnWorkflowExecutionFailure"])
+	require.True(t, eventTypes["OnBranchFailure"])
+	require.True(t, eventTypes["OnActivityFailure"])

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@execution_callbacks_test.go` around lines 209 - 217, The test currently
asserts eventTypes contains only the six non-failure hooks (using require.Equal
on eventTypes) but omits the failure hooks; update the assertion that checks
eventTypes (the map used in the require.Equal) to include
"OnWorkflowExecutionFailure", "OnBranchFailure", and "OnActivityFailure" set to
true (or replace the single require.Equal with explicit require.True checks for
each of those keys) so the test explicitly verifies those failure callbacks are
present when callbacks fire (targets: the eventTypes variable and the existing
require.Equal/assertion around it).

🧹 Nitpick comments (7)

checkpoint_test.go (1)

81-84: Interface conformance checks could be package-level declarations.

The compile-time interface assertions work but are more idiomatically placed as package-level var _ = ... declarations rather than inside a test function. This is a minor style preference.

♻️ Optional: Move to package-level

+var (
+	_ workflow.Checkpointer       = (*workflowtest.MemoryCheckpointer)(nil)
+	_ workflow.AtomicCheckpointer = (*workflowtest.MemoryCheckpointer)(nil)
+)
+
 func TestAtomicCheckpointerInterface(t *testing.T) {
-	var _ workflow.Checkpointer = (*workflowtest.MemoryCheckpointer)(nil)
-	var _ workflow.AtomicCheckpointer = (*workflowtest.MemoryCheckpointer)(nil)
+	// Interface conformance is checked at package level
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@checkpoint_test.go` around lines 81 - 84, The compile-time interface
assertions currently inside TestAtomicCheckpointerInterface should be moved to
package-level declarations: replace the two lines in
TestAtomicCheckpointerInterface (the var _ checks referencing
workflow.Checkpointer, workflow.AtomicCheckpointer and
workflowtest.MemoryCheckpointer) with package-level var _ = ... style assertions
placed outside any function (e.g., at top of the file), and remove or simplify
TestAtomicCheckpointerInterface accordingly so the assertions are evaluated at
compile time without being inside a test function.

planning/review/v1_implementation_plan.md (1)

300-305: Add language specifier to fenced code block.

The code block at line 300 lacks a language specifier, which may affect rendering in some Markdown viewers.

📝 Proposed fix

-```
+```text
 activities/             # safe, in-process primitives only
     print, json, fail, time helpers, random helpers
 activities/httpx/       # http client (safe but I/O)
 activities/contrib/     # shell, file, anything environment-specific or risky

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @planning/review/v1_implementation_plan.md around lines 300 - 305, The fenced
code block containing the directory list (the block starting with triple
backticks above the lines "activities/ # safe, in-process primitives
only" through "activities/contrib/ # shell, file, anything
environment-specific or risky") needs a language specifier to ensure proper
Markdown rendering; update the opening fence from totext (or another
appropriate language token) so the block is marked as plain text in the file.

</details>

</blockquote></details>
<details>
<summary>branch_join_test.go (1)</summary><blockquote>

`66-78`: **Unsafe type assertions may cause test panics.**

The type assertions on lines 67-68, 71-72, and 75-77 (e.g., `value.(int)`) don't check the `ok` boolean. If the type doesn't match, these will panic rather than producing a clear test failure.



<details>
<summary>♻️ Proposed fix to add type safety</summary>

```diff
 reg.MustRegister(ActivityFunc("double", func(ctx Context, params map[string]any) (any, error) {
-    value, _ := ctx.Get("value")
-    return value.(int) * 2, nil
+    value, ok := ctx.Get("value")
+    if !ok {
+        return nil, fmt.Errorf("variable 'value' not found")
+    }
+    v, ok := value.(int)
+    if !ok {
+        return nil, fmt.Errorf("variable 'value' is not an int")
+    }
+    return v * 2, nil
 }))
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@branch_join_test.go` around lines 66 - 78, The activities "double", "triple",
and "sum" use unchecked type assertions (e.g., value.(int), doubled.(int),
tripled.(int)) which can panic; update each ActivityFunc handler to safely
assert types by capturing the comma-ok result (and check ctx.Get errors) and
return a descriptive error if the value is missing or not an int instead of
asserting blindly; specifically change the "double" and "triple" handlers to
verify that ctx.Get("value") yields an int before using it, and change the "sum"
handler to verify that ctx.Get("doubled") and ctx.Get("tripled") yield ints
before adding, returning an error when checks fail.
```

</details>

</blockquote></details>
<details>
<summary>planning/review/combined_api_review.md (1)</summary><blockquote>

`403-409`: **Add language specifier to fenced code block.**

The code block lacks a language specifier.



<details>
<summary>📝 Proposed fix</summary>

```diff
-```
+```text
 activities/             # safe, in-process, side-effect-free primitives
     print, time, random, json, fail
 activities/httpx/       # http client
 activities/contrib/     # shell, file, anything environment-specific
 ```
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @planning/review/combined_api_review.md around lines 403 - 409, The fenced
code block showing the activity directory listing (lines containing
"activities/", "activities/httpx/", "activities/contrib/") is missing a language
specifier; update the opening fence to include a language (for example change
totext or ```bash) so the block is rendered with the correct syntax
highlighting and preserved formatting.

</details>

</blockquote></details>
<details>
<summary>typed_activity_example_test.go (1)</summary><blockquote>

`44-44`: **Consider renaming `BranchID` value to match the new terminology.**  
Using `"path1"` under `BranchID` is a small terminology mismatch; `"branch1"` would be clearer in v1 examples/tests.

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@typed_activity_example_test.go` at line 44, Rename the BranchID test value to
use the new terminology: change the literal assigned to the BranchID field (in
the typed_activity_example_test.go example/test) from "path1" to "branch1" so
the BranchID value matches v1 naming conventions.
```

</details>

</blockquote></details>
<details>
<summary>docs/suspension.md (1)</summary><blockquote>

`89-91`: **Don't freeze internal state layout in the public docs.**

Calling out `BranchState.ActivityHistory` here makes an internal storage detail look like supported API. I'd describe this as a per-step replay cache and avoid naming the backing field.

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@docs/suspension.md` around lines 89 - 91, Replace the explicit internal field
reference "BranchState.ActivityHistory" in the docs with a description that it
is a per-step replay cache (or "per-step history cache") and avoid naming any
backing field; update the sentence that currently reads "The `History` cache is
per-step: it lives on `BranchState.ActivityHistory` and is cleared when the step
advances past the activity." to instead say something like "The history is a
per-step replay cache that is cleared when the step advances past the activity"
so the docs describe behavior without exposing internal storage names.
```

</details>

</blockquote></details>
<details>
<summary>activity_functions_test.go (1)</summary><blockquote>

`79-83`: **Avoid asserting the unexported backing type here.**

This locks the test to `*typedActivityFunc[...]`, so harmless adapter refactors will fail the test even if `TypedActivityFunc` still exposes the same type metadata.



<details>
<summary>♻️ Proposed refactor</summary>

```diff
-	typedFunc, ok := adapter.Activity().(*typedActivityFunc[Person, string])
+	typedFunc, ok := adapter.Activity().(interface {
+		ParametersType() reflect.Type
+		ResultType() reflect.Type
+	})
 	require.True(t, ok)
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@activity_functions_test.go` around lines 79 - 83, The test is asserting the
concrete unexported backing type (*typedActivityFunc[Person, string]) which
couples the test to an implementation detail; instead, change the assertion to
check that adapter.Activity() satisfies the exported TypedActivityFunc[Person,
string] interface (or at minimum exposes the ParametersType() and ResultType()
methods) and then assert on those methods' return values (ParametersType() and
ResultType()) rather than the concrete type name (typedActivityFunc). Ensure you
remove the require.True check against *typedActivityFunc and replace it with a
type assertion or interface check against TypedActivityFunc and then verify
reflect.TypeOf(Person{}) and reflect.TypeOf("") via ParametersType() and
ResultType().
```

</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @activities/child_workflow_activity.go:

• Around line 17-20: ChildWorkflowInput (and other DTOs like
  HTTPActivity.Timeout, ShellActivity.Timeout, RetryConfig, WaitSignalConfig)
  currently use time.Duration as a JSON-bound field which causes string forms like
  "10m" to unmarshal to 0; change the wire format to a string and parse it
  explicitly in the activity: update the JSON-facing structs to accept a string
  (or implement a custom UnmarshalJSON for a DurationString type) for the timeout
  fields, then call time.ParseDuration on the parsed string inside the
  activity/handler and pass the resulting time.Duration into
  ChildWorkflowSpec.Timeout (and the equivalent downstream places); ensure errors
  from ParseDuration are handled and surfaced rather than silently defaulting to
  zero.

In @activities/contrib/shell_activity.go:

• Line 21: The Timeout field on ShellInput is being interpreted as nanoseconds
  when numeric JSON/YAML values are provided; implement a fix by adding a custom
  duration type (e.g., type Duration) with an UnmarshalJSON method and changing
  ShellInput.Timeout to that type so it accepts either a string duration ("5s",
  "5000ms") or a bare number interpreted as seconds (or otherwise validate and
  emit a clear error); update ShellInput's struct comment to document accepted
  formats and add validation in UnmarshalJSON to return a helpful error message
  for ambiguous numeric inputs; reference ShellInput, Timeout, and UnmarshalJSON
  in activities/contrib/shell_activity.go when making the change.

In @activities/httpx/http_activity.go:

• Line 22: Add a custom UnmarshalJSON on the HTTPInput type to accept numeric
  timeouts as seconds and string durations; implement an alias struct with Timeout
  as interface{}, json.Unmarshal into it, then if aux.Timeout is a float64 set
  h.Timeout = time.Duration(v) * time.Second, if it's a string parse with
  time.ParseDuration and return an error wrapped with fmt.Errorf on parse failure;
  ensure the method signature is func (h *HTTPInput) UnmarshalJSON(data []byte)
  error and import encoding/json, time, and fmt as needed to compile.

In @activity_registry.go:

• Around line 28-39: Register currently panics for a nil receiver or when
  r.activities is nil; update Register (receiver *ActivityRegistry) to first guard
  against a nil receiver (return a descriptive error) and then ensure r.activities
  is initialized (if r.activities == nil { make(map[string]Activity) }) before
  writing; keep the existing duplicate-name check using ErrDuplicateActivity and
  then store the activity.

In @branch_join_test.go:

• Around line 161-175: The combine activity (registered via
  reg2.MustRegister(ActivityFunc("combine", ...))) currently uses require.Equal
  inside the activity body which can call t.FailNow() from a goroutine; instead
  remove those require.* calls and either (a) perform runtime checks inside
  combine and return an error (fmt.Errorf(...)) when expectations fail, or (b)
  stop validating inside the activity and move the assertions to the test after
  execution.Execute() completes by inspecting the workflow result / captured
  branch state. Update the combine activity to only return its computed result or
  an error, and add the require.Equal assertions immediately after
  execution.Execute() in the test.

In @branch_local_state.go:

• Around line 76-80: The method BranchLocalState.variablesMap() is unused;
  either remove it to eliminate dead code or clarify intended future use by adding
  a TODO comment and rationale above the function (e.g., "preserved for engine
  snapshot/restore — add callers in X module") so reviewers know it's
  intentionally kept; search for any planned snapshot/restore code paths before
  deleting and update BranchLocalState.variablesMap's comment accordingly.

In @branch.go:

• Around line 1028-1046: resolveEachItems currently treats any string as raw
  script via evaluateExpression, breaking literal single-item lists and the new
  ${...}-only templates; update resolveEachItems to send Each.Items through the
  same template/contextual-typing path used by other parameter values instead of
  unconditionally calling evaluateExpression. Specifically, remove the
  unconditional string->evaluateExpression branch and instead call the
  branch-level template evaluation function (the same helper used elsewhere for
  parameters—e.g., the resolve/resolveParam/evaluateTemplate helper used by p) to:

1. detect and render ${...} templates into an array or single value, 2) coerce a
   rendered single value into a []any with one element for literal strings, and 3)
   preserve existing []any handling; use the symbols resolveEachItems, Each.Items
   and evaluateExpression only as references to locate and replace the logic.

In @checkpoint.go:

• Around line 13-17: The readers currently only reject SchemaVersion >
  CheckpointSchemaVersion and thus accept missing/old schemas (SchemaVersion ==
  0); update each checkpoint reader (memory_checkpointer.go, execution.go,
  checkpointer_file.go) to also reject SchemaVersion < 1 by adding a
  minimum-version check alongside the existing CheckpointSchemaVersion comparison
  (use the SchemaVersion field and the existing CheckpointSchemaVersion constant),
  return an error when SchemaVersion < 1 or SchemaVersion >
  CheckpointSchemaVersion, and add a unit test (e.g.,
  TestCheckpointOlderSchemaVersionIsRejected) that attempts to load a checkpoint
  with SchemaVersion == 0 and expects a failure; alternatively update the
  top-of-file docs near the existing comment to explicitly state that consumers
  must reject SchemaVersion == 0 if you prefer documentation-only—prefer the code
  change to ensure safety.

In @child_workflow.go:

• Around line 190-205: The ExecuteSync wrapper is returning execErr directly
  while execution.Execute(execCtx) encodes failures in its ExecutionResult (not
  always in execErr), so failed child workflows can be returned as (status=failed,
  nil); update the post-execution logic in ExecuteSync to inspect the
  ExecutionResult (result) and execution.Status(): if the status indicates failure
  or cancelled, return a non-nil error instead of nil (propagate execErr if
  present, otherwise construct and return an error that includes
  execution.Status() and any useful result information) while still populating and
  returning the ChildWorkflowResult struct (ExecutionID, Status, Duration,
  Outputs) as before.

In @context.go:

• Around line 24-115: You changed the exported Context interface by
  adding/renaming methods (notably ReportProgress), which breaks existing custom
  contexts and tests; revert Context to its original shape and move new
  functionality into an optional side interface (e.g., declare type
  ProgressReporter interface { ReportProgress(detail ProgressDetail) }), update
  call sites to type-assert the Context to ProgressReporter before invoking
  ReportProgress, and ensure any new helpers/wrappers use that side interface so
  existing Context implementers are not forced to change.

In @docs/production_checklist.md:

• Around line 31-33: The doc incorrectly states Runner as the production-only
  entry point; update the wording around the Runner and Execution.Execute to
  clarify that Runner is a recommended, convenient production entrypoint but
  optional — consumers may call Execution.Execute directly and implement their own
  heartbeat, timeout, resume, and completion orchestration. Edit the section that
  currently claims "Runner (not bare Execution.Execute) is the production entry
  point" to explain that Runner composes those concerns for convenience, while
  Execution.Execute and the modular heartbeat/resume hooks can be used directly
  in custom deployments.

In @examples/join_branches/main.go:

• Around line 123-137: The code calls execution.Execute(ctx) but only checks
  err; instead capture the returned ExecutionResult (e.g., result, err :=
  execution.Execute(ctx)), then verify the result indicates success (check fields
  like result.Error or result.Success / result.Failed() depending on the
  ExecutionResult API) before printing "Workflow completed" or reading outputs; if
  the result signals failure, log or handle that failure and avoid calling
  execution.GetOutputs() and printing outputs until the result is confirmed
  successful.

In @examples/retry_simple/main.go:

• Around line 66-67: Assign the result of execution.Execute(...) to a variable
  (e.g., res, err := execution.Execute(ctx)), then after the nil-error check also
  verify the returned ExecutionResult indicates success before proceeding to
  marshal outputs; for example, inspect the ExecutionResult's status/fields
  (ExecutionResult, ExecutionResult.Status or any IsSuccessful/Failed helper) and
  log/exit if the workflow failed even when err == nil so you don't serialize
  outputs from a failed run.

In @execution_callbacks.go:

• Around line 14-16: Restore the original exported path-level methods/types on
  ExecutionCallbacks (re-add the existing BeforePathExecution/AfterPathExecution
  signatures and any existing PathExecutionEvent type) so existing implementations
  keep compiling, then introduce a new optional side interface (e.g.,
  BranchExecutionCallbacks with BeforeBranchExecution(ctx, *BranchExecutionEvent)
  and AfterBranchExecution(ctx, *BranchExecutionEvent)) or an adapter type for
  branch terminology; update CallbackChain to detect and fan out to both the
  original path-level callbacks and the new BranchExecutionCallbacks (and map
  events between PathExecutionEvent and BranchExecutionEvent as needed) so new
  branch hooks are supported without breaking existing implementations.

In @execution_test.go:

• Around line 1151-1192: The activities modify_state_alpha, modify_state_beta,
  and modify_state_gamma perform test assertions using require.* inside activity
  goroutines, which violates Go's testing rules; change each activity to validate
  via plain condition checks and return an error (e.g., return nil,
  errors.New("...") ) when validation fails instead of calling require.*, then
  move the require.NoError / require.Equal assertions into the test body after
  calling execution.Execute to assert overall execution status and errors from
  those activities.

In @pause.go:

• Around line 200-235: mutatePauseInCheckpoint must guard against a nil
  Checkpointer to avoid panics: at the start of mutatePauseInCheckpoint check if
  cp == nil and return a descriptive error (e.g., fmt.Errorf("nil Checkpointer"))
  instead of proceeding to the type assertion or calling
  AtomicUpdate/LoadCheckpoint; this ensures PauseBranchInCheckpoint and
  UnpauseBranchInCheckpoint return an error rather than panicking when cp is nil.

In @script/eval.go:

• Around line 39-46: The current NewTemplate precheck using strings.Count can
  miss cases like '"} ${foo"' and silently accept malformed templates; modify
  NewTemplate to validate that every "${" has a matching closing "}" that comes
  after it by using templateExprRE: run
  templateExprRE.FindAllStringSubmatchIndex(raw, -1) and if there are zero matches
  but raw contains "${" return an error, and more generally ensure for each match
  that the submatch indices correspond to a "${...}" occurrence; if any "${" lacks
  a corresponding regex match (i.e. a "${" index that isn't covered by a match's
  start index) return a descriptive error instead of falling back to
  &Template{raw: raw}; keep references to NewTemplate and templateExprRE to locate
  changes.

---

Outside diff comments:
In @child_workflow.go:

• Around line 285-305: The goroutine that calls execution.Execute(execCtx)
  currently discards its returned ExecutionResult/error; capture that terminal
  result and store it together with the handle in the asyncExecutions map so
  GetResult() can return the real outcome and timing. Specifically, change the
  asyncExecutions value to include a field for the terminal result/error (or add a
  separate results map keyed by execution.ID()), assign the result of
  execution.Execute(execCtx) into that storage before deleting the entry, and
  ensure the existing cleanup logic (the time.Sleep cleanup goroutine) does not
  delete the stored result before GetResult() reads it; update GetResult() to
  consult the stored ExecutionResult/error for the given execution.ID() instead of
  synthesizing a generic message. Ensure uses of asyncExecutionsMtx protect both
  writing the result and deleting the entry.

In @cmd/workflow/main.go:

• Around line 134-139: The code currently discards the ExecutionResult returned
  by execution.Execute and only checks err, which can be nil while result.Status
  indicates failure; capture the returned ExecutionResult (e.g., result := ...)
  from Execute and pass that result into showExecutionResults instead of only
  passing execution and err so showExecutionResults can inspect result.Status;
  update the call sites and any function signature of showExecutionResults if
  needed to accept the ExecutionResult (or add an overload) and use result for
  final success/failure determination.
• Around line 279-289: The CLI help advertises a "wait" activity but
  createActivityRegistry no longer registers it; fix by restoring the built-in or
  removing it from the usage text: either add the Wait activity back into
  createActivityRegistry (e.g., insert the wait activity constructor into the
  activityList alongside activities.NewPrintActivity(),
  activities.NewTimeActivity(), etc.) or remove "wait" from the CLI usage/help
  string where activities are listed (search for the help/usage text that mentions
  "wait"); ensure the symbol names createActivityRegistry and the wait activity
  constructor you add/remove are updated consistently so help and the registered
  activities no longer drift.

In @context.go:

• Around line 264-306: The helpers WithTimeout and WithCancel currently replace
  non-*executionContext parents with a fresh executionContext losing the parent's
  state; instead, after creating ctx and cancel (ctx, cancel :=
  context.WithTimeout(parent, timeout) / context.WithCancel(parent)), return the
  original parent along with cancel when parent is not a *executionContext so
  arbitrary Context implementations are preserved (i.e., in WithTimeout and
  WithCancel, change the final return from &executionContext{Context: ctx} to
  return parent, cancel).

In @examples/branching/main.go:

• Around line 243-248: Update the example summary strings printed in main (the
  fmt.Println calls) so item 4 no longer says "Script activities for calculations"
  and instead reflects that the example uses Go activities (e.g., change to "Go
  activities for calculations" or similar); locate the fmt.Println lines in main()
  that print the numbered list (the five fmt.Println calls shown) and modify the
  fourth entry text accordingly to remove the stale "Script activities" wording.

In @examples/error_handling/main.go:

• Around line 118-126: The current code treats a nil error from
  execution.Execute(...) as success, but Execute can return nil err while the
  workflow finished with a failed status; check execution.Status() after Execute
  returns and treat non-success (e.g., "Failed" or not "Completed") as an error
  case. Update the post-Execute logic around execution.Status() (and optionally
  execution.GetOutputs()) to print a failure message and call os.Exit(1) when the
  status indicates failure, otherwise print the success messages as currently
  done.

In @execution_callbacks_test.go:

• Around line 209-217: The test currently asserts eventTypes contains only the
  six non-failure hooks (using require.Equal on eventTypes) but omits the failure
  hooks; update the assertion that checks eventTypes (the map used in the
  require.Equal) to include "OnWorkflowExecutionFailure", "OnBranchFailure", and
  "OnActivityFailure" set to true (or replace the single require.Equal with
  explicit require.True checks for each of those keys) so the test explicitly
  verifies those failure callbacks are present when callbacks fire (targets: the
  eventTypes variable and the existing require.Equal/assertion around it).

In @README.md:

• Around line 55-73: The example assigns the activity result to branch state via
  Step.Store but never declares a workflow output, so calls like
  result.OutputString("result") read nothing; update the workflow definition
  returned by workflow.New (workflow.Options) to include an Outputs entry
  declaring "result" and map it to the stored state key (e.g., state.result or the
  framework's output path) so that ExecutionResult.OutputString("result") can read
  the value; adjust the same pattern used in the other occurrence (lines ~98-100)
  to declare the output there as well.

---

Nitpick comments:
In @activity_functions_test.go:

• Around line 79-83: The test is asserting the concrete unexported backing type
  (*typedActivityFunc[Person, string]) which couples the test to an implementation
  detail; instead, change the assertion to check that adapter.Activity() satisfies
  the exported TypedActivityFunc[Person, string] interface (or at minimum exposes
  the ParametersType() and ResultType() methods) and then assert on those methods'
  return values (ParametersType() and ResultType()) rather than the concrete type
  name (typedActivityFunc). Ensure you remove the require.True check against
  *typedActivityFunc and replace it with a type assertion or interface check
  against TypedActivityFunc and then verify reflect.TypeOf(Person{}) and
  reflect.TypeOf("") via ParametersType() and ResultType().

In @branch_join_test.go:

• Around line 66-78: The activities "double", "triple", and "sum" use unchecked
  type assertions (e.g., value.(int), doubled.(int), tripled.(int)) which can
  panic; update each ActivityFunc handler to safely assert types by capturing the
  comma-ok result (and check ctx.Get errors) and return a descriptive error if the
  value is missing or not an int instead of asserting blindly; specifically change
  the "double" and "triple" handlers to verify that ctx.Get("value") yields an int
  before using it, and change the "sum" handler to verify that ctx.Get("doubled")
  and ctx.Get("tripled") yield ints before adding, returning an error when checks
  fail.

In @checkpoint_test.go:

• Around line 81-84: The compile-time interface assertions currently inside
  TestAtomicCheckpointerInterface should be moved to package-level declarations:
  replace the two lines in TestAtomicCheckpointerInterface (the var _ checks
  referencing workflow.Checkpointer, workflow.AtomicCheckpointer and
  workflowtest.MemoryCheckpointer) with package-level var _ = ... style assertions
  placed outside any function (e.g., at top of the file), and remove or simplify
  TestAtomicCheckpointerInterface accordingly so the assertions are evaluated at
  compile time without being inside a test function.

In @docs/suspension.md:

• Around line 89-91: Replace the explicit internal field reference
  "BranchState.ActivityHistory" in the docs with a description that it is a
  per-step replay cache (or "per-step history cache") and avoid naming any backing
  field; update the sentence that currently reads "The History cache is
  per-step: it lives on BranchState.ActivityHistory and is cleared when the step
  advances past the activity." to instead say something like "The history is a
  per-step replay cache that is cleared when the step advances past the activity"
  so the docs describe behavior without exposing internal storage names.

In @planning/review/combined_api_review.md:

• Around line 403-409: The fenced code block showing the activity directory
  listing (lines containing "activities/", "activities/httpx/",
  "activities/contrib/") is missing a language specifier; update the opening fence
  to include a language (for example change totext or ```bash) so the
  block is rendered with the correct syntax highlighting and preserved formatting.

In @planning/review/v1_implementation_plan.md:

• Around line 300-305: The fenced code block containing the directory list (the
  block starting with triple backticks above the lines "activities/ #
  safe, in-process primitives only" through "activities/contrib/ # shell,
  file, anything environment-specific or risky") needs a language specifier to
  ensure proper Markdown rendering; update the opening fence from totext
  (or another appropriate language token) so the block is marked as plain text in
  the file.

In @typed_activity_example_test.go:

• Line 44: Rename the BranchID test value to use the new terminology: change the
  literal assigned to the BranchID field (in the typed_activity_example_test.go
  example/test) from "path1" to "branch1" so the BranchID value matches v1 naming
  conventions.

</details>

<details>
<summary>🪄 Autofix (Beta)</summary>

Fix all unresolved CodeRabbit comments on this PR:

- [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended)
- [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes

</details>

---

<details>
<summary>ℹ️ Review info</summary>

<details>
<summary>⚙️ Run configuration</summary>

**Configuration used**: defaults

**Review profile**: CHILL

**Plan**: Pro

**Run ID**: `da94b2be-b09a-4148-a507-7f0cb823eb6d`

</details>

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between d6ad3373590fd97f1046ede52a1c4cc00839fcbd and a46278d0b31e5f92524d5f034f083ee18844348b.

</details>

<details>
<summary>📒 Files selected for processing (114)</summary>

* `CLAUDE.md`
* `MIGRATION.md`
* `README.md`
* `activities/child_workflow_activity.go`
* `activities/child_workflow_activity_test.go`
* `activities/contrib/file_activity.go`
* `activities/contrib/file_activity_test.go`
* `activities/contrib/helpers_test.go`
* `activities/contrib/shell_activity.go`
* `activities/contrib/shell_activity_test.go`
* `activities/helpers_test.go`
* `activities/httpx/helpers_test.go`
* `activities/httpx/http_activity.go`
* `activities/httpx/http_activity_test.go`
* `activities/print_activity.go`
* `activities/wait_activity.go`
* `activities/wait_activity_test.go`
* `activity.go`
* `activity_functions.go`
* `activity_functions_test.go`
* `activity_history.go`
* `activity_history_test.go`
* `activity_logger.go`
* `activity_registry.go`
* `branch.go`
* `branch_join_test.go`
* `branch_local_state.go`
* `branch_state.go`
* `branch_test.go`
* `checkpoint.go`
* `checkpoint_test.go`
* `checkpointer.go`
* `checkpointer_file.go`
* `child_workflow.go`
* `cmd/workflow/main.go`
* `context.go`
* `coverage_test.go`
* `docs/production_checklist.md`
* `docs/suspension.md`
* `errors.go`
* `errors_test.go`
* `examples/branching/main.go`
* `examples/callbacks/main.go`
* `examples/child_workflows/README.md`
* `examples/child_workflows/main.go`
* `examples/durable_sleep/main.go`
* `examples/edge_matching/main.go`
* `examples/error_handling/main.go`
* `examples/expr/README.md`
* `examples/expr/basic/main.go`
* `examples/expr/compile_once/main.go`
* `examples/expr/funcs/main.go`
* `examples/expr/higher_order/main.go`
* `examples/expr/structs/main.go`
* `examples/expr/workflow/main.go`
* `examples/fenced_checkpointer/main.go`
* `examples/join_branches/README.md`
* `examples/join_branches/main.go`
* `examples/join_paths/main.go`
* `examples/pause_unpause/main.go`
* `examples/retry/main.go`
* `examples/retry_simple/main.go`
* `examples/runner/main.go`
* `examples/signal_wait/main.go`
* `examples/simple/main.go`
* `examples/step_progress/main.go`
* `examples/structured_result/main.go`
* `execution.go`
* `execution_adapter.go`
* `execution_callbacks.go`
* `execution_callbacks_test.go`
* `execution_result.go`
* `execution_result_test.go`
* `execution_state.go`
* `execution_test.go`
* `llms.txt`
* `path_join_test.go`
* `path_local_state.go`
* `pause.go`
* `pause_test.go`
* `planning/review/combined_api_review.md`
* `planning/review/v1_implementation_plan.md`
* `production_readiness_test.go`
* `progress.go`
* `review_fixes_test.go`
* `run_or_resume_test.go`
* `runner.go`
* `runner_suspension_test.go`
* `runner_test.go`
* `script/eval.go`
* `script/eval_test.go`
* `script_compiler_test.go`
* `signal_store.go`
* `sleep_test.go`
* `step.go`
* `step_progress.go`
* `step_progress_test.go`
* `typed_activity_example_test.go`
* `validate.go`
* `validate_test.go`
* `variable_container.go`
* `variable_container_test.go`
* `wait.go`
* `wait_state.go`
* `wait_test.go`
* `workflow.go`
* `workflow_formatter.go`
* `workflow_test.go`
* `workflowtest/fake_context.go`
* `workflowtest/fake_context_test.go`
* `workflowtest/memory_checkpointer.go`
* `workflowtest/mock.go`
* `workflowtest/workflowtest.go`
* `workflowtest/workflowtest_test.go`

</details>

<details>
<summary>💤 Files with no reviewable changes (13)</summary>

* activity.go
* examples/expr/README.md
* activities/wait_activity_test.go
* examples/expr/compile_once/main.go
* examples/expr/basic/main.go
* examples/expr/higher_order/main.go
* examples/join_paths/main.go
* examples/expr/funcs/main.go
* path_join_test.go
* examples/expr/workflow/main.go
* examples/expr/structs/main.go
* activities/wait_activity.go
* path_local_state.go

</details>

<details>
<summary>👮 Files not reviewed due to content moderation or server errors (19)</summary>

* activities/helpers_test.go
* progress.go
* step_progress.go
* activity_history.go
* execution.go
* examples/signal_wait/main.go
* llms.txt
* execution_state.go
* examples/structured_result/main.go
* activities/child_workflow_activity_test.go
* examples/runner/main.go
* step_progress_test.go
* examples/callbacks/main.go
* execution_adapter.go
* examples/step_progress/main.go
* examples/pause_unpause/main.go
* branch_test.go
* MIGRATION.md
* errors.go

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the actual file content around the specified lines
cat -n activities/child_workflow_activity.go | head -60

Repository: deepnoodle-ai/workflow

Length of output: 2372

---

🏁 Script executed:

# Check if there are any custom UnmarshalJSON methods for ChildWorkflowInput
rg "UnmarshalJSON|MarshalJSON" activities/child_workflow_activity.go -A 10

Repository: deepnoodle-ai/workflow

Length of output: 48

---

🏁 Script executed:

# Search for how ChildWorkflowInput is used - especially JSON binding
rg "ChildWorkflowInput" activities/ -B 2 -A 5

Repository: deepnoodle-ai/workflow

Length of output: 1895

---

🏁 Script executed:

# Look for time.Duration JSON handling patterns in the codebase
rg "time\.Duration" activities/ -B 2 -A 2

Repository: deepnoodle-ai/workflow

Length of output: 1366

---

🏁 Script executed:

# Search for tests related to ChildWorkflowActivity
find . -name "*test.go" -o -name "*_test.go" | xargs rg "ChildWorkflowActivity|ChildWorkflowInput" 2>/dev/null

Repository: deepnoodle-ai/workflow

Length of output: 439

---

🏁 Script executed:

# Look for how activity parameters are bound from JSON/workflow parameters
rg "Execute.*params|UnmarshalJSON|json.Unmarshal" activities/ -B 3 -A 5 | head -100

Repository: deepnoodle-ai/workflow

Length of output: 5207

---

🏁 Script executed:

# Check if there's documentation on timeout format expectations
rg -i "timeout.*format|timeout.*duration|30s|1h|nanosecond" . -B 2 -A 2 | head -80

Repository: deepnoodle-ai/workflow

Length of output: 4104

---

🏁 Script executed:

# Look at the Activity interface to understand parameter binding
rg "type Activity|type TypedActivity|func.*Activity.*Execute" --glob "*.go" -B 2 -A 5 | head -100

Repository: deepnoodle-ai/workflow

Length of output: 5210

---

🏁 Script executed:

# Let's examine the ChildWorkflowActivity test to see how timeout is handled
cat -n activities/child_workflow_activity_test.go

Repository: deepnoodle-ai/workflow

Length of output: 2812

---

🏁 Script executed:

# Look at the Activity interface definition and how parameters are bound
rg "type Activity interface|type TypedActivity|func NewTypedActivity" --glob="*.go" -B 2 -A 15

Repository: deepnoodle-ai/workflow

Length of output: 2145

---

🏁 Script executed:

# Check if there's any custom timeout parsing or JSON handling for activities
rg "time\.Duration.*json|json.*time\.Duration|UnmarshalJSON.*Duration" -B 3 -A 5

Repository: deepnoodle-ai/workflow

Length of output: 7363

---

🏁 Script executed:

# Check how workflow parameters are actually provided - look at workflow definition/execution code
rg "workflow parameters|step parameters|activity input|params map" --glob="*.go" -B 2 -A 3 | head -80

Repository: deepnoodle-ai/workflow

Length of output: 3811

---

🏁 Script executed:

# Look for any YAML/configuration examples that show timeout format
find . -name "*.yaml" -o -name "*.yml" -o -name "*.json" | xargs rg "timeout" 2>/dev/null | head -20

Repository: deepnoodle-ai/workflow

Length of output: 48

---

🏁 Script executed:

# Test Go's native time.Duration JSON behavior with a simple test
python3 << 'EOF'
import json
import subprocess

# Create a minimal Go test program to verify time.Duration JSON behavior
test_code = '''package main
import (
	"encoding/json"
	"fmt"
	"time"
)

type TestStruct struct {
	Timeout time.Duration `json:"timeout"`
}

func main() {
	// Test 1: numeric input (nanoseconds)
	numericJSON := `{"timeout": 30000000000}` // 30 seconds in nanoseconds
	var t1 TestStruct
	err := json.Unmarshal([]byte(numericJSON), &t1)
	fmt.Printf("Numeric (30s in ns): err=%v, value=%v\\n", err, t1.Timeout)

	// Test 2: string input ("30s" format)
	stringJSON := `{"timeout": "30s"}`
	var t2 TestStruct
	err = json.Unmarshal([]byte(stringJSON), &t2)
	fmt.Printf("String (\\\"30s\\\"): err=%v, value=%v\\n", err, t2.Timeout)

	// Test 3: invalid format
	invalidJSON := `{"timeout": "invalid"}`
	var t3 TestStruct
	err = json.Unmarshal([]byte(invalidJSON), &t3)
	fmt.Printf("Invalid: err=%v\\n", err != nil)
}
'''

with open('/tmp/test_duration.go', 'w') as f:
	f.write(test_code)

# Run it
result = subprocess.run(['go', 'run', '/tmp/test_duration.go'], 
					   capture_output=True, text=True, timeout=10)
print(result.stdout)
if result.stderr:
	print("STDERR:", result.stderr)
EOF

Repository: deepnoodle-ai/workflow

Length of output: 864

---

🏁 Script executed:

# Look at how parameters are documented in examples or README
find . -name "README*" -o -name "*.md" | xargs rg -i "activity.*param|timeout.*format" 2>/dev/null | head -20

Repository: deepnoodle-ai/workflow

Length of output: 2119

---

🏁 Script executed:

# Look at the activity.go implementation more carefully to understand the marshaling flow
sed -n '1,150p' activity.go | tail -50

Repository: deepnoodle-ai/workflow

Length of output: 1832

---

🏁 Script executed:

# Check if there are any documented examples of how to pass timeout to activities
rg "timeout" documentation/ examples/ --glob="*.md" -B 2 -A 2

Repository: deepnoodle-ai/workflow

Length of output: 5780

---

🏁 Script executed:

# Verify Go's time.Duration JSON support by checking if there are any comments or documentation about it
rg "time.Duration" -B 5 -A 5 | grep -i "json\|marshal\|format\|unmarshal" | head -30

Repository: deepnoodle-ai/workflow

Length of output: 2564

---

🏁 Script executed:

# Check step.go to see how WaitSignal.Timeout and other Duration fields are used
sed -n '1,200p' step.go | grep -A 10 -B 5 "Timeout.*Duration"

Repository: deepnoodle-ai/workflow

Length of output: 1242

---

🏁 Script executed:

# Quick verification of Go's time.Duration JSON behavior
cat > /tmp/test_duration.go << 'EOF'
package main
import (
	"encoding/json"
	"fmt"
	"time"
)

type TestInput struct {
	Timeout time.Duration `json:"timeout"`
}

func main() {
	// Test string format
	stringInput := `{"timeout":"30s"}`
	var t1 TestInput
	err1 := json.Unmarshal([]byte(stringInput), &t1)
	fmt.Printf("String format (\"30s\"): success=%v, value=%v\n", err1 == nil, t1.Timeout)
	
	// Test numeric format (nanoseconds)
	numInput := `{"timeout":30000000000}`
	var t2 TestInput
	err2 := json.Unmarshal([]byte(numInput), &t2)
	fmt.Printf("Numeric format (30s in ns): success=%v, value=%v\n", err2 == nil, t2.Timeout)
}
EOF
cd /tmp && go run test_duration.go

Repository: deepnoodle-ai/workflow

Length of output: 164

---

🏁 Script executed:

# Check the Go version used in the project to confirm time.Duration JSON support
head -20 go.mod

Repository: deepnoodle-ai/workflow

Length of output: 163

---

🏁 Script executed:

# Verify the actual behavior in context by checking if there are any workarounds or custom unmarshalers
rg "UnmarshalJSON|Duration.*json|json.*Duration" --glob="*.go" -B 5 -A 10 | head -100

Repository: deepnoodle-ai/workflow

Length of output: 4936

---

🏁 Script executed:

# Check if the documentation examples actually match what the code accepts
rg "timeout.*[0-9]" documentation/child-workflows.md examples/ --glob="*.md" --glob="*.go"

Repository: deepnoodle-ai/workflow

Length of output: 608

---

time.Duration as a JSON-bound activity input field is a silent data loss risk.

The wire format only accepts nanosecond integers—string forms like "30s" silently fail to unmarshal and become 0. This directly contradicts the documentation (e.g., child-workflows.md shows "timeout": "10m"), creating a compatibility trap where workflow parameters will silently lose their timeout values. The same issue affects HTTPActivity.Timeout, ShellActivity.Timeout, and duration fields in RetryConfig and WaitSignalConfig.

Use a string wire format and parse it in the activity before passing to ChildWorkflowSpec.Timeout to avoid silent data corruption.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@activities/child_workflow_activity.go` around lines 17 - 20,
ChildWorkflowInput (and other DTOs like HTTPActivity.Timeout,
ShellActivity.Timeout, RetryConfig, WaitSignalConfig) currently use
time.Duration as a JSON-bound field which causes string forms like "10m" to
unmarshal to 0; change the wire format to a string and parse it explicitly in
the activity: update the JSON-facing structs to accept a string (or implement a
custom UnmarshalJSON for a DurationString type) for the timeout fields, then
call time.ParseDuration on the parsed string inside the activity/handler and
pass the resulting time.Duration into ChildWorkflowSpec.Timeout (and the
equivalent downstream places); ensure errors from ParseDuration are handled and
surfaced rather than silently defaulting to zero.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Find how typed activity params are decoded into structs
rg -n -C3 'func NewTypedActivity|TypedActivityFunc|json\.Marshal|json\.Unmarshal|mapstructure|Decode' --type go

# 2) Find non-test timeout examples/usages to confirm expected units
rg -n -C2 '"timeout"\s*:' --type go --type md --type yaml --type yml --type json -g '!**/*_test.go'

# 3) Locate docs that define timeout units for shell/contrib activities
rg -n -C2 'shell|timeout|duration|seconds|time\.Duration' --type md

Repository: deepnoodle-ai/workflow

Length of output: 21601

---

🏁 Script executed:

# Find and inspect shell_activity.go to see actual timeout implementation
fd -type f -name "shell_activity.go" -exec cat -n {} +

Repository: deepnoodle-ai/workflow

Length of output: 236

---

🏁 Script executed:

# Search for shell activity usage examples and tests
rg -n "shell_activity|ShellInput|ShellActivity" --type go -A 5 -B 2 | head -100

Repository: deepnoodle-ai/workflow

Length of output: 3085

---

🏁 Script executed:

# Look for any timeout-related documentation or examples
fd -type f \( -name "*.md" -o -name "*.txt" -o -name "README*" \) -path "*/activities/*" | xargs rg -l "timeout" 2>/dev/null || true

Repository: deepnoodle-ai/workflow

Length of output: 1353

---

🏁 Script executed:

# Check how shell activity timeout is actually used in examples/tests
rg -n "Timeout.*time\.Duration|timeout.*:" activities/contrib/ --type go -A 3 -B 3

Repository: deepnoodle-ai/workflow

Length of output: 1779

---

🏁 Script executed:

# Read complete shell_activity.go to see all timeout handling
cat -n activities/contrib/shell_activity.go

Repository: deepnoodle-ai/workflow

Length of output: 2793

---

🏁 Script executed:

# Search for shell activity usage in workflow definitions or examples
rg -n "shell" --type-list | head -10 && rg -n '"shell"' -A 10 -B 2 | head -80

Repository: deepnoodle-ai/workflow

Length of output: 2310

---

🏁 Script executed:

# Look for any example workflows that use the shell activity
find . -name "*.json" -o -name "*.yaml" -o -name "*.yml" | xargs rg -l "shell" 2>/dev/null | head -10

Repository: deepnoodle-ai/workflow

Length of output: 48

---

🏁 Script executed:

# Check TypedActivity parameter decoding to understand time.Duration JSON unmarshal behavior
rg -n "time\.Duration|UnmarshalJSON" activities/contrib/ -A 3

Repository: deepnoodle-ai/workflow

Length of output: 353

---

🏁 Script executed:

# Search for examples of shell activity being used in workflows
rg -n "shell|timeout" examples/ -A 5 -B 2 | head -100

Repository: deepnoodle-ai/workflow

Length of output: 6505

---

🏁 Script executed:

# Check README and docs for shell activity documentation
cat -n activities/contrib/README.md 2>/dev/null || echo "No README found"

Repository: deepnoodle-ai/workflow

Length of output: 81

---

🏁 Script executed:

# Look for migration guide or breaking changes related to timeout
rg -n "timeout|duration|shell" MIGRATION.md -A 3 -B 3 | head -80

Repository: deepnoodle-ai/workflow

Length of output: 1255

---

🏁 Script executed:

# Check if there are any workflow test files that define timeouts
find . -name "*test*.json" -o -name "*example*.json" | xargs rg -l "timeout" 2>/dev/null | head -10

Repository: deepnoodle-ai/workflow

Length of output: 231

---

Timeout unit regression: numeric JSON values become nanoseconds instead of seconds.

When workflow definitions provide numeric timeout values (e.g., "timeout": 5), Go's standard time.Duration JSON unmarshal interprets them as nanoseconds. This causes timeout: 5 to become 5 nanoseconds, triggering near-immediate cancellation instead of the likely-intended 5 seconds.

The migration to time.Duration is documented in MIGRATION.md (lines 213–215), but the expected format for timeout in workflow definitions is unclear. Examples show Go code using 30 * time.Second, not JSON/YAML input. Consider:

1. Add documentation to ShellInput (line 21) clarifying that timeout must be a string duration format ("5s", "5000ms") or a numeric nanosecond value if provided via workflow definitions.
2. Add validation or a helpful error message if a user provides an ambiguous numeric format.
3. Update examples to demonstrate correct timeout format in workflow definitions.

Also applies to: 43-43

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@activities/contrib/shell_activity.go` at line 21, The Timeout field on
ShellInput is being interpreted as nanoseconds when numeric JSON/YAML values are
provided; implement a fix by adding a custom duration type (e.g., type Duration)
with an UnmarshalJSON method and changing ShellInput.Timeout to that type so it
accepts either a string duration ("5s", "5000ms") or a bare number interpreted
as seconds (or otherwise validate and emit a clear error); update ShellInput's
struct comment to document accepted formats and add validation in UnmarshalJSON
to return a helpful error message for ambiguous numeric inputs; reference
ShellInput, Timeout, and UnmarshalJSON in activities/contrib/shell_activity.go
when making the change.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

How does Go encoding/json handle time.Duration marshaling and unmarshaling?

💡 Result:

In standard encoding/json (as of Go 1.26), time.Duration does not implement json.Marshaler or json.Unmarshaler interfaces, nor encoding.TextMarshaler. Therefore, it is treated as an integer type and marshaled to a JSON number representing its nanosecond value (e.g., time.Second marshals to 1000000000). Unmarshaling from a JSON number sets the Duration to that integer value interpreted as nanoseconds; unmarshaling from a string or other types fails with a type error. Developers commonly wrap time.Duration in a custom type implementing MarshalJSON/UnmarshalJSON to use the human-readable time.Duration.String format (e.g., "1s") for both marshaling (as JSON string) and unmarshaling (parsing via time.ParseDuration, often also accepting numbers). An experimental encoding/json/v2 (enabled via GOEXPERIMENT=jsonv2, proposed in 2025 issues #71497/#71631, available in tip) adds native support for time.Duration with configurable formats via struct tags like json:",format:sec", defaulting to time.Duration.String representation. As of 2026-04-12, this remains experimental and is not the default behavior.

Citations:

• 1: https://stackoverflow.com/questions/48050945/how-to-unmarshal-json-into-durations
• 2: https://go.dev/issue/71631
• 3: https://pkg.go.dev/encoding/json
• 4: https://pkg.go.dev/encoding/json/v2
• 5: https://pkg.go.dev/encoding/json@go1.25.6
• 6: https://go.dev/issue/10275
• 7: https://go.dev/src/encoding/json/encode.go
• 8: proposal: time: add MarshalJSON and UnmarshalJSON support golang/go#65501

---

🏁 Script executed:

#!/bin/bash
# Locate and examine the HTTPInput struct in http_activity.go
fd -e go -p "http_activity" | head -5

Repository: deepnoodle-ai/workflow

Length of output: 138

---

🏁 Script executed:

#!/bin/bash
# Check the HTTPInput struct definition and surrounding code
cat -n activities/httpx/http_activity.go | head -50

Repository: deepnoodle-ai/workflow

Length of output: 2030

---

🏁 Script executed:

#!/bin/bash
# Look for JSON marshaling methods on HTTPInput or related types
rg -A 5 "(UnmarshalJSON|MarshalJSON)" activities/httpx/ --type go

Repository: deepnoodle-ai/workflow

Length of output: 48

---

🏁 Script executed:

#!/bin/bash
# Check test file for JSON usage patterns
cat -n activities/httpx/http_activity_test.go | head -100

Repository: deepnoodle-ai/workflow

Length of output: 3883

---

🏁 Script executed:

#!/bin/bash
# Search for timeout usage in tests or examples
rg -i "timeout" activities/httpx/ --type go -B 2 -A 2

Repository: deepnoodle-ai/workflow

Length of output: 1220

---

🏁 Script executed:

#!/bin/bash
# Check if there's any conversion or validation logic for Timeout in Execute method
cat -n activities/httpx/http_activity.go | sed -n '48,120p'

Repository: deepnoodle-ai/workflow

Length of output: 2491

---

Add custom UnmarshalJSON to HTTPInput to handle numeric timeout values.

Changing Timeout from float64 to time.Duration introduces a breaking change in JSON deserialization. The standard encoding/json marshals time.Duration as an integer representing nanoseconds. This means:

• Existing workflows with "timeout": 30 (expecting 30 seconds) would be deserialized as 30 nanoseconds
• Users must now pass nanosecond values like "timeout": 30000000000 for 30 seconds

Implement a custom UnmarshalJSON method on HTTPInput that interprets numeric JSON values as seconds (for backwards compatibility) while accepting duration strings like "30s":

func (h *HTTPInput) UnmarshalJSON(data []byte) error {
    type Alias HTTPInput
    aux := &struct {
        Timeout interface{} `json:"timeout"`
        *Alias
    }{
        Alias: (*Alias)(h),
    }
    if err := json.Unmarshal(data, &aux); err != nil {
        return err
    }
    if aux.Timeout != nil {
        switch v := aux.Timeout.(type) {
        case float64:
            h.Timeout = time.Duration(v) * time.Second
        case string:
            d, err := time.ParseDuration(v)
            if err != nil {
                return fmt.Errorf("invalid timeout: %w", err)
            }
            h.Timeout = d
        }
    }
    return nil
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@activities/httpx/http_activity.go` at line 22, Add a custom UnmarshalJSON on
the HTTPInput type to accept numeric timeouts as seconds and string durations;
implement an alias struct with Timeout as interface{}, json.Unmarshal into it,
then if aux.Timeout is a float64 set h.Timeout = time.Duration(v) * time.Second,
if it's a string parse with time.ParseDuration and return an error wrapped with
fmt.Errorf on parse failure; ensure the method signature is func (h *HTTPInput)
UnmarshalJSON(data []byte) error and import encoding/json, time, and fmt as
needed to compile.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Keep ExecutionCallbacks backward-compatible.

Renaming the exported path hooks/events forces every custom callback implementation to stop compiling. Please preserve the existing methods/types and add branch terminology through a side interface or adapter, then have CallbackChain fan out to both.

Based on learnings: Never modify exported interfaces; use optional side interfaces (such as the ProgressReporter pattern) to add new functionality.

Also applies to: 37-42, 55-56

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@execution_callbacks.go` around lines 14 - 16, Restore the original exported
path-level methods/types on ExecutionCallbacks (re-add the existing
BeforePathExecution/AfterPathExecution signatures and any existing
PathExecutionEvent type) so existing implementations keep compiling, then
introduce a new optional side interface (e.g., BranchExecutionCallbacks with
BeforeBranchExecution(ctx, *BranchExecutionEvent) and AfterBranchExecution(ctx,
*BranchExecutionEvent)) or an adapter type for branch terminology; update
CallbackChain to detect and fan out to both the original path-level callbacks
and the new BranchExecutionCallbacks (and map events between PathExecutionEvent
and BranchExecutionEvent as needed) so new branch hooks are supported without
breaking existing implementations.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, understand the internal/require package
find . -name require -type f | head -20
find . -path '*/internal/require*' -type f

Repository: deepnoodle-ai/workflow

Length of output: 95

---

🏁 Script executed:

# Search for internal/require usage and definition
rg -l 'internal/require' --type go | head -20
rg -A 5 'package require' --type go

Repository: deepnoodle-ai/workflow

Length of output: 735

---

🏁 Script executed:

# Check execution_test.go for the full context of these activities
wc -l execution_test.go
sed -n '1140,1200p' execution_test.go
sed -n '1470,1510p' execution_test.go

Repository: deepnoodle-ai/workflow

Length of output: 3671

---

🏁 Script executed:

# Understand how activities are executed - check the ActivityFunc type
ast-grep --pattern 'type ActivityFunc $_'
rg 'func.*ActivityFunc' --type go -B 2 -A 10 | head -50

Repository: deepnoodle-ai/workflow

Length of output: 2698

---

🏁 Script executed:

# Check how Execute runs activities and if they use goroutines
rg 'func.*Execute' --type go | grep -v test | head -20

Repository: deepnoodle-ai/workflow

Length of output: 2567

---

🏁 Script executed:

# Examine the execution_adapter to understand how activities are executed
cat -n execution_adapter.go

Repository: deepnoodle-ai/workflow

Length of output: 502

---

🏁 Script executed:

# Check how ExecuteActivity is called - look for goroutine spawning
rg 'go ' --type go -B 3 -A 3 | grep -A 10 -B 10 'ExecuteActivity\|activity'

Repository: deepnoodle-ai/workflow

Length of output: 48

---

🏁 Script executed:

# Examine the internal/require implementation to understand behavior on assertion failure
cat -n internal/require/require.go

Repository: deepnoodle-ai/workflow

Length of output: 13544

---

🏁 Script executed:

# Search for how activities are invoked during execution
rg 'executeStep\|executeActivity' --type go -A 20 | head -80

Repository: deepnoodle-ai/workflow

Length of output: 48

---

🏁 Script executed:

# Find the executeActivity implementation in execution.go
rg -A 30 'func.*executeActivity' --type go execution.go | head -80

Repository: deepnoodle-ai/workflow

Length of output: 1657

---

🏁 Script executed:

# Search for goroutine spawning related to activity execution
rg 'go.*executeActivity\|go.*Execute' --type go | head -20

Repository: deepnoodle-ai/workflow

Length of output: 48

---

🏁 Script executed:

# Look at execution.go to understand the execution flow
wc -l execution.go
head -200 execution.go

Repository: deepnoodle-ai/workflow

Length of output: 7759

---

🏁 Script executed:

# Check how the orchestrator processes steps and activities
rg 'orchestrator\|worker' --type go -B 2 -A 5 | head -100

Repository: deepnoodle-ai/workflow

Length of output: 48

---

🏁 Script executed:

# Search for how branches are executed - look for goroutine spawning patterns
rg 'runBranch\|processBranch' --type go -A 20 | head -100

Repository: deepnoodle-ai/workflow

Length of output: 48

---

🏁 Script executed:

# Look for the branch execution goroutine pattern
rg 'go func' --type go execution.go | head -20

Repository: deepnoodle-ai/workflow

Length of output: 88

---

🏁 Script executed:

# Search for how the orchestrator runs branches
rg 'type branch struct' --type go -A 30

Repository: deepnoodle-ai/workflow

Length of output: 1209

---

🏁 Script executed:

# Check execution.go for branch execution patterns
sed -n '400,600p' execution.go

Repository: deepnoodle-ai/workflow

Length of output: 6807

---

🏁 Script executed:

# Find the full goroutine pattern that spawns branches
rg 'go func.*branch' --type go -B 5 -A 15

Repository: deepnoodle-ai/workflow

Length of output: 877

---

🏁 Script executed:

# Look for the run() method to understand branch execution
rg 'func.*\(e \*Execution\) run' --type go -A 50

Repository: deepnoodle-ai/workflow

Length of output: 6750

---

🏁 Script executed:

# Check how activities are invoked within the branch execution
rg 'activity.Execute\|executeActivity' --type go -B 5 -A 10 | head -80

Repository: deepnoodle-ai/workflow

Length of output: 48

---

🏁 Script executed:

# Verify the testing.T behavior with FailNow from goroutines
python3 << 'EOF'
# From the internal/require code, when require.True/Equal fails:
# 1. It calls fail(t, msg, msgAndArgs)
# 2. fail() calls t.Errorf() and then t.FailNow()
# 3. FailNow() is *testing.T.FailNow()

# From Go's testing package documentation:
# - FailNow() marks the test as failed and stops execution of the test.
# - Must be called from the test goroutine
# - Calling from non-test goroutines is undefined behavior

print("Go testing.T.FailNow() behavior:")
print("- Designed to be called from the test goroutine only")
print("- Calling from activity goroutines is problematic:")
print("  * May not properly mark test as failed")
print("  * May cause panic or deadlock")
print("  * Test state becomes unpredictable")
print("\nThe review comment's concern is valid:")
print("- Activities execute in branch goroutines")
print("- require.* calls FailNow() in those goroutines")
print("- This violates Go's testing API contract")
EOF

Repository: deepnoodle-ai/workflow

Length of output: 484

---

🏁 Script executed:

# Search for branches being spawned and executed
sed -n '600,900p' execution.go

Repository: deepnoodle-ai/workflow

Length of output: 9771

---

🏁 Script executed:

# Check if there are any other patterns in the test file that show how to properly handle activity assertions
rg 'recordBranchExecution\|return.*error' execution_test.go -B 5 -A 5 | head -60

Repository: deepnoodle-ai/workflow

Length of output: 48

---

🏁 Script executed:

# Look for how the test checks results after execution
sed -n '1200,1250p' execution_test.go

Repository: deepnoodle-ai/workflow

Length of output: 1738

---

🏁 Script executed:

# Look at what recordBranchExecution does and how the test verifies results
sed -n '1050,1150p' execution_test.go

Repository: deepnoodle-ai/workflow

Length of output: 3236

---

🏁 Script executed:

# Check if there are examples of activities returning errors that get checked
rg 'return.*error' execution_test.go | grep -v 'nil' | head -20

Repository: deepnoodle-ai/workflow

Length of output: 279

---

🏁 Script executed:

# Verify the testing.T interface in internal/require to understand the contract
grep -A 5 'type TestingT interface' internal/require/require.go

Repository: deepnoodle-ai/workflow

Length of output: 151

---

Move assertion logic out of activity goroutines.

Activities execute in branch goroutines, not the test goroutine. Calling require.* there invokes FailNow() from a non-test goroutine, violating Go's testing API and potentially leaving the test in an unclear or hanging state. Return an error from the activity when validation fails, and assert on the result in the test body instead.

Example pattern (lines 1151-1192, 1479-1496)

Instead of:

ActivityFunc("modify_state_alpha", func(ctx Context, params map[string]any) (any, error) {
	counter, ok := ctx.Get("shared_counter")
	require.True(t, ok)
	require.Equal(t, 100, counter)
	// ...
	return "alpha-200", nil
})

Return an error and assert in the test body:

ActivityFunc("modify_state_alpha", func(ctx Context, params map[string]any) (any, error) {
	counter, ok := ctx.Get("shared_counter")
	if !ok || counter != 100 {
		return nil, errors.New("shared_counter validation failed")
	}
	// ...
	return "alpha-200", nil
})

Then in the test:

result, err := execution.Execute(ctx)
require.NoError(t, err)
require.Equal(t, ExecutionStatusCompleted, result.Status)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@execution_test.go` around lines 1151 - 1192, The activities
modify_state_alpha, modify_state_beta, and modify_state_gamma perform test
assertions using require.* inside activity goroutines, which violates Go's
testing rules; change each activity to validate via plain condition checks and
return an error (e.g., return nil, errors.New("...") ) when validation fails
instead of calling require.*, then move the require.NoError / require.Equal
assertions into the test body after calling execution.Execute to assert overall
execution status and errors from those activities.