Skip to content

fix(test): unstick Smithery probe lifecycle and pin DAEMON_GENERATION 54 - #3972

Merged
Yeachan-Heo merged 1 commit into
devfrom
fix/dev-ci-generation-pin-and-smithery-hang
Aug 7, 2026
Merged

fix(test): unstick Smithery probe lifecycle and pin DAEMON_GENERATION 54#3972
Yeachan-Heo merged 1 commit into
devfrom
fix/dev-ci-generation-pin-and-smithery-hang

Conversation

@Yeachan-Heo

Copy link
Copy Markdown
Owner

Blocker attribution (exact dev f1eb7b594 / run 31133356543)

Two coupled test-harness / pin failures after #3965; neither is product behavior for #3908/#3855.

(1) Stale generation pin — deterministic

File packages/coding-agent/test/notifications-topic-registry.test.ts
Case publishes generation 53 at serving epoch 5
Expected DAEMON_GENERATION === 53
Actual 54 (shipped by #3965 owner postmortem)

Cause: pin not updated when DAEMON_GENERATION bumped 53→54.
Sibling: notifications-telegram-daemon.test.ts already pinned 54.

(2) Smithery inherited-config hang — child-process lifecycle

File packages/coding-agent/test/smithery-env-trust.test.ts
Case still honors inherited Smithery configuration
Timing exactly 60001.59ms (outer it budget)
Neighbors previous 3 cases pass ~365–380ms; next case pass ~369ms

Causality chain (evidence-backed):

  1. Prior repairs (fix(test): set 30s timeout on smithery-env-trust child-process tests #3948 30s → fix(test): raise smithery-env-trust child-process timeout to 60s #3967 60s → fix(test): isolate smithery-env-trust first-spawn cold-start under CI contention #3970 beforeAll warmup + 45s kill) addressed cold-start and contention budgets.
  2. Run 31133356543 shows warmup worked for cases 1–3 (~370ms).
  3. Case 4 still hit outer 60s, not a ~45s "probe timed out" error → the 45s kill() timer fired (or would have) but Promise.all([stdout.text(), stderr.text(), exited]) never settled.
  4. That is a lifecycle hang: kill without abandoning hung pipe reads leaves the test async function blocked until the it timeout.

Not: assertion wrong, provider behavior, or need for a third blind timeout bump.

Repair (test harness only)

Generation pin

  • Rename/pin publishes generation 54 at serving epoch 5toBe(54).

Smithery probe harness

Change Why
Minimal child env (PATH/HOME/GJC_CODING_AGENT_DIR/TMPDIR/LANG + overrides) Avoid multi-KB CI process.env copy correlated with stalled child startup
stdin: "ignore" No accidental stdin wait
Per-attempt 15s deadline via settled Promise race + SIGKILL Fail the attempt even if pipe reads never complete after kill
Up to 2 timeout-only retries Recover from transient contention without changing assertions
Keep beforeAll warmup + 60s outer it Enough for 3×15s attempts; not a higher single-shot timeout

Assertions and Smithery product code unchanged.

Verification (local)

Check Result
publishes generation 54… pass
smithery 3× sequential 5 pass / ~3.2s each
smithery 6× parallel contention 6× 5 pass / ~2.8s

Non-goals

Dev CI run 31133356543 at f1eb7b5 failed two coupled contracts:

1) notifications-topic-registry.test.ts still pinned DAEMON_GENERATION to 53
   after #3965 shipped generation 54 — pure stale assertion.

2) Smithery "still honors inherited Smithery configuration" hit exactly
   60001.59ms. Prior harness set a 45s kill timer but still awaited
   Promise.all(stdout, stderr, exited). When kill did not complete the
   pipe reads, the outer it(..., 60_000) failed while sibling cases before
   and after completed in ~370ms. This is a child-process lifecycle hang,
   not an assertion bug and not fixed by another blind timeout bump.

Repair:
- Pin topic-registry generation contract to 54 / serving epoch 5.
- Probe harness: minimal child env (no full process.env copy), stdin
  ignore, per-attempt 15s deadline via settled Promise race + SIGKILL,
  up to 2 timeout-only retries. Assertions unchanged.

Lore-id: d4f2a801
Confidence: high
Scope-risk: narrow
Reversibility: trivial
Tested: topic-registry generation pin; smithery 3× sequential + 6× parallel
  contention (all 5 pass / ~3s)
Not-tested: full 8-shard Dev CI load shape
@Yeachan-Heo
Yeachan-Heo force-pushed the fix/dev-ci-generation-pin-and-smithery-hang branch from 05e3831 to 3946f88 Compare August 7, 2026 00:35

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3946f8883b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +130 to +131
settle(() => {
resolve(JSON.parse(stdout.trim()) as Resolved);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Parse probe output before marking the attempt settled

If the probe exits successfully but emits empty, truncated, or otherwise malformed stdout, settle sets settled = true before JSON.parse runs. The parse error reaches the surrounding catch, but its rejection is ignored because the attempt is already marked settled; the deadline callback is ignored for the same reason, leaving promise pending until the outer 60-second test timeout. Parse first or allow a callback failure to reject the attempt.

AGENTS.md reference: AGENTS.md:L146-L150

Useful? React with 👍 / 👎.

@Yeachan-Heo
Yeachan-Heo merged commit c5b8797 into dev Aug 7, 2026
20 checks passed

@Yeachan-Heo Yeachan-Heo left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Red-team review — MERGE_READY / CLEAR

Exact head: 3946f8883b6ce2ca0185aada91d6ac726c1a0a12
Base: dev (contains 627e87d1a; exact-head ancestry OK)

Scope (test harness only)

File Change
notifications-topic-registry.test.ts Pin DAEMON_GENERATION 53→54
smithery-env-trust.test.ts Probe lifecycle isolation (minimal env, race+SIGKILL, retries)
CHANGELOG.md Unreleased notes

No product / #3908 / #3855 implementation changes.

Causality (run 31133356543 @ f1eb7b5)

  1. Generation pin: deterministic stale assert after #3965.
  2. Smithery: case 4 hit 60001.59ms while neighbors ~370ms; prior 45s kill left Promise.all(stdout/stderr/exited) hung → outer it timeout. Lifecycle fix, not timeout inflation.

CI

  • Dev CI run 31135186765: success
  • Check-runs: 14 success / 6 skipped / 0 failure / 0 pending

Verdict

MERGE_READY / CLEAR.

— census CI/test-harness repair chain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant