docs: add ADR 0088 for tracker/forge harness config split - #6237
docs: add ADR 0088 for tracker/forge harness config split#6237ralphbean wants to merge 2 commits into
Conversation
Adds a tracker: harness key sibling to forge:, renaming ForgeConfig to PlatformConfig and generalizing its merge pipelines, so a code agent can read from one tracker (e.g. JIRA) while writing to an unrelated forge. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
PR Summary by QodoAdd ADR 0088 for tracker/forge harness config split
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
|
🤖 Finished Review · ✅ Success · Started 5:49 PM UTC · Completed 6:04 PM UTC Commit: |
Site previewPreview: https://3844b807-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Code Review by Qodo
1. docs/architecture.md missing ADR 0088
|
ReviewFindingsLow
Previous runReviewFindingsMedium
Low
Labels: PR adds an ADR about the harness config schema (tracker/forge split) |
|
Replying in reference to #6237 (comment) Addressed the architecture.md, multiple-decisions, and Consequences-bullets findings on their own threads:
On the "exceeds 100 lines" point — same reasoning as the multiple-decisions thread: I'm keeping this as one ADR since the rename, |
|
Replying in reference to #6237 (comment) Addressed the
On the label point — |
Fix Load()/LoadWithOpts function reference, add OpenShell.Profiles to the precedence-rules field list, and link ADR 0088 from architecture.md's Agent Harness "Decided:" list. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 6:22 PM UTC · Completed 6:38 PM UTC Commit: |
| This is scoped to `internal/harness`; it is unrelated to and does not touch | ||
| `internal/repos.ForgeConfig`, a separate type for admin-manifest CI paths. | ||
|
|
||
| Add a `Tracker map[string]*PlatformConfig` field to the `Harness` struct, |
There was a problem hiding this comment.
[low] scope-coherence
The ADR proposes renaming Go type ForgeConfig to PlatformConfig while the YAML key remains forge:. This introduces a naming divergence (the new tracker: YAML key would also be backed by PlatformConfig). The ADR acknowledges this as a deliberate trade-off, but worth confirming since ADR 0045 doc-comment explains why the type is currently named ForgeConfig.
| belong as a `forge:` key — while still opening a PR against GitHub or an MR | ||
| against GitLab, an unrelated forge. `forge:` has no way to express "read | ||
| from tracker X, write to forge Y" for the same harness; only one axis | ||
| exists, and JIRA doesn't fit on it. |
There was a problem hiding this comment.
Neither of those concerns are what the "forge" key deals with ATM - not should they be.
The key deals with conditioning certain configuration on the runtime its running on.Admittedly its design is rigid and assumes the runtime environment implies thing about the issue tracking and change tracking systems.
Neither the triggering issue system nor the code write systems should be a direct concern of the agent harness - the former can and should be inferred from the even data that caused the agent to trigger (indeed the same agent can be trggerd by events from multiple systems!), while the latter should be inferred from the repo-wide configuration, or possibly from the runtime in some cases (e.g. its ok to assume that if we are runing in GHA we probably want to create PRs for the repo that owns the workflow).
It seems I am completely rejecting the premise and direction of this ADR.
| - `compose.go`'s `mergeForgeBlocks`/`mergeForgeConfigInto` (the `base:` | ||
| composition merge) are likewise generalized to operate on either map, so | ||
| a `tracker:` block composes across `base:` the same way `forge:` already | ||
| does, without a second implementation. |
There was a problem hiding this comment.
I think that if we want to make various parts of the harness such as the scripts, the model, or others conditional upon various things like the system we are running on, the system we got the event from, or the system we would write the results to, if we pick and choose it ends up feeling rather arbitrary. I think we're better off generalizing this to conditional sections that fire when the triggering event or other factors match a particular CEL expression.
For eg:
trigger: ... # superset of all interesting events
pre-script: generic-read.sh
...
conditional-config:
- when: event.source.system == "github"
pre-script: github-comment-read.sh
- when: event.source.system == "jira"
pre-script: jira-comment-read.sh
...| - A code agent harness can declare `tracker.jira` (how to read/comment on | ||
| the triggering issue) alongside `forge.github` (how to open the resulting | ||
| PR) in one file — the two resolve independently against their own | ||
| platform values, with no `forge.jira` entry required or possible. |
There was a problem hiding this comment.
As mentioned before, I thing its a bad idea to put this in the harness, if anything e want to aim towards making agents platform agnostic, and all platform-specific concerns aught to be in the config.
This does not bar choosing between platform specific files within the agent when it simplifies its implementation, but ideally agent would always have files for all platforms. The ultimate combination of platforms used in practice for a particular repo needs to reside in the repo configuration file.
Summary
tracker:as a harness key sibling toforge:, so a code agent can read from one tracker (e.g. JIRA) while writing to an unrelated forge (GitHub/GitLab).internal/harness.ForgeConfigtoPlatformConfigand generalizes both existing merge pipelines (forge.goresolution,compose.gobase:composition) to operate on eitherforge:ortracker:, rather than duplicating them.fullsend-ai/agentsadd-jira-tracker-supportOpenSpec change, which depends on this schema/runtime piece landing here first.Test plan
make lintpasses (ADR frontmatter/status/number/link linters)