Skip to content

test(daemon): bound values in idle cleanup evidence - #4094

Merged
thymikee merged 1 commit into
mainfrom
t3/bound-idle-diagnostic-values
Oct 30, 2026
Merged

thymikee merged 1 commit into
mainfrom
t3/bound-idle-diagnostic-values

Conversation

@thymikee

@thymikee thymikee commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Summary

Bound the values appended to an idle-reap assertion failure. Parsed log fields are untrusted: registration reasons now match the five current declined states, and codes match the existing KNOWN_APP_ERROR_CODES vocabulary. Unknown strings and objects are omitted while the selected phase remains visible.

One test file changed. Original assertions, runtime behavior, ownership fences and budgets are unchanged. Follow-up to the optional #4090 review; refs #4086 and #4045. The single-use catch stays local.

Validation

Head: fb0dc518e3927a907492b119bd2a4dd2f117f147; base: 3c6673a2b960a6607bac1e080f635a133b86c3d3.

  • Frozen install/build, full format and all seven entrypoint tests passed.
  • Disposable forced-failure proof retained known reason/code and omitted unexpected strings/objects. Removing the filter exposed three sentinel values; restoring the checkout passed the idle test. Same command/default workers; disposable removed and reviewed source preserved.
  • Independent source review found no findings; reviewer ran no checks.
  • Exact affected validation passed all five selected checks (format, lint, typecheck, Fallow and related tests; seven passed). Receipt run-JnVNMA has exactCommit: true and checkoutChanged: false. CI coverage/integration remain obligations. This observability follow-up does not diagnose the original intermittent failures.

@vercel

vercel Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
agent-device-website Ready Ready Preview Oct 30, 2026 9:25am UTC

Request Review

@thymikee

Copy link
Copy Markdown
Member Author

Reproducible fault-proof scope at fb0dc518e3927a907492b119bd2a4dd2f117f147:

Command for all four runs, default workers:

pnpm exec vitest run --project unit-core src/daemon-entrypoint.test.ts -t 'daemon entrypoint reaps itself when idle'

A detached disposable checkout used its own frozen install/build. Baseline: one passed, six filtered. For the failure controls only, append synthetic NDJSON before the existing assertions and make the existing exit assertion expect -1. Supply selected phases with known unproven/COMMAND_FAILED, unexpected string reason/code, and object-valued reason/code; also supply an unrelated phase. No control is committed.

  • With the filter: one deliberately failed, six filtered. The original assertion still failed; appended evidence contained the selected known values, omitted all three unexpected-value sentinels and excluded the unrelated phase.
  • Revert just the filtering expressions to the previous direct data projection: the same assertion failed, and all three unexpected-value sentinels appeared. This control demonstrates the old diagnostic leak; it is not a new production failure.
  • Restore all experiment edits: one passed, six filtered. The complete seven-test suite separately passed at the reviewed head.

The disposable checkout was removed, and the implementation SHA and clean status were unchanged. Independent review inspected the diff and actual diagnostic/type producers, found no findings, and ran no checks. Exact affected result will be recorded separately; no full-unit or native pass is inferred from this bounded proof.

@github-actions

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.61 MB 5.61 MB 0 B
Package (unpacked) 5.61 MB 5.61 MB 0 B
Package (download) 1.71 MB 1.71 MB +6 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 45.6 ms 41.7 ms -3.8 ms
CLI --help 86.3 ms 86.8 ms +0.5 ms

@thymikee
thymikee marked this pull request as ready for review October 30, 2026 09:29
@thymikee

Copy link
Copy Markdown
Member Author

Exact affected validation passed at fb0dc518e3927a907492b119bd2a4dd2f117f147 against base 3c6673a2b960a6607bac1e080f635a133b86c3d3:

pnpm check:affected --run --base 3c6673a2b960a6607bac1e080f635a133b86c3d3 --head fb0dc518e3927a907492b119bd2a4dd2f117f147

All five selected checks passed: format, lint, typecheck, Fallow and related tests (seven passed). Receipt run-JnVNMA: exactCommit: true, checkoutChanged: false. The existing live lock owners finished normally; no lock was removed. The PR is ready for review. This narrowed validation is not a full-unit, CI coverage or live-device claim.

@cubic-dev-ai cubic-dev-ai 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.

1 issue found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/daemon-entrypoint.test.ts">

<violation number="1" location="src/daemon-entrypoint.test.ts:323">
P3: The declined-reason list is re-typed here as literals, separate from the `removal.state` union in daemon-contracts. If a declined state is added or renamed, this filter silently drops it from the evidence instead of failing. Export a runtime const for the declined states from the owning module and derive this list from it.</violation>
</file>

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

.map(({ phase, data }) => ({ phase, reason: data?.reason, code: data?.error?.code }));
.map(({ phase, data }) => ({
phase,
reason: ['retained', 'replaced', 'unproven', 'unreadable', 'ownerless'].find(

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.

P3: The declined-reason list is re-typed here as literals, separate from the removal.state union in daemon-contracts. If a declined state is added or renamed, this filter silently drops it from the evidence instead of failing. Export a runtime const for the declined states from the owning module and derive this list from it.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/daemon-entrypoint.test.ts, line 323:

<comment>The declined-reason list is re-typed here as literals, separate from the `removal.state` union in daemon-contracts. If a declined state is added or renamed, this filter silently drops it from the evidence instead of failing. Export a runtime const for the declined states from the owning module and derive this list from it.</comment>

<file context>
@@ -309,15 +310,21 @@ test('daemon entrypoint reaps itself when idle: exits cleanly, releases its regi
-          .map(({ phase, data }) => ({ phase, reason: data?.reason, code: data?.error?.code }));
+          .map(({ phase, data }) => ({
+            phase,
+            reason: ['retained', 'replaced', 'unproven', 'unreadable', 'ownerless'].find(
+              (reason) => reason === data?.reason,
+            ),
</file context>

@thymikee

Copy link
Copy Markdown
Member Author

@apex-sentinel-staging review

@apex-sentinel-staging

Copy link
Copy Markdown

Sentinel reviews this repository only for the account that connected the installation, and for logins in user_settings.reviewers on the default branch.

@thymikee

Copy link
Copy Markdown
Member Author

The code looks good at fb0dc51. The checks are green (21 checks, none failing) and there are no conflicts.

Not blocking, and you can take or leave it: the five declined states in src/daemon-entrypoint.test.ts:322 are retyped as literals, so a new or renamed state in RegisteredDaemonOwnership would silently drop that reason instead of failing the test. The owner is only a type union, so fixing this needs a runtime const exported from daemon-registration.ts that the union derives from. A one-line comment naming the owner would also do.

The open P3 thread on the retyped states still applies: #4094 (comment). I did not run the test or mutate the vocabulary locally.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 30, 2026
@thymikee
thymikee merged commit 14f4288 into main Oct 30, 2026
21 checks passed
@thymikee
thymikee deleted the t3/bound-idle-diagnostic-values branch October 30, 2026 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant