ADR-361: Wire per-Step EVENT_NAME through workflow-orchestrate dispatch to run-event-hooks - #97
Merged
Merged
Conversation
…criptor field Each Step subclass that dispatches exactly one spawn_agent/run_script action now declares a stable EVENT_NAME (debug, research, implement, validate, create-pr, review, fix, hand-off). DevTeamPipeline._do_get_actions_and_exit() injects an "event" key into every emitted descriptor when the dispatching step has an EVENT_NAME, and omits it otherwise. spec-finding and signoff (and signoff's own three child steps) intentionally have no EVENT_NAME.
Both the spawn_agent and run_script prompt templates now pass --event <item.event> to workflow-worker/workflow-script, following the same omission rule already applied to empty --skill-args/--command.
…ent hooks Adds an optional --event argument. When present, run-event-hooks is invoked before the wrapped skill (phase=before, no outcome) and after it (phase=after, outcome derived from whether the skill completed and wrote its output). A run-event-hooks 'failed' result from either call folds into workflow-worker's own step 5 return, even if the wrapped skill itself succeeded. When --event is absent, no hook calls are made — behavior is unchanged from before this argument existed.
…ooks Adds an optional --event argument, wrapping the command run with run-event-hooks the same way workflow-worker does. The after-hook's outcome is computed from the validation result itself (whether the last non-empty log line starts with 'Succeeded', mirroring ValidateStep.handle_results()'s own check) rather than from the command's exit code or this skill's existing 'a build/test failure is still a successful script run' return contract. A run-event-hooks failure folds into the overall return even when the base script-run result was successful. When --event is absent, behavior is unchanged.
build-and-test: Python test resultsStatus: ✅ Passed Test log |
jodavis-claude
commented
Jul 29, 2026
jodavis-claude
left a comment
jodavis-claude
left a comment
Collaborator
Author
There was a problem hiding this comment.
Reviewed ADR-361 against _spec_WorkflowEventHooks.md (ADR-361 task section, lines 703-746) and the task brief. The core wiring is solid: EVENT_NAME is declared on exactly the right Step subclasses (matching the spec's table 1:1), the descriptor-injection point in _do_get_actions_and_exit() correctly avoids leaking onto SignoffStep/its children, and both workflow-orchestrate/SKILL.md's dispatch templates and the workflow-worker/workflow-script before/after hook wrapping + outcome-folding logic look correct. python3 -m pytest plugins/dev-team/skills -q passes (378 passed, 1 pre-existing unrelated failure in test_concurrent_schedule.py caused by this repo's own max-parallel-tasks: 2 config, confirmed unrelated to this change).
Two issues below — one requirements gap, one minor doc-conformance note.
… wrapping Addresses PR #97 review comment (Priority 1, blocking): the exit criteria's testing plan required exercising both the spawn_agent dispatch path (workflow-worker) and the run_script dispatch path (workflow-script) against run-event-hooks with a throwaway instructions fixture, but no evidence of this was in the PR. Adds plugins/dev-team/fixtures/workflow-event-dispatch/ following the same model as fixtures/run-event-hooks/ (ADR-360): build_fixture.py materializes 4 target/scenario combinations (worker/script x with-event/no-event), each with a throwaway git repo and context file carrying a fictional `fizzle` event's instructions map (commit-producing before-hook, unrecognized- instruction after-hook). test_build_fixture.py (18 tests) confirms each combination's fixture state is correct. RUN.md documents the materialize -> dry-run -> grade procedure. Actually executed all 4 dry runs via fresh sub-agent sessions during this fix and confirmed: the with-event scenarios show the before-hook committing first, the wrapped skill/command running, and the after-hook's unrecognized instruction flipping the overall result to failure even though the wrapped invocation itself succeeded; the no-event scenarios show zero hook invocations and byte-for-byte `successful` results despite the same instructions map being present but unused.
…dering Addresses PR #97 review comment (Priority 5, non-blocking): the Data Flow section (_spec_WorkflowEventHooks.md) stated the after-hook runs "before writing the skill's own output to the context file," but workflow-worker's actual implemented order is write-then-after-hook (step 3, then step 4). Chose to update the spec to describe the implemented ordering rather than reorder the implementation, since the write happens regardless of the hook's outcome either way and there's no concurrency concern within one agent session -- reordering the implementation would be a behavior change with no benefit, whereas updating the doc to match is a pure documentation fix per CONTRIBUTING's "when designs are updated, documents should be updated to match."
jodavis-claude
commented
Jul 29, 2026
jodavis-claude
left a comment
jodavis-claude
left a comment
Collaborator
Author
There was a problem hiding this comment.
Sign-off review for ADR-361.
Both threads from the prior review pass were addressed with verifiable evidence and are now resolved:
- Testing plan (Priority 1, requirements) — a new scripted fixture harness at
plugins/dev-team/fixtures/workflow-event-dispatch/(build_fixture.py, test_build_fixture.py with 18 passing tests, RUN.md) mirrors ADR-360's precedent and covers all 4 target/scenario combinations. The PR reply documents 4 actual dry runs executed via fresh sub-agent sessions with concrete before/after-hook evidence (commit counts, failure attribution flipping the overall result). Re-ran the fixture's own test suite locally: 18 passed. - Spec Data Flow ordering (Priority 5, documentation) —
_spec_WorkflowEventHooks.md's Data Flow section (bullet 3) now accurately describes the implemented write-then-after-hook order, with the rationale for not reordering the implementation noted inline, per CONTRIBUTING's doc-sync guidance.
Scanned the modified files (_spec_WorkflowEventHooks.md, the new fixture directory) for new issues — none found. Re-ran plugins/dev-team/skills/workflow-orchestrate/scripts/test_dev_team.py (130 passed) to confirm no regressions from the fix commits.
Approving sign-off.
jodavis-claude
marked this pull request as ready for review
July 29, 2026 05:48
jodavis
approved these changes
Jul 29, 2026
jodavis-claude
added a commit
that referenced
this pull request
Jul 30, 2026
…ch to run-event-hooks (#97) * ADR-361: add EVENT_NAME per single-action Step and inject "event" descriptor field Each Step subclass that dispatches exactly one spawn_agent/run_script action now declares a stable EVENT_NAME (debug, research, implement, validate, create-pr, review, fix, hand-off). DevTeamPipeline._do_get_actions_and_exit() injects an "event" key into every emitted descriptor when the dispatching step has an EVENT_NAME, and omits it otherwise. spec-finding and signoff (and signoff's own three child steps) intentionally have no EVENT_NAME. * ADR-361: pass --event through workflow-orchestrate's dispatch prompt Both the spawn_agent and run_script prompt templates now pass --event <item.event> to workflow-worker/workflow-script, following the same omission rule already applied to empty --skill-args/--command. * ADR-361: wrap workflow-worker's skill invocation with before/after event hooks Adds an optional --event argument. When present, run-event-hooks is invoked before the wrapped skill (phase=before, no outcome) and after it (phase=after, outcome derived from whether the skill completed and wrote its output). A run-event-hooks 'failed' result from either call folds into workflow-worker's own step 5 return, even if the wrapped skill itself succeeded. When --event is absent, no hook calls are made — behavior is unchanged from before this argument existed. * ADR-361: wrap workflow-script's command run with before/after event hooks Adds an optional --event argument, wrapping the command run with run-event-hooks the same way workflow-worker does. The after-hook's outcome is computed from the validation result itself (whether the last non-empty log line starts with 'Succeeded', mirroring ValidateStep.handle_results()'s own check) rather than from the command's exit code or this skill's existing 'a build/test failure is still a successful script run' return contract. A run-event-hooks failure folds into the overall return even when the base script-run result was successful. When --event is absent, behavior is unchanged. * ADR-361: Add workflow-event-dispatch fixture harness for --event hook wrapping Addresses PR #97 review comment (Priority 1, blocking): the exit criteria's testing plan required exercising both the spawn_agent dispatch path (workflow-worker) and the run_script dispatch path (workflow-script) against run-event-hooks with a throwaway instructions fixture, but no evidence of this was in the PR. Adds plugins/dev-team/fixtures/workflow-event-dispatch/ following the same model as fixtures/run-event-hooks/ (ADR-360): build_fixture.py materializes 4 target/scenario combinations (worker/script x with-event/no-event), each with a throwaway git repo and context file carrying a fictional `fizzle` event's instructions map (commit-producing before-hook, unrecognized- instruction after-hook). test_build_fixture.py (18 tests) confirms each combination's fixture state is correct. RUN.md documents the materialize -> dry-run -> grade procedure. Actually executed all 4 dry runs via fresh sub-agent sessions during this fix and confirmed: the with-event scenarios show the before-hook committing first, the wrapped skill/command running, and the after-hook's unrecognized instruction flipping the overall result to failure even though the wrapped invocation itself succeeded; the no-event scenarios show zero hook invocations and byte-for-byte `successful` results despite the same instructions map being present but unused. * ADR-361: Update spec's Data Flow section to match implemented hook ordering Addresses PR #97 review comment (Priority 5, non-blocking): the Data Flow section (_spec_WorkflowEventHooks.md) stated the after-hook runs "before writing the skill's own output to the context file," but workflow-worker's actual implemented order is write-then-after-hook (step 3, then step 4). Chose to update the spec to describe the implemented ordering rather than reorder the implementation, since the write happens regardless of the hook's outcome either way and there's no concurrency concern within one agent session -- reordering the implementation would be a behavior change with no benefit, whereas updating the doc to match is a pure documentation fix per CONTRIBUTING's "when designs are updated, documents should be updated to match." --------- Co-authored-by: Joe Davis <ElwoodMoves@hotmail.com>
jodavis-claude
added a commit
that referenced
this pull request
Jul 30, 2026
…ch to run-event-hooks (#97) * ADR-361: add EVENT_NAME per single-action Step and inject "event" descriptor field Each Step subclass that dispatches exactly one spawn_agent/run_script action now declares a stable EVENT_NAME (debug, research, implement, validate, create-pr, review, fix, hand-off). DevTeamPipeline._do_get_actions_and_exit() injects an "event" key into every emitted descriptor when the dispatching step has an EVENT_NAME, and omits it otherwise. spec-finding and signoff (and signoff's own three child steps) intentionally have no EVENT_NAME. * ADR-361: pass --event through workflow-orchestrate's dispatch prompt Both the spawn_agent and run_script prompt templates now pass --event <item.event> to workflow-worker/workflow-script, following the same omission rule already applied to empty --skill-args/--command. * ADR-361: wrap workflow-worker's skill invocation with before/after event hooks Adds an optional --event argument. When present, run-event-hooks is invoked before the wrapped skill (phase=before, no outcome) and after it (phase=after, outcome derived from whether the skill completed and wrote its output). A run-event-hooks 'failed' result from either call folds into workflow-worker's own step 5 return, even if the wrapped skill itself succeeded. When --event is absent, no hook calls are made — behavior is unchanged from before this argument existed. * ADR-361: wrap workflow-script's command run with before/after event hooks Adds an optional --event argument, wrapping the command run with run-event-hooks the same way workflow-worker does. The after-hook's outcome is computed from the validation result itself (whether the last non-empty log line starts with 'Succeeded', mirroring ValidateStep.handle_results()'s own check) rather than from the command's exit code or this skill's existing 'a build/test failure is still a successful script run' return contract. A run-event-hooks failure folds into the overall return even when the base script-run result was successful. When --event is absent, behavior is unchanged. * ADR-361: Add workflow-event-dispatch fixture harness for --event hook wrapping Addresses PR #97 review comment (Priority 1, blocking): the exit criteria's testing plan required exercising both the spawn_agent dispatch path (workflow-worker) and the run_script dispatch path (workflow-script) against run-event-hooks with a throwaway instructions fixture, but no evidence of this was in the PR. Adds plugins/dev-team/fixtures/workflow-event-dispatch/ following the same model as fixtures/run-event-hooks/ (ADR-360): build_fixture.py materializes 4 target/scenario combinations (worker/script x with-event/no-event), each with a throwaway git repo and context file carrying a fictional `fizzle` event's instructions map (commit-producing before-hook, unrecognized- instruction after-hook). test_build_fixture.py (18 tests) confirms each combination's fixture state is correct. RUN.md documents the materialize -> dry-run -> grade procedure. Actually executed all 4 dry runs via fresh sub-agent sessions during this fix and confirmed: the with-event scenarios show the before-hook committing first, the wrapped skill/command running, and the after-hook's unrecognized instruction flipping the overall result to failure even though the wrapped invocation itself succeeded; the no-event scenarios show zero hook invocations and byte-for-byte `successful` results despite the same instructions map being present but unused. * ADR-361: Update spec's Data Flow section to match implemented hook ordering Addresses PR #97 review comment (Priority 5, non-blocking): the Data Flow section (_spec_WorkflowEventHooks.md) stated the after-hook runs "before writing the skill's own output to the context file," but workflow-worker's actual implemented order is write-then-after-hook (step 3, then step 4). Chose to update the spec to describe the implemented ordering rather than reorder the implementation, since the write happens regardless of the hook's outcome either way and there's no concurrency concern within one agent session -- reordering the implementation would be a behavior change with no benefit, whereas updating the doc to match is a pure documentation fix per CONTRIBUTING's "when designs are updated, documents should be updated to match." --------- Co-authored-by: Joe Davis <ElwoodMoves@hotmail.com>
jodavis
added a commit
that referenced
this pull request
Jul 30, 2026
…ngs (#118) * Add Workflow Event Hooks dev spec (ADR-337) Specs the inversion of git-repo's scattered commit/push/create-pr/promote-pr config blocks into a single instructions: section, executed via a new run-event-hooks mechanism. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5yAniX89Mf8RJmGFrYhBU * ADR-360: Add instructions: config schema, shipped defaults, and run-event-hooks skill - get-project-configuration/SKILL.md documents the new `instructions:` section (event name -> ordered label -> instruction map) at convention level, and drops the removed git-repo.commit/.push/.create-pr/.promote-pr documentation. - assets/default-config.yaml gains `instructions:` with every EVENT_NAME from the spec's Key Design Decisions table, populated with the shipped defaults where specified and null elsewhere; git-repo.commit/.push/.create-pr/ .promote-pr removed (git-repo keeps only user-alias/working-branches). - This repo's own .dev-team/config.yaml drops the same dead git-repo blocks and sets instructions.after-hand-off.request-review/.assign-work-item to the real reviewer identity (jodavis / jodasoft@outlook.com). - New run-event-hooks skill implements the full before-<event> / after-<event>-success/-failure/-after lookup-and-follow sequence, with a scripted fixture harness (plugins/dev-team/fixtures/run-event-hooks/) modeled on resolve-rebase-conflict's fixture model, covering the three minimum scenarios from the spec against a fictional `fizzle` event. Key decision: merge_config.py's custom YAML parser doesn't support flow-style `{}` (documented but not enforced), so empty instruction events are written as blank values (parses to null) instead of literal `{}` in default-config.yaml -- confirmed via merge_config.py's own JSON output. Filed the underlying silent-mis-parse behavior as a separate issue (#95) rather than fixing merge_config.py itself, which is out of this task's scope. * ADR-361: Wire per-Step EVENT_NAME through workflow-orchestrate dispatch to run-event-hooks (#97) * ADR-361: add EVENT_NAME per single-action Step and inject "event" descriptor field Each Step subclass that dispatches exactly one spawn_agent/run_script action now declares a stable EVENT_NAME (debug, research, implement, validate, create-pr, review, fix, hand-off). DevTeamPipeline._do_get_actions_and_exit() injects an "event" key into every emitted descriptor when the dispatching step has an EVENT_NAME, and omits it otherwise. spec-finding and signoff (and signoff's own three child steps) intentionally have no EVENT_NAME. * ADR-361: pass --event through workflow-orchestrate's dispatch prompt Both the spawn_agent and run_script prompt templates now pass --event <item.event> to workflow-worker/workflow-script, following the same omission rule already applied to empty --skill-args/--command. * ADR-361: wrap workflow-worker's skill invocation with before/after event hooks Adds an optional --event argument. When present, run-event-hooks is invoked before the wrapped skill (phase=before, no outcome) and after it (phase=after, outcome derived from whether the skill completed and wrote its output). A run-event-hooks 'failed' result from either call folds into workflow-worker's own step 5 return, even if the wrapped skill itself succeeded. When --event is absent, no hook calls are made — behavior is unchanged from before this argument existed. * ADR-361: wrap workflow-script's command run with before/after event hooks Adds an optional --event argument, wrapping the command run with run-event-hooks the same way workflow-worker does. The after-hook's outcome is computed from the validation result itself (whether the last non-empty log line starts with 'Succeeded', mirroring ValidateStep.handle_results()'s own check) rather than from the command's exit code or this skill's existing 'a build/test failure is still a successful script run' return contract. A run-event-hooks failure folds into the overall return even when the base script-run result was successful. When --event is absent, behavior is unchanged. * ADR-361: Add workflow-event-dispatch fixture harness for --event hook wrapping Addresses PR #97 review comment (Priority 1, blocking): the exit criteria's testing plan required exercising both the spawn_agent dispatch path (workflow-worker) and the run_script dispatch path (workflow-script) against run-event-hooks with a throwaway instructions fixture, but no evidence of this was in the PR. Adds plugins/dev-team/fixtures/workflow-event-dispatch/ following the same model as fixtures/run-event-hooks/ (ADR-360): build_fixture.py materializes 4 target/scenario combinations (worker/script x with-event/no-event), each with a throwaway git repo and context file carrying a fictional `fizzle` event's instructions map (commit-producing before-hook, unrecognized- instruction after-hook). test_build_fixture.py (18 tests) confirms each combination's fixture state is correct. RUN.md documents the materialize -> dry-run -> grade procedure. Actually executed all 4 dry runs via fresh sub-agent sessions during this fix and confirmed: the with-event scenarios show the before-hook committing first, the wrapped skill/command running, and the after-hook's unrecognized instruction flipping the overall result to failure even though the wrapped invocation itself succeeded; the no-event scenarios show zero hook invocations and byte-for-byte `successful` results despite the same instructions map being present but unused. * ADR-361: Update spec's Data Flow section to match implemented hook ordering Addresses PR #97 review comment (Priority 5, non-blocking): the Data Flow section (_spec_WorkflowEventHooks.md) stated the after-hook runs "before writing the skill's own output to the context file," but workflow-worker's actual implemented order is write-then-after-hook (step 3, then step 4). Chose to update the spec to describe the implemented ordering rather than reorder the implementation, since the write happens regardless of the hook's outcome either way and there's no concurrency concern within one agent session -- reordering the implementation would be a behavior change with no benefit, whereas updating the doc to match is a pure documentation fix per CONTRIBUTING's "when designs are updated, documents should be updated to match." --------- Co-authored-by: Joe Davis <ElwoodMoves@hotmail.com> * ADR-363: Trim final-sign-off to near-no-op; split work-with-pr hand-off ops (#99) * ADR-361: add EVENT_NAME per single-action Step and inject "event" descriptor field Each Step subclass that dispatches exactly one spawn_agent/run_script action now declares a stable EVENT_NAME (debug, research, implement, validate, create-pr, review, fix, hand-off). DevTeamPipeline._do_get_actions_and_exit() injects an "event" key into every emitted descriptor when the dispatching step has an EVENT_NAME, and omits it otherwise. spec-finding and signoff (and signoff's own three child steps) intentionally have no EVENT_NAME. * ADR-361: pass --event through workflow-orchestrate's dispatch prompt Both the spawn_agent and run_script prompt templates now pass --event <item.event> to workflow-worker/workflow-script, following the same omission rule already applied to empty --skill-args/--command. * ADR-361: wrap workflow-worker's skill invocation with before/after event hooks Adds an optional --event argument. When present, run-event-hooks is invoked before the wrapped skill (phase=before, no outcome) and after it (phase=after, outcome derived from whether the skill completed and wrote its output). A run-event-hooks 'failed' result from either call folds into workflow-worker's own step 5 return, even if the wrapped skill itself succeeded. When --event is absent, no hook calls are made — behavior is unchanged from before this argument existed. * ADR-361: wrap workflow-script's command run with before/after event hooks Adds an optional --event argument, wrapping the command run with run-event-hooks the same way workflow-worker does. The after-hook's outcome is computed from the validation result itself (whether the last non-empty log line starts with 'Succeeded', mirroring ValidateStep.handle_results()'s own check) rather than from the command's exit code or this skill's existing 'a build/test failure is still a successful script run' return contract. A run-event-hooks failure folds into the overall return even when the base script-run result was successful. When --event is absent, behavior is unchanged. * ADR-363: Trim final-sign-off to near-no-op; work-with-pr hand-off ops split out final-sign-off now reports hand-off readiness only (writes Handoff Result); promote/assign/request-review is performed afterward by after-hand-off instructions via run-event-hooks. work-with-pr's fixed 3-step hand-off sequence becomes three independently-callable operations: convert-to-ready, request-review, assign-issue (via work-with-Jira-tasks). Removed all REVIEW_ASSIGNEE_EMAIL references; reviewer/assignee identity is now a literal argument the caller supplies. * ADR-363: Fix signoff-skip bug; hand-off hooks fire on after-signoff-success PR #99 review (jodavis, CHANGES_REQUESTED): reviewing's own "approved" trigger routed straight to handoff, bypassing signoff entirely on a clean first-pass review — so the signoff parallel checks (and any hand-off hooks tied to them) never ran unless a fixing-pr cycle happened first. Fixes that by always routing reviewing --> signoff on approval; only signoff's own approval can now reach handoff. Renames HandoffStep's EVENT_NAME from hand-off to signoff so a project's hand-off instructions (promote PR, request review, assign work item) are now configured under after-signoff-success (was after-hand-off) — matching the reviewer's request to tie this work to signoff succeeding rather than a separate handoff-specific event. Updated the shipped default config, this repo's own .dev-team/config.yaml, and every skill doc that referenced the old before-hand-off/after-hand-off keys or the hand-off EVENT_NAME example. Did not eliminate the handoff pipeline state/agent turn itself: only an agent session has Jira/GitHub MCP tool access, so *some* single-action dispatch is structurally required to run those instructions, and _spec_WorkflowEventHooks.md already records this same reviewer's own prior, deliberate rejection of inventing a two-phase composite step for signoff for exactly this reason. Kept HandoffStep minimal and reachable only after signoff approves instead. Also fixed the stale pre-ADR-363 handoff description in both workflow plan assets (tracked separately in issue #98) since this change touches the same paragraph anyway. Added regression tests parsing both shipped workflow assets directly to catch any future reintroduction of a reviewing --> handoff shortcut. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADR-363: Rename creating-pr/fixing-pr mermaid states to creating_pr/fixing_pr Hyphens aren't valid mermaid syntax in GitHub. Coordinated the rename across dev_team.py (step_handlers keys, handles attrs), both plan assets (fix-issue-plan.md, implement-task-plan.md), run-event-hooks/SKILL.md prose, and their tests (test_dev_team.py, test_task_readiness.py). EVENT_NAME values (create-pr, fix) and the fix-pr skill name are distinct strings and were left untouched. --------- Co-authored-by: Joe Davis <ElwoodMoves@hotmail.com> Co-authored-by: Joe Davis <jodasoft@outlook.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * ADR-364: Rewrite update-project-configuration's git-repo walkthrough as an instructions: walkthrough - Repoint Step 3's single-setting table row from git-repo.push/.create-pr/.promote-pr at instructions: - Drop Step 4's git-repo bullets for commit.when/push.enabled+.when/ create-pr.enabled+.draft+.when/promote-pr.enabled+.when - Add a new instructions subsection covering add/edit/disable (label: "") of per-event labeled instructions, and the hand-off/sign-off reviewer question (written into after-signoff-success, the schema's real key for that step) - Rephrase the no-push/PR-rights guidance as disabling labeled entries instead of a block-level enabled: false - Update the stale 'six sections' menu text to 'seven sections' * ADR-362: Make ValidateStep's push conditional on which validation path ran _commit_and_push() now only fires when ValidateStep.handle_results() sees the literal marker substring "(no validation script configured for this project)" in the captured validate_result. When a real validation script ran, workflow-script's after-validate-success push hook (ADR-361) already pushed, so the old unconditional call would have pushed twice. No PipelineContext field changes; SignoffStep's separate _commit_and_push() call site is untouched. * ADR-366: Remove default instructions that are specific to my personal projects --------- Co-authored-by: Joe Davis <ElwoodMoves@hotmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Claude Code acting for jodavis <UptownGiraffe@outlook.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Work item
ADR-361— give every single-spawn_agent/run_scriptStepindev_team.pya stableEVENT_NAME, thread it throughworkflow-orchestrate's dispatch prompt as--event, and extend bothworkflow-workerandworkflow-scriptto call the already-implementedrun-event-hooksskill (ADR-360) around their existing single invocation, completing the end-to-end wiring so a project'sinstructions:config actually executes during the automated pipeline.Changes
plugins/dev-team/skills/workflow-orchestrate/scripts/dev_team.py— added anEVENT_NAME: str | Noneclass attribute (defaultNone) toDebugStep,ResearchStep,ImplementStep,ValidateStep,CreatePrStep,ReviewStep,FixStep,FixPrStep,HandoffStep(debug,research,implement,validate,create-pr,review,fix,fix,hand-off).FindSpecStepandSignoffStep(and its three children) intentionally have noEVENT_NAME.DevTeamPipeline._do_get_actions_and_exit()now injects an"event"key into every emittedspawn_agent/run_scriptdescriptor when the dispatching step has anEVENT_NAME, and omits the key otherwise.plugins/dev-team/skills/workflow-orchestrate/scripts/test_dev_team.py— addedTestEventNamePerStepandTestEventFieldInjectiontest classes covering event-name declarations and descriptor injection.plugins/dev-team/skills/workflow-orchestrate/SKILL.md— bothspawn_agentandrun_scriptdispatch prompt templates now pass--event <item.event>through toworkflow-worker/workflow-script, following the same omission rule already used for empty--skill-args/--command.plugins/dev-team/skills/workflow-worker/SKILL.md— added an optional--event <name>argument. When present, callsrun-event-hooksbefore the wrapped<skill>(phasebefore, no outcome) and after it (phaseafter, outcome derived from whether<skill>completed and wrote its output). Arun-event-hooksfailedresult from either call folds into the skill's own final return, even if<skill>itself succeeded. Behavior is byte-for-byte unchanged when--eventis absent.plugins/dev-team/skills/workflow-script/SKILL.md— added the same optional--event <name>argument and before/afterrun-event-hookswrapping, computing the after-hookoutcomefrom the validation result itself (last non-empty log line starting with"Succeeded", mirroringValidateStep.handle_results()), computed before the log/result section is written, and kept independent of the skill's pre-existing "stillsuccessfuleven on build/test failure" contract.Design decisions
"event"descriptor field centrally inDevTeamPipeline._do_get_actions_and_exit()rather than editing every individualget_actions()body, since that is the single point where all descriptors already flow through beforeexit_with_actions(). Confirmed this does not leak ontoSignoffStep/ParallelStepschildren sinceSignoffStepitself has noEVENT_NAMEandParallelSteps.get_actions()only concatenates children's already-built action dicts.plugins/dev-team/fixtures/workflow-worker/) was added — the three dry-run scenarios (spawn_agent path, run_script path, plain no---eventinvocation) were exercised directly against ADR-360'srun-event-hooksand existing throwaway fixtures rather than as a new durable checked-in asset.pipeline_context.py, either mermaid workflow file, or theinstructions:config schema/shipped defaults — all confirmed already correct/out of scope per the task brief.outcomeforworkflow-workerreuses the skill's existing informal judgment (did the agent encounter a failure while running<skill>'s steps) — no new formal success/failure protocol was introduced for invoked skills, per the brief's stated default reading of the spec's Data Flow section.Testing completed
python3 -m pytest plugins/dev-team/skills/workflow-orchestrate/scripts/test_dev_team.py -q→ 130 passed.plugins/dev-team/skillssuite → 378 passed, 1 pre-existing unrelated failure (test_concurrent_schedule.py::TestMaxParallelTasks::test_max_parallel_tasks_defaults_to_three, reproduces identically on thedev/claude/ADR-360base branch, caused by this repo's.dev-team/config.yamlsettingmax-parallel-tasks: 2, not by any ADR-361 change).run-event-hooksand a throwawayinstructions:fixture per the task brief's testing plan: spawn_agent-shaped dry run throughworkflow-worker, run_script-shaped dry run throughworkflow-script, and a plain no---eventinvocation of each confirming byte-for-byte identical behavior to today.