[fix] run-path correctness: Execute output-limit (P1-1) + per-run result reset (P1-2)#25
Merged
Merged
Conversation
…ult reset (P1-2) Independent review (2026-06-20) found two run-path invariants that the RunnerConfig and reuse paths violated: - P1-1: RunnerConfig.Execute() returned the machine result directly, bypassing applyOutputLimit - so SetMaxOutputEntries was silently ignored for the builder/thin-shell path (the path starcli is likely to use). It now returns b.applyOutputLimit(out, err), matching Run/RunFile/RunTimeout. - P1-2: the output() result slot was cleared only at AddResultBuiltin time, making it once-per-Box-lifetime instead of once-per-run as documented; a second Run on the same Box failed with "result already set once". A new markRun() helper (now the single place every run path bumps the counter) resets the slot at each run start. A second output() within one run is still an error (TestResultOnce). Tests (test-first, both reproduced before the fix): - TestRunnerConfig_OutputLimit: Execute() honors the output-entry cap. - TestResultResetPerRun: Box runs repeatedly; GetResult reflects each run; a no-output run reports unset even after a prior set.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 9 |
| Duplication | 0 |
🟢 Coverage 100.00% diff coverage · 0.00% coverage variation
Metric Results Coverage variation ✅ 0.00% coverage variation (-1.00%) Diff coverage ✅ 100.00% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (3098df0) 1155 1146 99.22% Head commit (e7d6a5a) 1154 (-1) 1145 (-1) 99.22% (0.00%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#25) 14 14 100.00% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #25 +/- ##
==========================================
- Coverage 98.95% 98.95% -0.01%
==========================================
Files 12 12
Lines 864 862 -2
==========================================
- Hits 855 853 -2
Misses 5 5
Partials 4 4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release blockers P1-1 + P1-2 (independent review 2026-06-20)
Two run-path invariants violated on the builder / reuse paths. Both fixed test-first (each repro confirmed failing before the fix).
P1-1 —
RunnerConfig.Execute()bypassed the output limitExecute()returned the machine result directly, skippingapplyOutputLimit, soSetMaxOutputEntrieswas silently ignored for the builder/thin-shell path (the one starcli is likely to use).Fix:
Execute()now returnsb.applyOutputLimit(out, err), matchingRun/RunFile/RunTimeout/RunInspect*.P1-2 —
output()slot was once-per-Box, not once-per-runThe result slot was cleared only at
AddResultBuiltintime, so a secondRunon the same Box failed withresult already set once— contradicting the documented "second call within a run is an error."Fix: a new
markRun()helper — now the single place every run path bumps the run counter — resets the slot at each run start. A secondoutput()within one run is still an error (TestResultOncestays green).Tests
TestRunnerConfig_OutputLimit—Execute()honors the output-entry cap (OutputLimitExceededError).TestResultResetPerRun— a reused Box runs repeatedly;GetResultreflects each run; a no-output run reports unset even after a prior set.markRun/applyOutputLimit100%; overall 99.3%.gofmt/vetclean,-race -count=2and Dockergolang:1.19floor green.