docs(workflow): correct doc comments that describe fields and behaviour that do not exist

[sushant-me]

Fixes #1541 (items 1–5). Item 6 is deliberately not touched — as the issue says, it needs a decision rather than a drive-by edit.

What changed

Only doc comments. git diff -U0 filtered for non-comment lines is empty, so nothing executable is modified.

PendingRequest is not a field. Every occurrence in non-test code was inside a comment. The real carriers are NodeState.Interrupts and the unexported interruptSchemas map. Corrected at all eight sites the issue lists. One more was in a test comment in agent/workflowagent/hitl_test.go.

RunStateSessionKey does not exist. It was named once, in a comment.

Run state is not persisted in session.State. The only runtime path is agent/workflowagent/workflow.go:148 → ReconstructRunState, which rebuilds from session event history, and persistence.go:59 says so explicitly. A grep for session.State writes in non-test workflow code returns comments and nothing else. Corrected in state.go, workflow.go (including the Name() accessor) and agent/workflowagent/workflow.go (including the detectResume doc).

The workflow name is not a state key. Run prefixes the node path it records with it, except for a root wrapper; RunNode and Resume add no prefix — w.Name() is read in exactly one place.

Resume step 2 was wrong in a way that matters. A duplicate is not a no-op. A re-entry node (RerunOnResume, the default for DynamicNode) is set to NodePending and re-runs; a handoff node yields ErrNothingToResume. Both cases are now stated, because a reader who trusted the old sentence wrote no handling for either.

JoinNode — validateFanIn skips every *JoinNode (validation.go:377), so a conditional predecessor into a join is never rejected. The comment now says that, and describes the symptom rather than claiming rejection.

DefaultRetryConfig said "retry every error"; defaultShouldRetry excludes ErrInputValidation.

Two adjacent inaccuracies in text this change did not otherwise touch were also corrected, since they are the same class and sit in the same doc comments: NodeState.Interrupts claimed "Non-empty iff Status == NodeWaiting", and a test comment named PendingRequest.

Note on the earlier attempt

I opened this once as #1710 and closed it within minutes: the branch had been cut while
another branch was checked out, so it inherited an unrelated commit, and git add -A
swept in an untracked scratch directory. It carried tool/mcptoolset/ and an
auditprobe/ directory that have nothing to do with this issue. #1710 is closed.

This PR is rebuilt from origin/main (9026ee8) and contains one commit touching only
the nine files below. I have left the closed PR up rather than deleting it so the
mistake is visible.

Testing Plan

go build -mod=readonly ./workflow/... ./agent/workflowagent/...   -> exit 0
go test -race -mod=readonly -count=1 -shuffle=on ./workflow/... ./agent/workflowagent/...
    ok  google.golang.org/adk/v2/workflow
    ok  google.golang.org/adk/v2/agent/workflowagent
gofmt -l                                                          -> clean
go mod tidy -diff                                                 -> prints nothing

golangci-lint is not installed locally at the CI-pinned v2.3.1, so it was not run. The change is comment-only and has no lint surface beyond gofmt, but I would rather say that than imply a clean lint run.

Behaviour change for someone already on the current release: none. Comments only.

I did not run anything against a live model or API — none is involved.

On the wording

The comments about JoinNode and about Resume's duplicate handling went through three rounds of independent review, and the first two attempts at the JoinNode paragraph were wrong: I claimed a route-skipped predecessor "never releases the barrier", then that it "still completes" and contributes a nil entry. Both are false — the outcome depends on the graph and on completion order. The committed wording deliberately states only what I could reproduce: that it is unvalidated, that the failure is silent, that the join and downstream can be skipped, and that the run still reports success. I have not described the barrier mechanism, because I could not state it correctly.

The same process caught a version of the Resume comment that claimed a duplicate always yields ErrNothingToResume (re-entry nodes re-run), and a version of the Interrupts fix that would have introduced a false invariant.

If you would rather the JoinNode paragraph carry the precise mechanism, it needs someone who can verify both orderings — the reproductions I used are described above and I am happy to add them as a table-driven test if that is wanted.

[wolo-lab]

Most of these corrections check out against the code, and removing the session.State and PendingRequest references is a clear improvement. Two places still describe behavior the code doesn't have, and since accuracy is the point of this PR, they should be fixed before merge. One is below, and the other is inline on resume.go.

The workflow name doesn't separate history during reconstruction. The new New doc says the prefixed node path "is what ReconstructRunState matches on when it attributes history back to nodes", and advises distinct names so paths "remain distinguishable" (workflow/workflow.go#L225-L236). Reconstruction attributes an event through eventNodeName, which returns the first path segment that matches one of this workflow's node names and ignores the workflow-name segment entirely. wfA@1/asker@1, wfB@1/asker@1 and asker@1 all resolve to asker. What actually keeps two runs apart is the invocation-ID filter (persistence.go#L72-L78), and with an empty invocationID two workflows sharing a node name collide whatever their names are. The accurate statement is that Run prefixes the path with the name and reconstruction is scoped by invocation ID. The "Give a workflow a name whenever more than one can run in the same session" sentence carries the same claim, and so does the name field comment at workflow.go#L153-L156.

[wolo-lab]

Step 3 still says the asker "does NOT re-execute", which is true only for the handoff case and now contradicts the re-entry bullet in step 2 directly above it. Folding step 3 into the handoff bullet would remove the conflict.

The comment inside the function at resume.go#L111-L116 has the same problem. It still says the gating "keeps Resume idempotent: a duplicate turn ... reschedules nothing", which the new step 2 correctly says is false for re-entry nodes. A duplicate resume of a re-entry node re-runs it and its successor a second time, with no error.

[wolo-lab]

"Does not clear this field" reads as if Interrupts may still hold IDs for a WaitForOutput park. The WaitForOutput park sets no interrupt ID at all (scheduler.go#L850-L855), so the useful fact for a reader is the opposite: Status == NodeWaiting can come with an empty Interrupts.

[wolo-lab]

Nothing from the first call carries over to the second one, since RunState is rebuilt from event history on every turn. The duplicate is rejected because the response is already in history (so it doesn't count as answered this turn) and the successor is already completed. The rewrap also leaves "waiting node matches and" on a line of its own.

hitl_test.go#L76-L79 still says the test "proves resume goes through session.State", which is the same stale reference this PR removes elsewhere.

[wolo-lab]

Non-blocking: this reflowed line runs to about 110 columns, while the rest of the block wraps at about 66.

[sushant-me]

Thanks — both points were right, and the eventNodeName explanation is the part I had wrong in my head when I wrote it. I had assumed the workflow-name segment participated in attribution; taking the first matching node-name segment and ignoring it makes the whole "distinct name so the paths stay distinguishable" advice false, and the invocation-ID filter is what actually scopes a run.

Fixed in c77e3a1, 0c8bdf4, 959bbcf:

• workflow.go, name field — now describes the prefix as a label on the recorded path rather than a namespace, and says history is attributed by invocation ID.
• workflow.go, New doc — removed the claim that the prefixed path is what ReconstructRunState matches on, and the distinct-name advice. It now states that eventNodeName matches the first path segment naming one of the workflow's nodes and ignores the workflow-name segment, that two workflows sharing a node name resolve to that name regardless of what they are called, and that with an empty invocation ID they collide whatever their names are.
• resume.go, step 3 — "does NOT re-execute" was true only for the handoff case and contradicted the re-entry bullet in step 2 directly above it. It now scopes the handoff case and says the re-entry case re-runs with the response as its input.
• state.go, Interrupts — "that park does not clear this field" read as if a WaitForOutput park could leave IDs behind. Replaced with the accurate statement: the park creates no interrupt ID, so the field is empty.

Still to do, and I wanted to check rather than guess:

• scheduler.go ~L806 — non-blocking, but I agree; I will re-wrap that line to the block's width.
• hitl_test.go ~L120 — I will correct the comment to say RunState is rebuilt from event history each turn, so nothing carries over and the duplicate is rejected because the response was already consumed.

Comments only, no behaviour change. Say the word if you would rather I fold the two remaining ones into the same revision.

[sushant-me]

Both remaining points are done — the review is now fully addressed.

• scheduler.go ~L806 — agreed it was out of step with the block; re-wrapped to the surrounding ~66 columns, no wording change (cb2ecf9).
• hitl_test.go ~L120 — you were right that the comment implied state carried over. It now says nothing carries over because RunState is rebuilt from event history every turn, and the duplicate is rejected because the response it replays has already been consumed (b65e50e).

All six items from your review, comments only, no behaviour change:

workflow.go name field	prefix described as a label; attribution is by invocation ID
workflow.go New doc	eventNodeName ignores the workflow-name segment; shared node names collide with an empty invocation ID
resume.go step 3	scoped to handoff mode; re-entry re-runs, per step 2
state.go Interrupts	a WaitForOutput park creates no interrupt ID, so the field is empty
scheduler.go	re-wrapped
hitl_test.go	nothing carries over; the response was already consumed

One correction worth flagging: my first attempt at the New doc anchored mid-sentence and left the original claim above the correction, so the doc contradicted itself for one commit. Caught and removed in 69578ed685.

No rush on the re-review — thanks for the detail in the original, it made the mechanism clear.