Repository navigation
fix(logs): name Alloy read positions in logs-reset; pin the exit drain bound and Loki hint wiring - #478
Merged
Conversation
…n bound and Loki hint wiring - Pin NATIVE_EXIT_DRAIN_BOUND_MS at its documented 5 s floor and above the late-writer delay, and assert the held-descendant cases (runtime client and foreground graph owner) keep reading for the whole production bound, so a smaller constant or default no longer passes. - Add late-closed-code cases for the one-off and recovery (selection and cleanup) bound drains. - Add CLI cases that carry --pretty, --query and --until into the empty-Loki hint through src/commands/project.ts. - `hack global logs-reset` now says it also deletes the Alloy read positions volume in its prompt and summary; regenerate docs/reference/cli.md. Refs HACK-1425 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
Refs HACK-1425. Follow-ups from the verifier reviews of #447 (merged as 686b6b8) and #449 (merged as 4c9eb3e). None of them blocked those PRs.
Root cause
NATIVE_EXIT_DRAIN_BOUND_MSitself, or injected a wide value through theexitDrainBoundMstest seam. The held-descendant cases only checked an upper bound (settled - exited < bound + margin). So setting the constant to 250 (or changing the?? NATIVE_EXIT_DRAIN_BOUND_MSdefault to?? 250) passed the whole suite.src/commands/project.tssurvived the tests.tests/logs-loki-empty-fallback-cli.test.tsnever passed--pretty,--queryor--untilthrough the CLI. Replacingpretty: args.options.pretty === true, or the selectors'query: opts.queryOptoruntil: opts.until, with a constant did not fail any test.hack global logs-resettext was out of date. The command runsdown -v, which (since fix(logs): persist Alloy positions, release SIGINT handlers and carry every selector in Loki hints #449) also deletes thealloy-datavolume that holds Alloy's read positions. The confirm prompt and the CLI summary named only Loki and Grafana.Fix
tests/native-runtime-client.test.tsthe production exit drain bound keeps its documented floor and outlasts a late writer. It checks thatNATIVE_EXIT_DRAIN_BOUND_MS >= 5000(the documented floor, now written in the constant's doc comment) and that the bound is larger than the late-writer delay plus a clock tolerance.exitcases (read, selection, cleanup) do not pass the seam. They now also checksettled - exited >= NATIVE_EXIT_DRAIN_BOUND_MS - 50 ms, which proves the production default is the bound in effect.LATE_WRITER_SELECTIONSgainsone-off,selectionandcleanup.tests/native-project-process.test.ts: the same lower bound on the foreground graph owner's two held-descendant cases. This covers that file's own?? NATIVE_EXIT_DRAIN_BOUND_MSdefault.tests/logs-loki-empty-fallback-cli.test.tsgets two CLI cases. Both use the stubdocker, the fake Loki, and a private HOME viaprivateCliEnv:--loki --no-follow --pretty migrate: the hint repeats--pretty.--no-follow --query '<custom LogQL>' --until 1m: Loki receives the custom query exactly as given, and the hint lists--query '…'and--until 1mas selectors that--composecannot apply.src/commands/global.ts: the summary is now "Wipe Loki, Grafana and Alloy volumes (fresh logs, dashboards and Alloy read positions)". The prompt now says it deletes "ALL Loki logs, Grafana state and Alloy read positions".docs/reference/cli.mdwas regenerated withbun run docs:cli-reference.docs/architecture.mdalready described the volume removal, so it is unchanged.There is no production behavior change apart from the logs-reset text and a doc comment.
Evidence
Mutation runs (each mutation applied on its own and restored from a backup afterward):
NATIVE_EXIT_DRAIN_BOUND_MS = 250selected ?? 250exitcasesopts.exitDrainBoundMs ?? 250readyandearly failureheld-descendant casesone-offonlyauthored one-off keeps a closed code written after the reported exitrecovery-selectiononlyauthored selection keeps a closed code …recovery-cleanuponlyauthored cleanup keeps a closed code …pretty: args.options.pretty === true→falseexplicit --loki snapshot hint repeats --prettyquery: opts.queryOpt→undefinedexplicit --query snapshot hint names --query and --untiluntil: opts.until→undefinedexplicit --query snapshot hint names --query and --untilLocal gates at this head (Bun 1.4.2):
bun run checkpassed, includingprivacy:check.bun run --cwd packages/cli typecheckand rootbun run typecheckreported 0 errors.origin/next..HEADpatch found no leaks.native-runtime-client,native-project-process,logs-loki-empty-fallback-cli,global-commandandprerequisites-matrixgave 74 pass, 0 fail.The full-suite evidence is hosted CI's sharded
testjob at this PR head.Gaps
confirmSafedeclines in non-interactive runs (initialValue: false), so a CLI test cannot see the prompt without a TTY harness. This matches the coverage before this PR.>=), not an exact value. Raising the bound still passes, which is intended.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.