Skip to content

docs(adr): ADR-179, a dropped forced source is recorded on the run - #828

Merged
CybotTM merged 1 commit into
mainfrom
docs/adr-dropped-source
Aug 19, 2026
Merged

docs(adr): ADR-179, a dropped forced source is recorded on the run#828
CybotTM merged 1 commit into
mainfrom
docs/adr-dropped-source

Conversation

@CybotTM

@CybotTM CybotTM commented Aug 19, 2026

Copy link
Copy Markdown
Member

The decision behind #809, before any code. No behaviour changes here.

The problem: a run queued with a forced snippet or skill loses it if someone switches that record off before the run starts. The run proceeds without it and says nothing. The only way to notice is to compare the queued request against the transcript.

ADR-175 made this uniform rather than lopsided — before it, a snippet disappeared and a skill survived — which is what turns it into one question with one answer.

Three decisions

Recorded on the run, not only logged. A log answers "did it happen", which serves support. The operator asks "why did this run differ from the one I queued", and that is asked at the run. It travels on RunAugmentation from the rehydration that drops it and is written through RunTrace — ADR-151's channel, whose steps Backend/AgentRun/Show.html already renders. No new surface is invented.

The run is not refused. Refusing a queued run is defensible on cost — nothing has been sent, so it costs only a message. Rejected on who pays: switching off a snippet is routine operator maintenance, and making it fail other people's queued work turns a safe action into one nobody dares take. A run that proceeds and says so is recoverable; a refused run is somebody else's incident.

"Deleted" and "switched off" read differently. Both resolve to nothing today and a single "dropped" would flatten them. They are different actions with different remedies — one is reversible, the other is not — and a reader who cannot tell them apart has to go looking.

Deliberately out

  • The resume path. ADR-166 and ADR-175 keep a deactivated source resolving there on purpose, because its text is already in the transcript. Nothing is dropped, so a report would imply otherwise.
  • Notifications. The record is readable at the run; it does not push. Whether a dropped source deserves a notification is the same open question as every other pending decision here, and answering it for one case would settle it for all of them by accident.

One correction I made while writing this

I had told the user the anchor was step metadata, citing droppedTurns in ToolLoopService. That line is a log call, not a step. Its own comment names the real one — the context step ADR-151 records through RunTrace. The decision above uses that channel; the sentence I said out loud first was wrong.

Why this is its own PR

The implementation is roughly the size of #823: a field on RunAugmentation, the codec computing the difference, the trace write, the template, and tests. Splitting the decision out means it can be argued before that exists, and it gives an early checkpoint rather than a long silent stretch — which is the pattern that went wrong earlier today.

68 ADR-suite tests pass; the three repo checks pass. No PHP is touched.

Refs #809

Copilot AI lite review requested due to automatic review settings August 19, 2026 14:07
@CybotTM
CybotTM requested a review from a team as a code owner August 19, 2026 14:07
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Aug 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

github-actions[bot]
github-actions Bot previously approved these changes Aug 19, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval for maintainer PR

All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.

@CybotTM

CybotTM commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

Review record — the Copilot review did not run (monthly, account-wide quota), so decision=APPROVED is github-actions and not a reader. What I checked:

The anchor claim was wrong when I first made it, and I corrected it before writing the record. I had said droppedTurns in ToolLoopService hangs off step metadata. It is a log call. Its own comment names the real channel — the context step ADR-151 records through RunTrace, whose steps Backend/AgentRun/Show.html renders. The decision uses that, and the PR body says so rather than quietly using the right one.

Each of the three decisions carries its reason, and two of them turn on something other than cost. Not refusing the run is decided on who pays: switching off a snippet is routine maintenance, and making it fail other people's queued work turns a safe action into one nobody dares take. Splitting "deleted" from "switched off" is decided on remedy: one is reversible, the other is not, so a single "dropped" would send a reader looking.

The scope exclusions were checked against the records they cite, not asserted: ADR-166 and ADR-175 do keep a deactivated source resolving on a resume, so nothing is dropped there and a report would imply otherwise.

:Amends: pairing: ADR-175 carries the matching :Amended:. The pairing rule is enforced by AdrLifecycleTest, which I have seen fail on a missing half earlier today, so this is a checked property rather than a convention I followed.

68 ADR-suite tests pass; the three repo checks pass. No PHP is touched, so no CHANGELOG entry and no behaviour moves.

The implementation follows as its own PR — deliberately split, so the decision can be argued before the code exists.

Merging on that basis.

@CybotTM

CybotTM commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

Self-review: a1a2479

The review this pull request demands is unsatisfiable (Copilot quota wall or repeated bot failures on this head). Per the documented fallback, the diff on this head was reviewed by the PR author; this comment is the on-the-record attestation the merge gate reads back. It stops matching on the next push.

@CybotTM
CybotTM added this pull request to the merge queue Aug 19, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 19, 2026
@CybotTM
CybotTM added this pull request to the merge queue Aug 19, 2026
@CybotTM
CybotTM removed this pull request from the merge queue due to a manual request Aug 19, 2026
ADR-175 settled which sources enter a queued run when it is dequeued. It
left what the operator is told, which today is nothing: the source is
dropped, the run proceeds without it, and the only way to notice is to
compare the queued request against the transcript.

Three decisions, and the reasons are the load-bearing part.

Recorded on the run rather than only logged. A log answers "did it
happen"; the operator asks "why did this run differ from the one I
queued", and that is asked at the run. It travels on RunAugmentation from
the rehydration that drops it and is written through RunTrace — ADR-151's
channel, already rendered by Backend/AgentRun/Show.html. No new surface.

The run is not refused. Refusing is defensible on cost, and rejected on
who pays: switching off a snippet is routine maintenance, and making it
fail other people's queued work turns a safe action into one nobody dares
take.

Deleted and switched off read differently. A single "dropped" would
flatten two operator actions with different remedies — one is reversible,
the other is not.

Deliberately out: the resume path, where ADR-166 and ADR-175 keep a
deactivated source resolving on purpose, so nothing is dropped; and
notifications, because answering that here for one case would settle it
for all of them by accident.

Numbered 179, not 178. PR #827 claimed 178 eleven minutes before this
branch existed, and I had checked the highest number on main instead of
across the open branches — which is exactly the collision the worktree
rule warns about. The earlier claim keeps the number.

Refs #809

Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
@CybotTM
CybotTM force-pushed the docs/adr-dropped-source branch from a1a2479 to 367ed08 Compare August 19, 2026 19:02
@CybotTM CybotTM changed the title docs(adr): ADR-178, a dropped forced source is recorded on the run docs(adr): ADR-179, a dropped forced source is recorded on the run Aug 19, 2026
@CybotTM

CybotTM commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

Renumbered from ADR-178 to ADR-179, now 367ed088.

#827 claimed 178 at 14:04, eleven minutes before this branch existed. I had checked the highest number on main — where 177 was the top — instead of across the open branches, which is the collision the worktree rule exists to prevent. The earlier claim keeps the number.

Renamed the record, its .. _adr-179: label and title, the toctree entry, and ADR-175's :Amended: reference. The ADR suites pass (68 tests), including the pairing check that would have caught a half-renamed reference.

The previous self-review attestation died with this push, as designed. It rested on the old head and this is a different one.

The toctree now reads … 177, 179 with a gap where #827's 178 lands. Whichever of the two merges second will conflict on that line and the resolution is to keep both in ascending order — mechanical, and worth naming here so it is not re-derived under time pressure.

@sonarqubecloud

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval for maintainer PR

All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.

@CybotTM

CybotTM commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

Review record for head 367ed088 — the previous attestation died with the renumbering push, as designed, so this is a fresh read of the current head rather than a re-post.

What changed since the last record is the number and every reference to it: the record file, its .. _adr-179: label, the title, the toctree entry and ADR-175's :Amended: back-reference. The ADR suites pass (68 tests), and the pairing assertion among them is what would fail on a half-renamed reference — I have watched it fail on exactly that earlier today, so it is a checked property here and not a convention I trusted.

The decision content is unchanged and reviewed as before: recorded on the run rather than only logged, the run is not refused, and "deleted" reads differently from "switched off". The two exclusions were checked against the records they cite — ADR-166 and ADR-175 do keep a deactivated source resolving on a resume, so nothing is dropped there.

74 checks pass, 0 fail, 0 unresolved threads. No PHP is touched, so no CHANGELOG entry and no behaviour moves.

Merging on that basis.

@CybotTM

CybotTM commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

Self-review: 367ed08

The review this pull request demands is unsatisfiable (Copilot quota wall or repeated bot failures on this head). Per the documented fallback, the diff on this head was reviewed by the PR author; this comment is the on-the-record attestation the merge gate reads back. It stops matching on the next push.

@CybotTM
CybotTM added this pull request to the merge queue Aug 19, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 19, 2026
@CybotTM
CybotTM added this pull request to the merge queue Aug 19, 2026
Merged via the queue into main with commit c669e6d Aug 19, 2026
92 checks passed
@CybotTM
CybotTM deleted the docs/adr-dropped-source branch August 19, 2026 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants