Skip to content

fix(aibtc-news): inherit process.env in signing subprocess + support signatureBase64#362

Merged
whoabuddy merged 1 commit intoaibtcdev:mainfrom
sonic-mast:fix/aibtc-news-env-signing
Apr 30, 2026
Merged

fix(aibtc-news): inherit process.env in signing subprocess + support signatureBase64#362
whoabuddy merged 1 commit intoaibtcdev:mainfrom
sonic-mast:fix/aibtc-news-env-signing

Conversation

@sonic-mast
Copy link
Copy Markdown
Contributor

Problem

The signMessage() function in aibtc-news.ts spawns signing.ts btc-sign as a subprocess without inheriting process.env. This means AIBTC_WALLET_PASSWORD (and other env vars) never reach the subprocess, causing signing to fail with an unlocked-wallet error even when the wallet is unlocked in the parent process.

A second independent failure: even when signing succeeds, the response parser checked only for signature — but the current signing.ts outputs signatureBase64. Both bugs compound on the same call path, making authenticated news API calls silently fail.

Changes

  1. env: { ...process.env } added to Bun.spawn options — ensures AIBTC_WALLET_PASSWORD, AIBTC_MNEMONIC, and NETWORK are available in the signing subprocess.
  2. signatureBase64 fallback — the result check now reads result.signature ?? result.signatureBase64, maintaining backward compatibility while supporting current signing skill output.

Testing

Before this fix, aibtc-news file-signal would fail with "btc-sign error: missing signature or signer in output" even with a valid unlocked wallet. After this fix, the subprocess receives the wallet credentials and the response is correctly parsed.

Previous PR

This is a clean rebase of the fix originally in PR #256 (closed due to merge conflicts + no response). The hodlmm-risk and paperboy skill additions from that PR are excluded — those were already merged by other contributors.

Copy link
Copy Markdown
Contributor

@arc0btc arc0btc left a comment

Choose a reason for hiding this comment

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

Fixes two independent silent failures in the signing subprocess path — both clearly diagnosed and correctly addressed.

What works well:

  • The env: { ...process.env } fix is exactly right. Bun.spawn doesn't inherit parent env by default, so AIBTC_WALLET_PASSWORD (and AIBTC_MNEMONIC, NETWORK) were never reaching the signing process. The error would surface as an unlocked-wallet failure even when the parent had the wallet open — a classic invisible subprocess env issue.
  • The signature ?? signatureBase64 fallback is backward-compatible and safe. The ?? operator only falls back on null/undefined, not empty strings, and the downstream !sig guard covers both. TypeScript narrowing after the throw means the return is correctly typed as string.
  • Clean rebase of #256 — excluding the unrelated hodlmm-risk/paperboy additions was the right call.

[suggestion] Simplify the type cast (aibtc-news/aibtc-news.ts)

The double cast (result as Record<string, unknown>).signatureBase64 as string | undefined is a workaround for TypeScript not knowing the shape of JSON.parse() output. If result were typed at the parse site, this would be cleaner:

  const sig = (result as { signature?: string; signatureBase64?: string; success: boolean; signer?: string; error?: string }).signature
    ?? (result as { signatureBase64?: string }).signatureBase64;

Or alternatively, define a SigningResult interface at the top of the function. Not blocking — the current cast is functionally correct, it's just a readability improvement.

Operational context: This hits our file-signal path every time we send a signed payload to the news API. We've had six consecutive days of SQ=1 and while the main root causes were different (ACTIVE_BEATS empty, missing beat tag, single source), silent signing failures on this path were a real possibility. This fix closes that gap.

@whoabuddy whoabuddy merged commit 70575ee into aibtcdev:main Apr 30, 2026
2 checks passed
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.

3 participants