ADR-367: Add architecture doc for Workflow Event Hooks - #119
Merged
Conversation
build-and-test: Python test resultsStatus: ✅ Passed Test log |
jodavis-claude
commented
Jul 30, 2026
jodavis-claude
left a comment
Collaborator
Author
There was a problem hiding this comment.
Reviewed _doc_WorkflowEventHooks.md (the only change in this PR) against the shipped implementation. Summary of verification performed:
- Cross-checked every factual claim against source:
Step.EVENT_NAMEtable (debug/research/implement/validate/create-pr/review/fix/signoff) indev_team.py,_commit_and_pushcall sites (only inValidateStep.handle_results()when no script is configured, and inSignoffStep.get_actions()),HandoffStep's docstring andEVENT_NAME = "signoff",CreatePrStep's unconditional dispatch,FindSpecStep/SignoffStephaving noEVENT_NAME. All match exactly. - Verified
run-event-hooks/SKILL.md,workflow-worker/SKILL.md,workflow-script/SKILL.md,final-sign-off/SKILL.md, andwork-with-pr/SKILL.md(hand-off operations section) against the doc's Key Classes/Interfaces and Data Flow claims — consistent throughout, including theREVIEW_ASSIGNEE_EMAILremoval. - Confirmed
assets/default-config.yamlonly shipspush/ensure-pushedas generic defaults, matching the doc's Key Design Decisions claim about the config-tier split. - Confirmed doc structure follows
write-repo-documentationconventions exactly:Summary:line, then only the five allowed sections in the required order, no implementation detail restated (links to source throughout). - Confirmed
_spec_WorkflowEventHooks.mdis unmodified by this PR (exit criterion #2) and the new doc exists at the correct location/name (exit criterion #1). - Checked
.editorconfigcompliance (UTF-8, LF, trailing final newline) — all good.
No Priority 1-4 issues found. One non-blocking observation: the doc's Key Design Decisions section includes the literal reviewer email (jodasoft@outlook.com) as a worked example of the config-tier split. This isn't new exposure — the same email is already committed in _spec_WorkflowEventHooks.md from an earlier PR — so it's consistent with existing precedent and not a blocker, just flagging for awareness since it's now in the permanent architecture doc as well.
Approving.
jodavis-claude
marked this pull request as ready for review
July 30, 2026 06:24
jodavis-claude
force-pushed
the
dev/claude/ADR-367
branch
from
July 30, 2026 13:13
4efbd9b to
5fd2a74
Compare
…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.
Adds _doc_WorkflowEventHooks.md describing the shipped instructions: config mechanism, the per-Step EVENT_NAME table, and the run-event-hooks before/after execution model built in ADR-360 through ADR-366. _spec_WorkflowEventHooks.md is left in place unchanged, per convention.
The content is covered in _doc_WorkflowEventHooks now.
jodavis-claude
force-pushed
the
dev/claude/ADR-367
branch
from
July 30, 2026 13:26
5fd2a74 to
6b22302
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
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-367 — Author
_doc_WorkflowEventHooks.md, a post-implementation architecture doc describing the shippedinstructions:config mechanism, the per-pipeline-stateEVENT_NAMEtable, and therun-event-hooksexecution model that Workflow Event Hooks (ADR-360 through ADR-366) actually built.Changes
_doc_WorkflowEventHooks.md(repo root), written perwrite-repo-documentationconventions: starts with aSummary:line, contains only the five allowed sections in order (Overview, Responsibilities & Boundaries, Key Design Decisions, Key Classes / Interfaces, Data Flow).instructions:config section (event name → orderedlabel: instructionmap) that replaces the oldgit-repo.commit/.push/.create-pr/.promote-prblocks, andrun-event-hooksas the mechanism that executes them.merge_config.py's existing recursive dict-merge, ordering guarantee across config tiers,create-prstaying structural/non-hookable-for-creation-itself,signoff's three parallel children andspec-findinghaving noEVENT_NAME,ValidateStep's conditional push) and links to source rather than restating implementation detail.run-event-hooks(event, phase, outcome, context_file), theEVENT_NAMEtable perStepsubclass, and the--eventargument onworkflow-worker/workflow-script."event"field fromworkflow-orchestratethrough to the before/after hook calls inworkflow-worker/workflow-script._spec_WorkflowEventHooks.mdleft in place, unmodified — it persists afterward for harvesting, per its own header and per exit criterion Fix repo structure to match what's expected for marketplace #2.Design decisions
signoff(not the spec's originalhand-off) — confirmed viadev_team.py'sHandoffStep.EVENT_NAME,default-config.yaml, andupdate-project-configuration/SKILL.md, all of which usesignoff. This is a deliberate description of what shipped, not an error to reconcile with the spec.write-repo-documentation's "no implementation detail, link to source instead" convention.write-repo-documentation's fixed section list doesn't include one; cross-references are inline links within the five allowed sections instead, matching_doc_Projects.md's existing precedent.Testing completed
None — this is a documentation-only task with no code changes, no unit tests, and no new externally-observable behavior for E2E coverage.
scripts/validate.shwas run as part of the pipeline's validation step.