docs(updates): add ADR discussing automatic versioning - #2079
Conversation
Site previewPreview: https://dee57b4e-site.fullsend-ai.workers.dev Commit: |
|
🤖 Finished Review · ✅ Success · Started 3:13 PM UTC · Completed 3:26 PM UTC |
ReviewFindingsCritical
Medium
Low
Info
Previous runReviewFindingsMedium
Low
Info
Previous run (2)ReviewFindingsMedium
Low
Info
Previous run (3)ReviewFindingsMedium
Low
Info
Previous run (4)ReviewFindingsMedium
Low
Info
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| ## Decision | ||
|
|
||
| Our decision is to provide two tags: | ||
|
|
There was a problem hiding this comment.
[medium] design-document-alignment
ADR 0031 established that thin callers pin upstream by tag (@v0) or SHA, and scaffold workflows hardcode @v0 references. This ADR proposes a 'latest' floating tag and states 'v0 should be migrated to the new floating tag' but does not address whether v0 continues to exist, is deprecated, or becomes an alias for latest. Since v0 is hardcoded in workflow uses: directives (which do not support expressions), the migration path is non-trivial and unaddressed.
| @@ -0,0 +1,60 @@ | |||
| --- | |||
| title: "43. Automatic Updates" | |||
| status: Accepted | |||
There was a problem hiding this comment.
[low] ADR-frontmatter
The relates_to field is empty ([]). This ADR should reference ADR 0031 (reusable workflows) and ADR 0035 (layered content resolution) which are the mechanisms enabling automatic updates.
| * Version tags that track releases (`vMAJOR.MINOR.PATCH`, already implemented). | ||
|
|
||
| By default Fullsend should be installed in a way that it tracks the version the binary | ||
| is from. Users should explicitly change something to track a new version tag or the |
There was a problem hiding this comment.
[low] missing-documentation-update
No user-facing documentation explains how to pin to a specific version, opt into the floating tag, or upgrade between versions. This is expected at the ADR stage but should be tracked for implementation.
| is from. Users should explicitly change something to track a new version tag or the | ||
| floating tag. | ||
|
|
||
| Fullsend must make users aware of the implications of choosing a floating tag: |
There was a problem hiding this comment.
[info] user-communication-gap
ADR states 'Fullsend must make users aware of the implications' but does not specify where this communication will happen (installation docs, action.yml descriptions, release announcements).
|
|
||
| ## Consequences | ||
|
|
||
| * `v0` should be migrated to the new floating tag. |
There was a problem hiding this comment.
[info] implementation-gap
No code in the codebase implements the migration logic described in consequences. Scaffold generation has no version-aware migration. This is expected for an ADR.
ralphbean
left a comment
ralphbean
left a comment
There was a problem hiding this comment.
Two things I think need sorting out before this lands — see inline.
9b8237e to
cef3a81
Compare
|
🤖 Finished Review · ✅ Success · Started 7:42 AM UTC · Completed 7:54 AM UTC |
|
|
||
| * Broken releases. | ||
| * Infrastructure changes required. | ||
|
|
There was a problem hiding this comment.
[medium] internal-consistency
The Consequences state "Current users track the new floating tag automatically" but the Decision states default should pin to binary version with explicit opt-in for the moving tag. These contradict each other.
| ## Context | ||
|
|
||
| Currently Fullsend uses a moving tag (`v0`) so users pick up the latest changes. When a release happens | ||
| a new tag `vMAJOR.MINOR.PATCH` gets created and the moving tag gets moved to the same SHA. New Fullsend |
There was a problem hiding this comment.
[medium] internal-consistency
The Context section conflates two distinct distribution channels (binary via action.yml latest and workflow refs via @v0) without distinguishing them. The Decision then proposes latest as the new moving tag name, colliding with the existing binary resolution mechanism.
| * Infrastructure changes required. | ||
|
|
||
| ## Consequences | ||
|
|
There was a problem hiding this comment.
[medium] design-document-alignment
v0 deletion understated: referenced in 17+ scaffold workflows, release process, and ADR 0031. No migration plan, timeline, or deprecation strategy specified.
Suggested fix: Add consequences acknowledging full migration scope: scaffold template updates, release process changes, coordinated downstream org migration, and deprecation period.
| Our decision is to provide two tags: | ||
|
|
||
| * Moving tag that tracks the latest release (probably called `latest`). | ||
| * Version tags that track releases (`vMAJOR.MINOR.PATCH` which area already created). |
There was a problem hiding this comment.
[medium] edge-case-correctness
Typo (area -> are) and misleading already created claim: semver tags exist but no scaffold workflow mechanism can consume them.
|
|
||
| * Moving tag that tracks the latest release (probably called `latest`). | ||
| * Version tags that track releases (`vMAJOR.MINOR.PATCH` which area already created). | ||
|
|
There was a problem hiding this comment.
[medium] implementation-ambiguity
Default to binary version does not specify the mechanism for each distribution channel. Implementation is ambiguous.
cef3a81 to
7b68b0c
Compare
|
🤖 Finished Review · ✅ Success · Started 8:23 AM UTC · Completed 8:39 AM UTC |
fullsend-ai-review
Bot
added
the
requires-manual-review
ralphbean
left a comment
ralphbean
left a comment
There was a problem hiding this comment.
LGTM. One non-blocking note inline.
| --- | ||
| title: "43. Automatic Updates" | ||
| status: Accepted | ||
| relates_to: [] |
There was a problem hiding this comment.
[nit] relates_to is empty — other ADRs in the repo populate this with problem doc filenames. agent-infrastructure seems like a fit here, since this is about the platform's own distribution mechanism.
7b68b0c to
129e5ab
Compare
|
🤖 Finished Review · ✅ Success · Started 8:56 AM UTC · Completed 9:13 AM UTC |
| @@ -0,0 +1,62 @@ | |||
| --- | |||
There was a problem hiding this comment.
[critical] adr-number-collision
ADR number 43 is already allocated on main branch to 'managed-file-headers' (accepted 2026-06-06). This PR introduces a second ADR 43 for 'automatic-updates', creating a numbering collision. ADR 44 is the next available number.
Suggested fix: Renumber this ADR to 0044. Update the filename to 0044-automatic-updates.md, change the title frontmatter to '44. Automatic Updates', update the heading, and update the reference in docs/plans/automatic-updates.md from 'ADR 43' to 'ADR 44'.
|
|
||
| * Broken releases. | ||
| * Infrastructure changes required. | ||
|
|
There was a problem hiding this comment.
[medium] internal-consistency
The Consequences state 'Current users track the new floating tag automatically' but the Decision states users must 'explicitly change something to track a new version tag or the moving tag.' These statements contradict each other regarding migration behavior for existing users.
| from the `config.yaml` and will pass it around. However the reusable workflows can't | ||
| benefit from this. | ||
|
|
||
| So the version pinning should happen another way. We will introduce a new parameter |
There was a problem hiding this comment.
[medium] design-implementation-mismatch
ADR 47 (vendored-installs-with-vendor-flag, accepted) explicitly lists '--upstream-ref CLI flags' under 'What this PR removes.' This design proposes reintroducing --upstream-ref, contradicting an accepted ADR.
Suggested fix: Reconcile with ADR 47's architecture, or explicitly note that this ADR supersedes portions of ADR 47.
|
|
||
| With `fullsend_ai_ref` and `fullsend_version` it is easy to control from a single | ||
| place which version should be use. A step in the shim would pull the version | ||
| from the `config.yaml` and will pass it around. However the reusable workflows can't |
There was a problem hiding this comment.
[medium] design-document-alignment
The design proposes FULLSEND_REF as a new template placeholder, but internal/scaffold/render.go already uses REUSABLE_DISPATCH and REUSABLE_WORKFLOW placeholders driven by config.DefaultUpstreamRef. No reconciliation plan is provided.
| # fullsend.yaml or <stage>.yml | ||
| uses: fullsend-ai/fullsend/.../reusable-*.yml@__FULLSEND_REF__ | ||
| with: | ||
| fullsend_actions_ref: __FULLSEND_REF__ |
There was a problem hiding this comment.
[medium] edge-case-correctness
A single FULLSEND_REF value is used for git refs, action checkout refs, and CLI binary versions, but these resolve through fundamentally different mechanisms. The example '--upstream-ref main' would produce fullsend_cli_ref: main, which is not a valid CLI binary version.
| Given that we are changing this code, we may as well update the variable names to reflect | ||
| better their real usage: | ||
|
|
||
| * `fullsend_ai_ref` -> `fullsend_actions_ref` |
There was a problem hiding this comment.
[low] variable-naming
fullsend_cli_ref is misleading — it is not a git ref to the CLI, it is a version/tag specifier.
| The template string will be `__FULLSEND_REF__`. | ||
|
|
||
| Given that we are changing this code, we may as well update the variable names to reflect | ||
| better their real usage: |
There was a problem hiding this comment.
[low] scope-alignment
Variable renaming proposed in the design but not authorized in the ADR itself.
| the decision itself needs to change, write a new ADR that supersedes this | ||
| one. For evolving design narrative, use docs/architecture.md. --> | ||
|
|
||
| ## Context |
There was a problem hiding this comment.
[low] incomplete-context
The ADR does not reference the existing versioning architecture in ADR 31, ADR 47, or config.DefaultUpstreamRef.
|
|
||
| ## Status | ||
|
|
||
| Accepted |
There was a problem hiding this comment.
[low] missing-architecture-update
Accepted ADRs require a corresponding update to docs/architecture.md per the writing-adrs skill.
| fullsend_actions_ref: v0.15.0 | ||
| fullsend_cli_ref: v0.15.0 | ||
| ``` | ||
|
|
There was a problem hiding this comment.
[low] design-completeness
Sandbox images are identified as unversioned but the ADR claims unified version control for all components.
fullsend-ai-review
Bot
removed
the
requires-manual-review
Signed-off-by: Hector Martinez <hemartin@redhat.com>
129e5ab to
8dc0b93
Compare
|
🤖 Finished Review · ✅ Success · Started 1:17 PM UTC · Completed 1:32 PM UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Review skipped — this PR is already merged. The Posted by fullsend post-review check |
|
🤖 Finished Retro · ✅ Success · Started 1:35 PM UTC · Completed 1:44 PM UTC |
Retro: PR #2079 — ADR for automatic versioningTimeline: A human-authored docs PR (2 new files: ADR + plan document) went through 5 review agent runs over 8 days before merging. The review agent correctly caught a critical ADR number collision (ADR 43 was already allocated), which was a genuine value-add. However, the review cycle was expensive relative to the change size. What went well:
Inefficiencies observed (all covered by existing issues):
No new proposals filed. All identified improvement opportunities are already tracked by existing open issues. Resolving the above issues (particularly #1900 and #1452) would have reduced this workflow from 5 review runs to ~2, saving significant token cost and review noise. |
2 participants
ADR for discussion about automatic versioning