Working PR for ADR-337: Invert task and PR behaviors in project settings - #118
Merged
Conversation
jodavis-claude
force-pushed
the
feature/ADR-337-config
branch
from
July 30, 2026 13:14
12d800a to
dc463b2
Compare
build-and-test: Python test resultsStatus: ✅ Passed Test log |
jodavis
marked this pull request as ready for review
July 30, 2026 13:19
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
…vent-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.
…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>
…ff 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>
…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'
…h 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.
jodavis-claude
force-pushed
the
feature/ADR-337-config
branch
from
July 30, 2026 13:21
dc463b2 to
0f43ad2
Compare
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.
No description provided.