feat(agent): say which forced source a queued run did not get - #842
Conversation
ADR-175 settled that a source switched off between enqueue and start does not enter the run. Nothing said so. The run proceeded without it and the only way to notice was to compare the queued request against the transcript. The dequeue now reports what it asked for and did not get, and why: deactivated for a record that is switched off, gone for one that no longer resolves. Telling those apart is the point — one is reversible, the other is not — and it needs the SECOND lookup: a uid the enabled-only lookup skipped but the existence lookup finds is switched off. Without both, the two reasons are one. It travels on RunAugmentation, is written through RunTrace as its own step kind before assembly, and reaches the run view through RunStep's payload and the timeline's allow-list. Recorded on the resolve side rather than inferred from the assembled messages, because a source that never arrived leaves no mark in them — that absence is what an operator cannot see today. Flattened to "kind#uid reason" strings, not a count: "2 dropped" would flatten exactly the distinction the record exists to make. A run that dropped nothing records no step, so the step's presence is the signal. The resume path is untouched: ADR-166 and ADR-175 keep a deactivated source resolving there, so nothing is dropped and a step would imply otherwise. An existing test caught a real defect in the first version. ToolLoopServiceAssemblyOrderTest pins that the enabled-only lookup runs once per dequeue; my detection called it a second time. The records that did arrive are now passed in rather than re-queried — one extra query per kind instead of two. Two controls, each observed: resolving both sides with the enabled-only lookup makes the two reasons indistinguishable and fails two tests; removing the empty-list guard makes a clean run record a step and fails the test that forbids it. ADR-179 claimed RunAugmentation was @internal. It is on the frozen surface, and that sentence is corrected in the record rather than left for the next reader. The growth is additive: a parameter with a default plus one method, announced under ### Added. ADR-176 said five builtins write through the DataHandler. ADR-180 added a sixth within a day, so the number is replaced by the command that answers it — the trait every writer uses, not a search for the class name, which also matches the two traits and overcounts by two. Closes #809 Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
github-actions
Bot
added
documentation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #842 +/- ##
============================================
- Coverage 86.74% 86.64% -0.10%
- Complexity 9865 9896 +31
============================================
Files 573 574 +1
Lines 31920 32037 +117
============================================
+ Hits 27689 27760 +71
- Misses 4231 4277 +46
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Review record — no Copilot review exists (monthly, account-wide quota), so the green gate is not a read of the change. An existing test found a real defect in my first version. Both new assertions were seen to fail, not just seen to pass: resolving both sides with the enabled-only lookup makes the two reasons indistinguishable and fails two tests; removing the empty-list guard makes a clean run record a step and fails the test that forbids it. Two of the seven detection tests are the ones I would not have written from the ADR alone. A Two corrections to records, both mine, both found by opening the file rather than trusting the sentence. ADR-179 claimed Deliberately not flattened to a count. Gates: Assisted by claude-code:claude-opus-5 — Session |
|
Self-review: 137c496 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. |
Closes #809, implementing ADR-179 (#828).
The problem: switch a snippet or skill off between enqueue and start and the queued run loses it — that is ADR-175's rule and it stays. Nothing said so. The run did something other than what was asked for, and the only way to notice was to diff the queued request against the transcript.
What it does
The dequeue reports what it asked for and did not get, and why:
deactivatedfor a record that is switched off,gonefor one that no longer resolves.Telling those apart is the whole decision — one is reversible, the other is not — and it needs the second lookup: a uid the enabled-only lookup skipped but the existence lookup finds is switched off. Without both, the two reasons collapse into one and the reader is told "dropped" without being told what to do about it.
It travels on
RunAugmentation, is written throughRunTraceas its own step kind before assembly, and reaches the run view throughRunStep's payload and the timeline's allow-list.Recorded on the resolve side, not inferred from the assembled messages — a source that never arrived leaves no mark in them, and that absence is precisely what an operator cannot see today.
Flattened to
kind#uid reason, not counted.snippet#41 deactivatedsays what to do;2 droppedflattens the distinction the record exists to make.A run that dropped nothing records no step, so the step's presence is the signal — an empty step on every run is noise, and then nobody looks. The resume path is untouched: ADR-166 and ADR-175 keep a deactivated source resolving there, so nothing is dropped and a step would imply otherwise.
An existing test caught a real defect
ToolLoopServiceAssemblyOrderTestpins that the enabled-only lookup runs once per dequeue. My first version called it a second time — two identical queries on every dequeue. The records that did arrive are now passed in rather than re-queried: one extra query per kind instead of two. That assertion existed for a different reason and caught this for free.Controls, each observed
11 new tests: 4 on the step, 7 on the detection against real rows — including that a
hiddenbut enabled source is not reported (the repositories ignore enable fields, sohiddenis FormEngine visibility; reporting it would send an operator to fix something that is not broken), and that a deleted record readsgonerather thandeactivated.Two corrections to records, both mine
ADR-179 claimed
RunAugmentationis@internal. It is on the frozen surface —api-surface.txtlists it with its full constructor. Corrected in the record rather than left for the next reader. The growth is additive (a parameter with a default plus one method) and announced under### Added.ADR-176 said "five builtins write through the DataHandler". ADR-180 added a sixth within a day of that record being written. The number is replaced by the command that answers it —
grep -l 'use WritesThroughDataHandlerTrait' …, the trait every writer uses, rather than a search for the class name, which also matches the two traits themselves and overcounts by two. I wrote that wrong anchor first and checked it before trusting it.Gates:
phpstanlevel 10 clean,unit7267 pass,fuzzypass,functional -d sqliteon the touched classes pass,cglandrector -nstable at PHP 8.2.Closes #809