-
Notifications
You must be signed in to change notification settings - Fork 56
docs(adr): amend ADR 48 for dispatch version-skew and migration #2453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,7 +56,11 @@ Fullsend must make users aware of the implications of choosing a moving tag: | |
| ## Consequences | ||
|
|
||
| * `v0` should be migrated to the new moving tag and deleted. | ||
| * Current users track the new floating tag automatically to keep behavior consistent. | ||
| * ~~Current users track the new floating tag automatically to keep behavior consistent.~~ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] amendment-annotation-style The (added 2026-06-19) annotation format and strikethrough markdown are novel in this codebase's ADR corpus. No other ADR uses either pattern. |
||
| * Current users have a period of transition in which version to track, after a while we introduce | ||
| breaking changes to the workflows. | ||
|
Comment on lines
58
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Adr 0048 consequences rewritten docs/ADRs/0048-automatic-updates.md is an already Accepted ADR, but the PR changes the meaning of a Consequences bullet (replacing the original outcome with a new transition/breaking-changes statement). Accepted ADR sections should only receive minor annotations/cross-references, not substantive content rewrites. Agent Prompt
|
||
| * New users track the version tag they install at. | ||
|
|
||
| See [Automatic Updates](../plans/automatic-updates.md) for the design details. | ||
| See [Automatic Updates](../plans/automatic-updates.md) for the design details, | ||
| including amendments for the dispatch version-skew problem and the | ||
| release-branch solution (added 2026-06-19). | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -107,6 +107,80 @@ with: | |
| fullsend_cli_ref: v0.15.0 | ||
| ``` | ||
|
|
||
| ## 2026-06-19 amendments | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] adr-amendment-scope 74 lines of new content with design decisions and rejected alternatives added to a plan document. The volume and ADR-style language suggest these decisions could warrant a separate ADR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] design-coherence The amendments use ADR-style rejected/accepted language for options, which is stylistically unusual for a plan document. |
||
|
|
||
| ### Per-repo `reusable-dispatch.yml` hardcoded `v0` overlook | ||
|
|
||
| The initial plan to implement this overlooked that `reusable-dispatch.yml` hardcodes | ||
| `v0` when triggering other workflows (`reusable-triage.yml` for example). This only | ||
| happens on per-repo mode, as per-org uses its own `dispatch.yml`. | ||
|
|
||
| This is a problem, as the version transmitted to the `reusable-dispatch.yml` from the | ||
| shim can't be used to call the appropriate version of `reusable-<stage>.yml`. | ||
|
|
||
| There are a couple of options: | ||
|
|
||
| * **Re-introduce `reusable-dispatch.yml` to the user repository** - rejected | ||
| as we already extracted it to reduce update noise in the user's repository | ||
| * **Convert dispatch to a composite action** - rejected because actions | ||
| can't spawn jobs naturally, and can't use `workflow_call`. | ||
| * **Commit changes to reusable-dispatch.yml on release** - accepted, more details | ||
| below. | ||
|
|
||
| #### Commit changes to reusable-dispatch.yml on release | ||
|
|
||
| This approach changes our release process to use tags on release branches instead of | ||
| tags in the `main` branch. This enables us to continue using `main` and have there `main` | ||
| hardcoded. | ||
|
|
||
| 1. `git checkout main && git fetch origin && git pull` | ||
| 1. Decide the next version based on the previous tags and the difference between HEAD and the tag. | ||
| 1. `git checkout -b release-${VERSION}` | ||
| 1. Make changes to `reusable-dispatch.yml` to change `uses:...@v0` to `uses:...@${VERSION}`. | ||
| 1. Make changes to `reusable-dispatch.yml` to change defaults values for `fullsend_actions_ref`, | ||
| `fullsend_cli_ref`, `fullsend_ai_ref` and `fullsend_version` from `v0` to `${VERSION}`. | ||
| 1. Commit the changes to the branch. | ||
| 1. Tag the branch with `${VERSION}` and `latest`. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] edge-case-correctness The release procedure does not mention force-pushing the latest tag, which will fail on subsequent releases because latest already exists on the remote. Suggested fix: Clarify step 7/8 to specify that the latest tag must be force-pushed. |
||
| 1. Push branch and tag. | ||
|
|
||
| ### The `main` branch will use `@main` | ||
|
|
||
| Currently `reusable-dispatch.yml` uses `@v0` on the `main` branch, that should be changed | ||
| to `@main` on `uses:` and other variables, so pointing to `main` on the shim will have the | ||
| desired effect of tracking the development changes. | ||
|
|
||
| ### Period of migration | ||
|
|
||
| The ADR proposed that current users would migrate automatically to follow the new | ||
| floating tag `latest`. However at implementation time a limitation has been detected: | ||
| if `v0` is changed to the new changes (or dropped) users will break. There seems to | ||
| be two solutions: | ||
|
|
||
| * Do not update anymore `v0`. Users will keep pulling from `v0` and they will be behind, | ||
| so a communication needs to happen so they run install commands again to refresh | ||
| their shims. Rejected. | ||
| * Update `v0` a last time to these changes. This would mean that workflows would | ||
| need to preserve behaviour. Accepted, more details below. | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] internal-consistency The fallback snippet uses fullsend_cli_ref and fullsend_actions_ref as primary input names, but downstream stage workflows only accept fullsend_version and fullsend_ai_ref. The plan does not document updating stage workflow input declarations. Suggested fix: Add an explicit step noting that all reusable-{stage}.yml workflow input declarations must be updated to accept the new input names before the release-branch process is executed. |
||
| #### Period of migration by moving `v0` | ||
|
|
||
| Moving `v0` one last time to these new changes mean that updated workflows | ||
| receive `fullsend_ai_ref` and `fullsend_version` so they can't be removed. Instead | ||
| they are used as a fallback to preserve behavior: | ||
|
|
||
| ```yaml | ||
| # reusable-dispatch.yaml/dispatch.yaml | ||
| triage: | ||
| uses: ... | ||
| with: | ||
| fullsend_actions_ref: ${{ inputs.fullsend_actions_ref || inputs.fullsend_ai_ref }} | ||
| fullsend_cli_ref: ${{ inputs.fullsend_cli_ref || inputs.fullsend_version }} | ||
| ``` | ||
|
|
||
| A deprecation notice needs to happen, so users install again to get their | ||
| shims refreshed with the new variables. After a while we can remove the old | ||
| variables from the workflows. Everyone not migrated by then will be broken. | ||
|
|
||
| ## Some Future Problems | ||
|
|
||
| * Currently images are not versioned, they just have the `latest` tag. This needs to | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[low] internal-consistency
The original ADR consequence is struck through and replaced with materially different text, which is borderline with the ADR policy of not substantially rewriting Consequences sections.