Skip to content

test(daemon): retain sanitized evidence on idle cleanup failure - #4090

Merged
thymikee merged 1 commit into
mainfrom
t3/explain-idle-registration-failures
Oct 30, 2026
Merged

thymikee merged 1 commit into
mainfrom
t3/explain-idle-registration-failures

Conversation

@thymikee

@thymikee thymikee commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

Summary

Preserve evidence when the daemon idle-reap test fails before its temporary state is removed. Append the existing identity reader's PID/start time, info/lock presence and selected removal/finish phase/reason/code to the original assertion error. Tokens, raw registration/log data and error messages are excluded. Unreadable diagnostics leave the original failure intact.

One test file changed; no new tests, runtime/API changes, budget changes or ownership-fence changes. Refs #4086; part of #4045. This adds observability, not a fix or reproduction claim for the intermittent failures.

Validation

Head: eae13bce45b94dabce7cc2fc200ca8514df9792a.

  • Frozen install/build, full format and all seven focused entrypoint tests passed.
  • Disposable fault proof: baseline/restored idle case each passed; forced unproven identity and thrown finish each failed the original assertion with the expected evidence. Token/private-note/error-message sentinels stayed absent. Identical command/default workers; checkout removed and source state preserved.
  • Independent source review: no findings, no tests run by reviewer.
  • Full exact-head validation passed tooling, Fallow and unit: 1,849 files / 16,447 passed / one skipped (run-804amS, exactCommit: true). The two original full-suite failures remain unexplained; base unit passed and the phase probe passed eight cases. SDK registration API work is deferred separately until after fix(runtime): preserve platform owner shutdown failures #4063.

View guided diff Turn on auto-fix

@vercel

vercel Bot commented Oct 29, 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 29, 2026 7:12pm UTC

Request Review

@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 -2 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 34.4 ms 35.5 ms +1.2 ms
CLI --help 69.8 ms 68.5 ms -1.4 ms

@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.

No issues found across 1 file

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

@thymikee

Copy link
Copy Markdown
Member Author

The code looks good at eae13bc. The change is test-only, so it touches no runtime route, and all 18 checks pass. I found no conflicts. I read the source and relied on green CI, so I did not run the tests or your fault proof, and the success path was not exercised because the catch only fires on failure. Not blocking: the 30-line catch block in src/daemon-entrypoint.test.ts could move into a small local helper, since it hand-parses the daemon log and duplicates phase names from daemon-startup-diagnostics.ts, and the appended reason and code are not checked against a whitelist (they are typed today); take or leave both. #4086 can stay open until a failing trace is captured, since this PR adds observability only.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 29, 2026
@thymikee
thymikee merged commit 2e6e28c into main Oct 30, 2026
18 checks passed
@thymikee
thymikee deleted the t3/explain-idle-registration-failures branch October 30, 2026 06:55
@thymikee

Copy link
Copy Markdown
Member Author

Addressing the optional diagnostic review in follow-up #4094 at fb0dc518e3927a907492b119bd2a4dd2f117f147:

  • Accepted value filtering. The parsed JSON is not runtime-validated, and AppErrorCode deliberately admits arbitrary strings, so its static type does not establish safe diagnostic contents. The follow-up emits only the five current declined-registration reasons and codes from the existing known-code vocabulary. Unexpected strings/objects are omitted; the selected phase remains visible.
  • Kept the catch inline. It has one caller, and extracting it would retain the same parsing and phase selection while introducing a helper used only once. It makes no new production export or diagnostic-state owner.
  • Seven entrypoint tests pass. An isolated forced failure retained known values and excluded unexpected string/object sentinels; reverting the filters exposed the three sentinels, and restoration passed. Independent read-only review found no findings. Exact affected validation is waiting for a live shared-lock owner; test(daemon): bound values in idle cleanup evidence #4094 is draft until that result is recorded.

#4086 stays open for an actual intermittent-failure trace. This follow-up makes no diagnosis, SDK API change or budget adjustment.

@thymikee

Copy link
Copy Markdown
Member Author

Follow-up #4094 is now ready for review: all five exact affected checks passed at fb0dc518e3 (run-JnVNMA, exact commit, checkout unchanged), including seven related tests. The previously reported lock wait is complete. This replaces only the pending validation status for the bounded diagnostic-filter follow-up; it makes no claim about #4086's original failure cause.

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