ENGN-8456: Identity & Wallet Foundation (builder-kit) — workerctl link + managed-custody worker lifecycle - #36
Conversation
Adds wallet_link.py: a gh-auth-login-style device flow that signs an ADR-036 challenge with the on-disk worker key (cosmpy), opens the browser for the logged-in Forge user to approve, and polls to completion. The mnemonic never leaves the dev kit and it works headless. Exposed as `workerctl link`.
Lets a worker run with Privy-managed custody (auto-provision a wallet bound to
its topic) and migrates worker_runtime to the branch SDK's worker API:
- worker_runtime.py: add --custody {local,managed}; managed builds
AlloraWalletConfig.from_env() (FORGE_API_KEY → provision per-topic, no local
key). Migrate to AlloraWorker.inferer(...) and adapt the run fn to the
RunContext-based signature (raw_fn(ctx.nonce)).
- pyproject.toml: LOCAL-DEV [tool.uv.sources] pin of allora-sdk to the local
branch worktree (carries the unreleased auto-provision changes). Remove and
bump the version once the SDK release ships.
WorkerManager can now run Privy-managed workers end-to-end, not just local self-custody: - deploy_worker(custody="managed") provisions (idempotent get-or-create) a managed wallet bound to the topic via the Forge backend and registers the worker against the backend-assigned address — no local mnemonic/key file. One wallet per topic; a re-deploy refreshes the artifact on the same wallet. - start_worker launches managed workers with --custody managed and injects FORGE_API_KEY/FORGE_BACKEND_URL into the subprocess env (no --mnemonic-file). - remove_worker releases the (user, topic) binding via clear-association on decommission (best-effort: local removal always proceeds). - workers table gains custody + signing_wallet_id (lightweight migration); Forge client built lazily from FORGE_* env (injectable for tests). Reuses allora_sdk's ForgeBackendClient (provision_wallet + new clear_association). Tests cover provision/register, idempotent redeploy, managed command/env construction, clear-on-remove (incl. failure tolerance), local-not-cleared, and the missing-credentials error.
The poll loop aborted the entire device flow on any single transient HTTP/network failure (502/503/429 from the server rate limiter, DNS blip) because _post_json raises SystemExit. Catch SystemExit around the poll call and keep polling until the wall-clock deadline; only terminal statuses (denied/expired) fail hard. Matches RFC 8628 §3.5 and gh/gcloud. Refs: synth-001
A hostile/buggy server could send interval=86400 (stalls past the timeout), 0/negative (spins the rate limiter), or a non-int (raised an uncaught ValueError). Tolerate non-int values and clamp to [1, 60]s. Refs: synth-002
…t loop Managed custody calls AlloraWalletConfig.from_env(), which performs a blocking wallet-info HTTPS fetch. Doing it inside the async _run() stalled the event loop at startup (and delayed cancellation if the backend hung). Move resolution into a sync _resolve_wallet_cfg() helper invoked from main() before asyncio.run(), and pass the pre-resolved wallet_cfg into _run(). Addresses synth-003.
Square-bracket reads of device_code/user_code/verification_uri_complete raised a raw KeyError traceback on a version-skewed or malformed server response. Validate the required fields with .get() and print a clean stderr error returning 1, matching the rest of the module. Also skip malformed entries in the challenges array instead of KeyError-ing. Refs: synth-003
…ssion drops run_fn unconditionally called raw_fn(ctx.nonce). An artifact written to the newer RunContext contract (raw_fn(ctx)) would get an int, raise AttributeError inside Inferer.submit, and be silently caught — dropping every submission with no alarm. Resolve the artifact's call shape once at load via inspect.signature (defaulting to the legacy nonce form for back-compat) and document the supported artifact contract in _run's docstring. Addresses synth-005.
Without scheme validation, --forge-url http://... sent the device_code bearer credential and ADR-036 signatures over plaintext, allowing an on-path attacker to hijack the approval. Reject non-https URLs unless the host is localhost/127.0.0.1 or the new --insecure flag is set. Flag added to both wallet_link.main() and the workerctl link subparser. Refs: synth-004
The inferer callback closure had no parameter or return annotation. Import RunContext under a TYPE_CHECKING guard and annotate run_fn(ctx: RunContext) -> float so pyright catches call-site type errors (e.g. a future ctx.nonce rename). Addresses synth-009.
verification_uri_complete was opened/printed verbatim, letting a compromised server phish via a different origin or hand a file://, javascript:, or app-launcher URI to the OS handler. Require the URL's hostname to match the forge URL and the scheme to be https (http only for localhost or when --insecure is set); reject otherwise. Refs: synth-005
The custody routing parameter only ever takes two values. Narrow it from a bare str to Literal["local", "managed"] (the resolution moved into _resolve_wallet_cfg) per the strong-typing mandate, so direct callers and pyright catch invalid values instead of silently falling through. argparse choices still enforce the CLI boundary. Addresses synth-008.
webbrowser.open() returns False (without raising) on headless setups, so the except branch was dead code and the user always saw "Opened your browser" even when nothing opened, hiding the copy-the-URL fallback. Check the return value and only claim success when it is truthy. Refs: synth-006
Passing --custody managed --mnemonic-file silently discarded the key file (managed custody resolves the wallet from FORGE_API_KEY), leaving an operator to believe a local key was in play. Surface the conflict as a parser.error before any network calls. Addresses synth-004.
A missing/unreadable key_file or an address-derivation mismatch raised a raw traceback mid-loop and orphaned the server-side device session. Validate every selected key file exists before /device/start, and wrap the read+sign in try/except (ValueError, OSError) to print a clean stderr error and return 1. Refs: synth-007
Fail fast with a clear usage error when --custody managed is used without FORGE_API_KEY, naming the missing variable instead of surfacing it deep in the SDK. Also align the _resolve_wallet_cfg docstring with the real auto-provision SDK contract: FORGE_API_KEY (no FORGE_SIGNING_WALLET_ID) yields a deferred managed config and takes precedence over PRIVATE_KEY/MNEMONIC, so there is no silent local-key fallback. Hardening for synth-001 (false positive against the wrong SDK branch; see PR reply).
The bare dict[str, dict] return type lost all information on the inner
dict. Introduce a _KeyEntry TypedDict ({alias, key_file}) so downstream
accesses like keys[address]["key_file"] are type-checked.
Refs: synth-008
A managed (Privy) wallet never receives faucet drips, so gasless submission requires a fee granter; without FEE_GRANTER every transaction fails with 'insufficient fees'. Emit a startup warning naming the variable. Note: unlike the reviewer's claim, FORGE_SIGNING_WALLET_ID is NOT required — the auto-provision SDK defers wallet creation when only FORGE_API_KEY is set. Addresses synth-006.
The bare dict payload/return annotations gave callers no type checking. JSON is an external boundary, so dict[str, Any] is the right type. Refs: synth-009
Add a module docstring clarifying that ALLORA_API_KEY (faucet/consumer, via --api-key) and FORGE_API_KEY (managed-custody backend) are different secrets for different backends, and that --custody managed reads FORGE_API_KEY only. Reduces the naming-collision footgun. Addresses synth-012.
The link subparser didn't declare --secrets-path, so the natural `workerctl link --secrets-path foo` was rejected and the flag was absent from `workerctl link --help`. Add it with default=argparse.SUPPRESS so it appears in help and works after the subcommand without clobbering a top-level --secrets-path given before it. Refs: synth-010
discover_keys swallowed OSError and JSONDecodeError silently, so a permissions error or corrupt worker_secrets.json surfaced as the misleading "no worker keys, create one" message. Keep the not-found case silent but print a clear stderr error for the unreadable/corrupt case. Refs: synth-011
…patch Add pytest cases asserting custody='managed' routes to AlloraWalletConfig.from_env() without reading mnemonic_file, custody='local' builds AlloraWalletConfig(mnemonic_file=...) without calling from_env(), the managed+mnemonic-file and managed-without-FORGE_API_KEY CLI guards exit, and _artifact_expects_context distinguishes legacy nonce vs RunContext artifacts. Addresses synth-007.
cosmpy is required by `workerctl link` but was only pinned in environment.yml, so a plain `pip install allora-forge-builder-kit` user could not run the verb. Add a [wallet-link] extra pinned to cosmpy==0.11.1 (matching environment.yml) and point the runtime import hint at it. Kept optional rather than core since the module degrades gracefully and the heavy crypto dep isn't needed by the kit's ML workflows. Refs: synth-012
…erer The raw example still used the removed AlloraWorker(...) direct constructor with a predict_fn(nonce) signature, which would break against the branch SDK this PR targets. Mirror worker_runtime.py: switch to AlloraWorker.inferer(...) and wrap the pickled nonce fn in a RunContext adapter (_run_fn). Addresses synth-010.
user_code and verification_uri_complete were printed verbatim, so a compromised server could embed ANSI/CSI control sequences to clear the screen or spoof prior output. Strip non-printable characters via a _printable() helper before printing. Refs: synth-013
…er env (synth-002, synth-014) Pin the DB-stored signing_wallet_id into the managed subprocess env so the worker signs with the exact provisioned wallet (deterministic) rather than re-deriving one via topic get-or-create. Precheck the Forge api key / backend url / wallet id before spawning so a misconfigured managed worker fails loudly via start_worker/reconcile instead of the subprocess exiting right after the DB row is marked 'running'.
The signer is concatenated into the canonical JSON sign-doc without escaping. For bech32 addresses this matches the Go verifier byte-for-byte (golden test still passes), but a future caller passing a quote, backslash, or control byte would silently produce corrupt/injected JSON. Reject signers that don't match the bech32 grammar [a-z0-9]+. Refs: synth-014
…001, synth-003) Add tests that exercise the actual allora_sdk surface the managed lifecycle depends on so a cross-repo contract break cannot hide behind the _FakeForgeClient stub or an argv-only check: - assert the real ForgeBackendClient exposes provision_wallet and clear_association - assert AlloraWalletConfig.from_env() defers (no crash) with only FORGE_API_KEY set - drive from_env() with the exact env _build_run_command injects and assert it builds a wallet-backed config without raising (HTTP mocked).
Both read paths slurped the entire response body with no limit, letting a hostile/misbehaving server OOM the CLI or flood the terminal via the HTTPError detail. Cap reads at _MAX_RESPONSE_BYTES (512 KiB). Refs: synth-015
_resolve_wallet_cfg's docstring described only the SDK deferred get-or-create mode, but WorkerManager-spawned workers always pin FORGE_SIGNING_WALLET_ID, selecting the SDK's immediate make_remote_wallet fetch instead. Document both entry points so a reader tracing the managed flow isn't misled. Addresses: rest-3489848532 Review-Thread: #36 (comment)
The poll loop computed deadline = time.time() + timeout and guarded on time.time(); a wall-clock backwards step (NTP slew, manual change, DST) would silently extend the session past the server-advertised expires_in and a forward step would prematurely abort an in-progress link. Use time.monotonic(). Addresses: rest-3489848490 Review-Thread: #36 (comment)
Both the plaintext-HTTP refusal and the verification-URL origin pin accepted only localhost / 127.0.0.1, so a local Forge bound to [::1] (common on dual-stack hosts) required --insecure (which disables ALL plaintext checks). Add ::1 via a shared _LOOPBACK_HOSTS constant used at both sites. Addresses: rest-3489848515 Review-Thread: #36 (comment)
discover_keys printed to stderr and returned {} for a corrupt/unreadable
secrets file, so run_link then printed the misleading 'No worker keys found …
create one'. Add a domain-specific SecretsLoadError raised for OSError /
JSONDecodeError / non-object root (return {} only when the file is absent);
run_link catches it and reports the real error. Adds regression tests.
Addresses: rest-3489848551
Review-Thread: #36 (comment)
The origin pin required same host + safe scheme but not the port, so a compromised server could hand back https://forge.allora.network:<arbitrary>/... and the CLI would still open it. Pin the port too (scheme default when unspecified). Client-side hardening; adds a regression test. Addresses: rest-3489848565 Review-Thread: #36 (comment)
_post_json used urllib's default opener, leaving the POST-redirect non-leak an implicit emergent property. Make it explicit with a _NoRedirectHandler opener so a 3xx surfaces as HTTPError and the signed body is never replayed to a redirect target. Adds a unit test; updates the existing transport-mock test. Addresses: rest-3489848570 Review-Thread: #36 (comment)
_select_proxy passed parsed.port (None for HTTPS_PROXY=proxy.corp.local) straight through; http.client then silently defaults to 443/80, so an operator expecting a typical corporate port (3128/8080) gets a confusing connection failure. Emit a warning surfacing the implicit default. Addresses: rest-3489848577 Review-Thread: #36 (comment)
… TODO The hand-rolled _post_json / _JsonPoster transport (proxy/keep-alive/tunneling) duplicates the requests.Session transport allora-sdk-py's ForgeBackendClient already owns for the same host. Add a @@todo pointing at consolidating it into the SDK (cross-repo follow-up); no behavior change. Addresses: rest-3489848540 Review-Thread: #36 (comment)
__init__ stored forge_api_key / forge_backend_url without .strip(), so a whitespace-only value passed _build_run_command's truthiness guard and spawned a subprocess that worker_runtime's strict .strip() check rejects immediately — after the DB row was already marked 'running' (a reconcile restart-fail loop). Strip and coerce empty to None at the boundary. Addresses: rest-3489848469 Review-Thread: #36 (comment)
_provision_wallet_bounded wrapped provision_wallet in a daemon thread joined with a timeout but had no concurrency cap (unlike _release_managed_binding), so repeated deploys against a hung backend accumulated unbounded stuck threads. Add a dedicated BoundedSemaphore(8) (kept separate from the cleanup cap so teardown and deploy can't starve each other), release it in the worker finally, and release+re-raise if Thread.start() fails. Adds regression tests. Addresses: rest-3489848456 Review-Thread: #36 (comment)
_release_managed_binding acquired _cleanup_sem before constructing the daemon thread and released it only in the thread's finally. If Thread.start() raised (e.g. 'can't start new thread' under resource exhaustion) the slot leaked permanently, shrinking the BoundedSemaphore(8) cap until all backend releases were disabled for the process. Release the slot on a start() failure. Adds a regression test. Addresses: rest-3489848475 Review-Thread: #36 (comment)
_validate_artifact_for_deploy read the whole artifact via read_bytes() to check for the legacy load_raw banner, and it runs before the custody branch — so a managed deploy of a 100MB+ pickle peaked at 100MB in the guard alone even though the rest of the managed path streams. Scan in 64 KiB chunks with a needle-sized overlap and early-exit, mirroring _artifact_sha256. Adds tests incl. a marker-across-chunk-boundary case. Addresses: rest-3489848484 Review-Thread: #36 (comment)
_build_run_command called the static _allora_api_key_present() on every worker launch; reconcile() iterating an N-worker file-keyed fleet did up to 2N file reads per pass. Memoize the result lazily on the instance (stable for the process lifetime) behind _ensure_allora_api_key_present; keep the static resolver intact. Addresses: rest-3489848523 Review-Thread: #36 (comment)
ForgeClientProtocol/ProvisionedWallet are declared in the consumer (builder-kit), so SDK method drift is only caught at the first managed deploy's runtime isinstance check. Add a source-of-truth note + @@todo to move the canonical contract into allora-sdk-py and import it (cross-repo follow-up). Addresses: rest-3489848521 Review-Thread: #36 (comment)
…ic API The lazy import targets the internal module allora_sdk.rpc_client.remote_signer, so an SDK refactor moving ForgeBackendClient would break builder-kit silently. Add a @@todo noting allora-sdk-py should add a public re-export so this becomes a top-level import (cross-repo follow-up); no behavior change. Addresses: rest-3489848544 Review-Thread: #36 (comment)
The clear-association SDK call is not cancellable, so under a sustained degraded backend the BoundedSemaphore cap can stay saturated for the process lifetime. Document that recovery is a process restart (no binding leaks — the idempotent get-or-create reconstitutes any skipped release) and add a @@todo for a shorter SDK per-request timeout (cross-repo follow-up). Addresses: rest-3489848560 Review-Thread: #36 (comment)
Hash-aware idempotent reuse (action='reused' on a byte-identical redeploy) is a managed-custody-only guarantee; local custody intentionally allocates a new address (auto) or raises (strict) and only rotates on explicit replace=True. Document the asymmetry on deploy_worker so callers don't expect uniform idempotency across custody modes. Addresses: rest-3489848535 Review-Thread: #36 (comment)
The custody discriminant is switched on in ~6 methods. Add a @@todo at the CustodyMode declaration pointing at a future CustodyStrategy Protocol so a contributor adding a third mode finds the refactor pointer; the abstraction stays deferred (YAGNI) with only two modes today. Addresses: rest-3489848529 Review-Thread: #36 (comment)
…rn type The stub's provision_wallet had no return annotation while the production ForgeClientProtocol.provision_wallet is typed -> ProvisionedWallet, so a type checker couldn't statically confirm the stub satisfies the protocol. Annotate it -> SimpleNamespace. Addresses: rest-3489848575 Review-Thread: #36 (comment)
Per the style guide (don't over-comment function bodies), collapse the multi-sentence inline blocks in _JsonPoster.post and _deploy_managed_worker that restated rationale already in their docstrings down to one-line decision cues, keeping the appropriate RFC/close-before-raise/perf cues. Comment-only; no behavior change. Addresses: rest-3489848548 Review-Thread: #36 (comment)
fix-forge summary — #36Acted on the 28 unresolved review threads that carry a GraphQL thread id. (The other Fixed (23):
Deferred to Linear (0): Deferral is disabled for this run (team Engineering) — would-be deferrals were handled as needs-human below. Won't fix (2):
Needs human input (3, left unresolved):
Cross-repo: No sibling commits made. The cross-repo items are follow-ups, not contract breaks (builder-kit works as-is), and were flagged in the threads + as
Push: ok — |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="allora_forge_builder_kit/worker_manager.py">
<violation number="1" location="allora_forge_builder_kit/worker_manager.py:425">
P2: `acquire(blocking=False)` creates a hard 8-provision concurrency ceiling that applies even to healthy backends, contrary to the docstring's stated intent of capping only timed-out-but-still-running threads. On a healthy backend, the 9th concurrent deploy immediately fails with a misleading "backend may be degraded" message.</violation>
</file>
<file name="allora_forge_builder_kit/wallet_link.py">
<violation number="1" location="allora_forge_builder_kit/wallet_link.py:157">
P3: `discover_keys` can raise `UnicodeDecodeError` instead of `SecretsLoadError` when the secrets file is binary. `path.read_text()` raises `UnicodeDecodeError` (a `ValueError` subclass) on non-UTF-8 content, but the except clause only catches `json.JSONDecodeError` and `OSError`. Add `ValueError` or `UnicodeError` to the except tuple so the error is cleanly wrapped.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| """ | ||
| result: dict[str, Any] = {} | ||
|
|
||
| if not self._provision_sem.acquire(blocking=False): |
There was a problem hiding this comment.
P2: acquire(blocking=False) creates a hard 8-provision concurrency ceiling that applies even to healthy backends, contrary to the docstring's stated intent of capping only timed-out-but-still-running threads. On a healthy backend, the 9th concurrent deploy immediately fails with a misleading "backend may be degraded" message.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At allora_forge_builder_kit/worker_manager.py, line 425:
<comment>`acquire(blocking=False)` creates a hard 8-provision concurrency ceiling that applies even to healthy backends, contrary to the docstring's stated intent of capping only timed-out-but-still-running threads. On a healthy backend, the 9th concurrent deploy immediately fails with a misleading "backend may be degraded" message.</comment>
<file context>
@@ -375,19 +415,37 @@ def _provision_wallet_bounded(
"""
result: dict[str, Any] = {}
+ if not self._provision_sem.acquire(blocking=False):
+ raise TimeoutError(
+ f"too many in-flight managed-wallet provisions; refusing to provision topic "
</file context>
| except (json.JSONDecodeError, OSError) as exc: | ||
| # Present-but-unreadable/corrupt is distinct from not-found: surface it instead of | ||
| # masking it as the "no worker keys, create one" case. | ||
| raise SecretsLoadError(f"could not read worker secrets at {secrets_path}: {exc}") from exc |
There was a problem hiding this comment.
P3: discover_keys can raise UnicodeDecodeError instead of SecretsLoadError when the secrets file is binary. path.read_text() raises UnicodeDecodeError (a ValueError subclass) on non-UTF-8 content, but the except clause only catches json.JSONDecodeError and OSError. Add ValueError or UnicodeError to the except tuple so the error is cleanly wrapped.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At allora_forge_builder_kit/wallet_link.py, line 157:
<comment>`discover_keys` can raise `UnicodeDecodeError` instead of `SecretsLoadError` when the secrets file is binary. `path.read_text()` raises `UnicodeDecodeError` (a `ValueError` subclass) on non-UTF-8 content, but the except clause only catches `json.JSONDecodeError` and `OSError`. Add `ValueError` or `UnicodeError` to the except tuple so the error is cleanly wrapped.</comment>
<file context>
@@ -126,26 +130,35 @@ def _checked_key_file(base: str, address: str, key_file: str) -> str:
- return {}
+ # Present-but-unreadable/corrupt is distinct from not-found: surface it instead of
+ # masking it as the "no worker keys, create one" case.
+ raise SecretsLoadError(f"could not read worker secrets at {secrets_path}: {exc}") from exc
if not isinstance(raw, dict):
- # Valid JSON whose root is a list/scalar would crash on raw.items(); treat a
</file context>
TaniBuilds
left a comment
TaniBuilds
left a comment
There was a problem hiding this comment.
🔍 Multi-Agent Inline Review
26 inline findings posted as resolvable threads.
Generated by code-review-forge
| ) | ||
| return (parsed.hostname, parsed.port, auth) | ||
|
|
||
| def _connect(self) -> http.client.HTTPConnection: |
There was a problem hiding this comment.
🟠 HIGH · 🔬 deep-analysis · synth-028
_JsonPoster._connect uses HTTPSConnection to all proxies, breaking standard HTTP CONNECT proxy setup
🤝 flagged by 2 agents: deep-analysis, adversarial
_JsonPoster._connect() (wallet_link.py:373-387) decides the connection type by the Forge target scheme, not the proxy scheme. When self._https is True and a proxy is set, it always creates http.client.HTTPSConnection(proxy_host, proxy_port). For the canonical enterprise configuration HTTPS_PROXY=http://proxy:3128 (plaintext HTTP proxy that CONNECT-tunnels TLS sessions), this incorrectly attempts TLS to the proxy itself rather than issuing a plaintext CONNECT request. Result: /device/start and /device/submit succeed via urllib (which handles proxy scheme correctly), but every /device/poll iteration fails with a TLS handshake error against the proxy. The device-link flow appears permanently stuck behind corporate proxies even though the proxy config is correct.
_select_proxy discards parsed.scheme and returns only (host, port, auth), so _connect has no way to distinguish an http:// proxy from an https:// proxy — the bug is structural, requiring an API change to _select_proxy to surface the proxy scheme.
Additionally, the regression test test_jsonposter_connect_tunnels_https_through_proxy (tests/test_wallet_link.py:214-229) asserts conn.host, conn.port, and conn._tunnel_host but never asserts type(conn) is http.client.HTTPConnection. The test passes against the buggy code and cements the broken behavior as expected, meaning a fix also requires updating the test.
🎭 _adversarial note: adv-001 confirmed the bug is real and escalates: the unit test at tests/test_wallet_link.py:214-229 does NOT assert connection type, so it passes against the buggy code and codifies the broken behavior. A fix without updating the test is incomplete. The select_proxy API must also be changed to return the proxy scheme, making this architectural rather than a one-liner.
| def _connect(self) -> http.client.HTTPConnection: | |
| @staticmethod | |
| def _select_proxy(base_url: str, https: bool) -> tuple[str, int | None, str | None, str] | None: | |
| """Returns (host, port, auth, proxy_scheme) or None for direct connection.""" | |
| host = urlsplit(base_url).hostname or "" | |
| if urllib.request.proxy_bypass(host): | |
| return None | |
| proxies = urllib.request.getproxies() | |
| proxy_url = proxies.get("https" if https else "http") or proxies.get("all") | |
| if not proxy_url: | |
| return None | |
| parsed = urlsplit(proxy_url if "://" in proxy_url else f"//{proxy_url}", scheme="http") | |
| if not parsed.hostname: | |
| return None | |
| auth = None | |
| if parsed.username is not None: | |
| user = unquote(parsed.username) | |
| password = unquote(parsed.password) if parsed.password is not None else "" | |
| token = base64.b64encode(f"{user}:{password}".encode("utf-8")).decode("ascii") | |
| auth = f"Basic {token}" | |
| return (parsed.hostname, parsed.port, auth, parsed.scheme or "http") | |
| def _connect(self) -> http.client.HTTPConnection: | |
| if self._proxy is not None: | |
| proxy_host, proxy_port, proxy_auth, proxy_scheme = (*self._proxy, self._proxy_auth, self._proxy_scheme) | |
| # Choose connection type by PROXY scheme, not Forge target scheme. | |
| # For http:// proxies: plaintext socket to proxy, CONNECT tunnel carries TLS. | |
| # For https:// proxies: TLS socket to proxy, CONNECT tunnel carries TLS. | |
| if proxy_scheme == "https": | |
| conn = http.client.HTTPSConnection(proxy_host, proxy_port, timeout=self._timeout) | |
| else: | |
| conn = http.client.HTTPConnection(proxy_host, proxy_port, timeout=self._timeout) | |
| if self._https: | |
| tunnel_headers = {"Proxy-Authorization": self._proxy_auth} if self._proxy_auth else {} | |
| conn.set_tunnel(self._host, self._port, headers=tunnel_headers) | |
| return conn | |
| if self._https: | |
| return http.client.HTTPSConnection(self._host, self._port, timeout=self._timeout) | |
| return http.client.HTTPConnection(self._host, self._port, timeout=self._timeout) |
| self._conn = None | ||
|
|
||
|
|
||
| def run_link( |
There was a problem hiding this comment.
🟠 HIGH · 🏗️ architecture · synth-029
wallet_link / workerctl link ignore $FORGE_BACKEND_URL, silently targeting production Forge when staging is intended
🤝 flagged by 2 agents: architecture, adversarial
run_link() (wallet_link.py:431-437) declares forge_url: str = DEFAULT_FORGE_URL as a default parameter and never reads $FORGE_BACKEND_URL. The managed-custody half of this same PR treats $FORGE_BACKEND_URL as the canonical Forge backend env var (worker_manager._forge_client at line 176, worker_runtime._validate_managed_env at line 71). The workerctl link subparser (workerctl.py:79-93) also uses a hard default of the production URL.
An operator on a staging or self-hosted deployment who exports FORGE_BACKEND_URL=https://staging.forge.allora.network and runs workerctl link silently starts a device session against the PRODUCTION Forge backend. The production Forge records the ADR-036 signatures and binds the worker addresses to the production user's account. The operator's staging account never sees the device session. The failure is silent: production returns HTTP 200 and the CLI prints a success message.
This is not merely confusing — it binds the operator's worker wallets to a Forge account they did not intend, and the binding must be explicitly released server-side. A CI pipeline that exports FORGE_BACKEND_URL=staging for every job would silently link workers to production on every run.
The allora-sdk-py sibling's AlloraWalletConfig.from_env() already reads FORGE_BACKEND_URL as the canonical variable, establishing the env-var as the cross-SDK contract.
🎭 adversarial note: adv-005 argues this should be CRITICAL, not HIGH: the failure mode is silent and the prod-binding is irreversible from the operator side (requires the prod-Forge user to release the binding). Three additional factors cited: (1) no diagnostic surface — unlike the managed runtime path that warns, wallet_link has neither warning nor env-var; (2) CI pipelines that export FORGE_BACKEND_URL=staging would link to production on every run; (3) the worker wallets are bound to a stranger's production account, not the operator's. The original HIGH was kept over CRITICAL because the exploit requires the operator already having production Forge credentials reachable, but operators evaluating severity should factor in irreversibility.
| def run_link( | |
| def run_link( | |
| forge_url: str | None = None, | |
| secrets_path: str = DEFAULT_SECRETS_PATH, | |
| addresses: list[str] | None = None, | |
| open_browser: bool = True, | |
| insecure: bool = False, | |
| ) -> int: | |
| """Drive the full device flow. Returns a process exit code.""" | |
| # Match worker_manager / worker_runtime: --forge-url overrides, then $FORGE_BACKEND_URL, | |
| # then the prod default. Without this, a staging operator who exports | |
| # FORGE_BACKEND_URL=https://staging.forge.allora.network and runs `workerctl link` | |
| # silently links against the production Forge account. | |
| if forge_url is None: | |
| forge_url = os.environ.get("FORGE_BACKEND_URL") or DEFAULT_FORGE_URL | |
| if not os.environ.get("FORGE_BACKEND_URL", "").strip(): | |
| print( | |
| "warning: FORGE_BACKEND_URL is not set; defaulting to the public production " | |
| "backend (https://forge.allora.network). Set it to target a staging or " | |
| "self-hosted Forge instance.", | |
| file=sys.stderr, | |
| ) | |
| forge_url = forge_url.rstrip("/") |
🔗 Cross-repo: affects allora-sdk-py, forge-v2
| @@ -696,7 +1268,7 @@ def _default_identity_creator() -> tuple[str, str, str]: | |||
| alias = f"identity_{uuid.uuid4().hex[:12]}" | |||
There was a problem hiding this comment.
🟡 MEDIUM · ✅ correctness · synth-030
WorkerManager._load_secrets silently destroys all prior identities on corrupt secrets file
WorkerManager._load_secrets (worker_manager.py:1246-1250) uses a blanket except Exception: return {} that conflates two distinct cases: a missing file (expected empty on first run) and a corrupt/unreadable file (a signal of damage to existing data).
When worker_secrets.json exists but contains invalid JSON, _load_secrets() silently returns {}. The caller _insert_identity (worker_manager.py:1183-1186) then does:
data = self._load_secrets() # returns {}
data[alias] = {"address": address, "key_file": str(key_file)}
self._save_secrets(data) # overwrites the file with only the new entryThe very first deploy after a corruption event silently nukes every prior worker's key file reference. The operator then sees No key file found for address X on subsequent starts — a phantom missing-key bug with no indication the secrets file was corrupted.
This is directly asymmetric with wallet_link.discover_keys (wallet_link.py:133-161), added in this same PR, which explicitly distinguishes the corrupt case from the absent case and raises SecretsLoadError rather than masking it as "no keys found". The PR author even wrote: "Present-but-unreadable/corrupt is distinct from not-found: surface it instead of masking it as the 'no worker keys, create one' case." That principle was applied to the wallet-link half and not the worker-manager half.
| alias = f"identity_{uuid.uuid4().hex[:12]}" | |
| def _load_secrets(self) -> dict: | |
| # Treat missing-file as empty (first-run bootstrap), but a present-but-unreadable file is a | |
| # corruption signal — surface it instead of silently overwriting prior identities on the | |
| # next _save_secrets. This mirrors wallet_link.discover_keys' SecretsLoadError handling. | |
| if not self.secrets_path.exists(): | |
| return {} | |
| try: | |
| raw = json.loads(self.secrets_path.read_text()) | |
| except (json.JSONDecodeError, OSError) as exc: | |
| raise RuntimeError( | |
| f"worker secrets at {self.secrets_path} is unreadable/corrupt: {exc}; " | |
| "refusing to overwrite existing entries" | |
| ) from exc | |
| if not isinstance(raw, dict): | |
| raise RuntimeError( | |
| f"worker secrets at {self.secrets_path} is not a JSON object" | |
| ) | |
| return raw |
| @@ -732,7 +1324,22 @@ def _materialize_artifact(self, topic_id: int, address: str, source_artifact: Pa | |||
| shutil.copy2(source_artifact, target_path) | |||
There was a problem hiding this comment.
🟡 MEDIUM · 🔒 security · synth-031
Worker address used unsanitized in filesystem paths — backend-supplied or user-supplied address can escape artifact_dir and runtime_log_dir
_materialize_artifact (worker_manager.py:1320-1325) does:
target_dir = self.artifact_dir / f"topic_{topic_id}" / address
target_dir.mkdir(parents=True, exist_ok=True)start_worker (worker_manager.py:932) does:
log_path = self.runtime_log_dir / f"worker_{topic_id}_{address}.log"address is used verbatim in both path constructions with no validation. pathlib does NOT normalize .. in joins: Path('/managed_artifacts/topic_1') / '../../etc' resolves to Path('/managed_artifacts/topic_1/../../etc') which the OS resolves to /etc, and mkdir(parents=True) will create it.
Two paths reach this without operator vetting:
- Managed custody:
info.addressfrom_provision_wallet_bounded(line 718) comes from the Forge backend'sprovision_walletresponse. A compromised backend returninginfo.address = '../../tmp/exfil'would write the pickled model outsideartifact_dirand the log path outsideruntime_log_dir. The address is then stored in the workers table and trusted on all subsequent starts. - Local custody / programmatic callers:
deploy_worker(address='../etc/xyz', mnemonic=...)from a notebook or test harness silently escapes the materialized artifact directory.
Aliases are explicitly sanitized via _sanitize_alias (worker_manager.py:1188-1191) with re.sub(r'[^a-zA-Z0-9_\-]', '_', alias), but addresses receive no equivalent treatment. The wallet_link half of this PR already enforces the bech32 grammar on the signer field (re.fullmatch(r'[a-z0-9]+', signer) at wallet_link.py:64).
| shutil.copy2(source_artifact, target_path) | |
| _ADDRESS_RE = re.compile(r"^allo1[a-z0-9]+$") | |
| @classmethod | |
| def _validated_address(cls, address: str) -> str: | |
| """Validate address against the bech32 grammar to prevent path traversal. | |
| Mirrors wallet_link.build_adr036_sign_doc's signer regex. Rejects addresses containing | |
| path components (e.g. '../../etc') before they reach artifact_dir / runtime_log_dir. | |
| """ | |
| if not isinstance(address, str) or not cls._ADDRESS_RE.fullmatch(address): | |
| raise ValueError(f"invalid worker address: {address!r}") | |
| return address |
| if all(found): | ||
| break | ||
| tail = window[-overlap:] | ||
| except Exception: |
There was a problem hiding this comment.
🟡 MEDIUM · ✅ correctness · synth-032
_validate_artifact_for_deploy swallows every Exception, converting the guardrail into a silent bypass
_validate_artifact_for_deploy (worker_manager.py:1417-1427) wraps the entire chunked-read in a blanket except Exception: return. The docstring bills it as a guardrail that blocks known-bad artifacts. But any I/O error reading the file — PermissionError, FileNotFoundError between the exists() check and open(), ENOSPC, a TOCTOU race — silently returns without validation, and the deploy proceeds with an un-validated artifact.
The deploy path then calls _artifact_sha256 and shutil.copy2 against the same file. If the file raised PermissionError on the first read, those subsequent calls will also fail — so the deploy eventually errors, but with no indication that validation never ran. Operators chase a misleading downstream error instead of a clear 'could not validate artifact' message.
Worse: if a symlink or a race causes the validation pass to read a different file that raises (e.g. a dev/zero symlink that times out), the actual target file is deployed without ever being scanned for the load_raw forbidden pattern.
The deploy path correctly fails loudly on missing artifacts (if not artifact.exists(): raise FileNotFoundError at worker_manager.py:595-596); the validation step should apply the same fail-loud-early principle.
| except Exception: | |
| def _validate_artifact_for_deploy(self, artifact_path: Path) -> None: | |
| """Block known-bad artifact variants from deployment. Streams in 64 KiB chunks. | |
| Raises ValueError on a known-bad needle. I/O errors propagate so a permission/read | |
| failure fails the deploy at this step rather than silently bypassing the guard. | |
| """ | |
| needles = (b"load_raw", b"Could not get current price from raw data") | |
| overlap = max(len(n) for n in needles) - 1 | |
| found = [False, False] | |
| with artifact_path.open("rb") as f: | |
| tail = b"" | |
| for chunk in iter(lambda: f.read(65536), b""): | |
| window = tail + chunk | |
| found = [hit or needle in window for hit, needle in zip(found, needles)] | |
| if all(found): | |
| break | |
| tail = window[-overlap:] | |
| if all(found): | |
| raise ValueError( | |
| f"Refusing to deploy artifact with raw-data inference path: {artifact_path}. " | |
| "Use export_predict_self_contained.py." | |
| ) |
| return {"https": 443, "http": 80}.get(scheme) | ||
|
|
||
|
|
||
| def _submit_rejection(submit: dict[str, Any]) -> str | None: |
There was a problem hiding this comment.
🔵 LOW · 🔗 cross-repo · synth-049
_submit_rejection parses a 200-with-rejected/error response shape the forge-v2 server never emits
_submit_rejection (wallet_link.py:275-297) parses a {rejected: [{address, reason}], error: ...} body from a 200 response. The forge-v2 sibling branch's SubmitDeviceLink only emits {"status": "submitted"} on success or a 4xx/5xx via handleError on failure — it never writes a rejected JSON key. The function therefore always returns None in production. The per-address rejection UX is dead code against the current server contract, and the docstring misrepresents server behavior.
This is benign today (no crash), but the docstring-vs-server drift could mislead future developers who add partial-rejection logic and assume the client handles it. Either the forge-v2 server should emit the documented shape, or the builder-kit docstring and _submit_rejection should be simplified to match the actual {status: submitted} / HTTP-error contract.
💡 Suggestion: Confirm with the forge-v2 team whether partial-signature submit rejection returning 200 is planned. If not, simplify to check only for submit.get('error') and rely on the existing HTTP-error path, or remove _submit_rejection and update the docstring.
🔗 Cross-repo: affects forge-v2
| ) | ||
| return 1 | ||
|
|
||
| selected = addresses or list(keys.keys()) |
There was a problem hiding this comment.
⚪ INFO · ✅ correctness · synth-050
Duplicate --address flags pass through to submit payload without deduplication
In run_link, selected = addresses or list(keys.keys()) (wallet_link.py:477) preserves duplicates if a user repeats --address allo1foo --address allo1foo. The CLI iterates over selected and produces a fresh signature per occurrence, so the submit payload contains duplicate {address, pubkey, signature} objects. The forge-v2 server dedupes addresses in StartDeviceSession so the challenges map contains only one entry per unique address — but the submit endpoint doesn't dedupe signatures received, and a server-side validator checking len(signatures) == len(challenges) could reject a duplicated-address submission.
💡 Suggestion: Dedupe while preserving order: selected = list(dict.fromkeys(addresses or keys.keys())).
🔗 Cross-repo: affects forge-v2
| # topic_id fallback so the backend wallet label is human-meaningful. | ||
| label = self._resolve_topic_desc(topic_id, topic_desc) or f"worker-topic-{topic_id}" | ||
| info = self._provision_wallet_bounded(client, topic_id, label) | ||
| if not getattr(info, "address", None) or not getattr(info, "id", None): |
There was a problem hiding this comment.
⚪ INFO · ✅ correctness · synth-051
_deploy_managed_worker validates wallet info with truthiness only, not isinstance(str)
_deploy_managed_worker (worker_manager.py:713-717) validates the wallet info returned by _provision_wallet_bounded with getattr(info, 'address', None) / getattr(info, 'id', None) truthy checks. Non-string values (e.g. info.address = 12345) pass the check and propagate into FS path construction and env injection. Current SDK validates via _validate(SigningWalletInfo, raw, ...) so this is defensive-not-actual today, but adding isinstance(info.address, str) and isinstance(info.id, str) at the boundary catches SDK contract drift.
💡 Suggestion: Tighten to: if not (isinstance(getattr(info, 'address', None), str) and info.address and isinstance(getattr(info, 'id', None), str) and info.id):
🔗 Cross-repo: affects allora-sdk-py
| return None | ||
|
|
||
|
|
||
| # @@TODO: This module hand-rolls an HTTP transport (_post_json + _JsonPoster: proxy resolution, |
There was a problem hiding this comment.
⚪ INFO · 🎨 style · synth-052
Non-standard @@todo comment marker not recognized by IDE tooling or CI
The PR uses @@TODO: as a comment prefix at wallet_link.py:300 and worker_manager.py:28, 87, 212, 351. Standard Python tooling (PyCharm, VS Code, flake8) recognizes # TODO: and # FIXME: for task tracking. The double-@ prefix means these items do not appear in IDE TODO panels and would be missed by any CI job scanning for open TODOs.
💡 Suggestion: Replace @@TODO: with # TODO(ENGN-XXXX): throughout, including worker_manager.py:28, 87, 212, 351.
| return None | ||
|
|
||
|
|
||
| # @@TODO: This module hand-rolls an HTTP transport (_post_json + _JsonPoster: proxy resolution, |
There was a problem hiding this comment.
⚪ INFO · 🏗️ architecture · synth-053
wallet_link hand-rolls a bespoke HTTP transport bypassing the ForgeClientProtocol seam
wallet_link.py talks to the same Forge backend (wallet-link endpoints) as worker_manager, but bypasses the ForgeClientProtocol seam entirely, hand-rolling a urllib + http.client transport (_post_json, _JsonPoster) with its own proxy resolution, redirect protection, and bounded-read semantics. The module carries a @@TODO at line 300 acknowledging the duplication. This is not a merge blocker (the wallet-link endpoints are unauthenticated device-flow endpoints the SDK's ForgeBackendClient does not yet expose), but it means two HTTP transports with divergent retry/proxy/redirect semantics coexist for the same product host.
💡 Suggestion: No change required for this PR. Track as a cross-repo follow-up: when allora-sdk-py exposes a DeviceFlowClient, move the wallet-link HTTP transport there so the kit owns only CLI orchestration and ADR-036 signing.
🔗 Cross-repo: affects allora-sdk-py, forge-v2
…_sdk re-export Resolves the @@todo: WorkerManager imported ForgeBackendClient from the internal allora_sdk.rpc_client.remote_signer path, which a future allora-sdk-py refactor could move. allora-sdk-py#83 adds the public re-export (allora_sdk.ForgeBackendClient); use it. The lazy import still runs only on the managed-custody path, so local-custody installs are unaffected. Refs: allora-network/allora-sdk-py#83 (synth-003)
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="allora_forge_builder_kit/worker_manager.py">
<violation number="1" location="allora_forge_builder_kit/worker_manager.py:216">
P1: Import `from allora_sdk import ForgeBackendClient` will fail with the released `allora-sdk>=1.0.6` from PyPI because `ForgeBackendClient` is not re-exported from `allora_sdk.__init__`. The comment says it was "added in allora-sdk-py#83" which is unreleased; only the local dev pin (`[tool.uv.sources]` in pyproject.toml) makes this work. Any user or CI installing the released SDK will get an `ImportError`. Either bump the `allora-sdk` dependency version to a release that includes PR #83, add a conditional fallback to the old internal path, or keep the `from allora_sdk.rpc_client.remote_signer import ForgeBackendClient` import until the SDK release ships.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| # internal allora_sdk.rpc_client.remote_signer path, so an SDK refactor moving the class | ||
| # can't silently break this import. | ||
| try: | ||
| from allora_sdk import ForgeBackendClient |
There was a problem hiding this comment.
P1: Import from allora_sdk import ForgeBackendClient will fail with the released allora-sdk>=1.0.6 from PyPI because ForgeBackendClient is not re-exported from allora_sdk.__init__. The comment says it was "added in allora-sdk-py#83" which is unreleased; only the local dev pin ([tool.uv.sources] in pyproject.toml) makes this work. Any user or CI installing the released SDK will get an ImportError. Either bump the allora-sdk dependency version to a release that includes PR #83, add a conditional fallback to the old internal path, or keep the from allora_sdk.rpc_client.remote_signer import ForgeBackendClient import until the SDK release ships.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At allora_forge_builder_kit/worker_manager.py, line 216:
<comment>Import `from allora_sdk import ForgeBackendClient` will fail with the released `allora-sdk>=1.0.6` from PyPI because `ForgeBackendClient` is not re-exported from `allora_sdk.__init__`. The comment says it was "added in allora-sdk-py#83" which is unreleased; only the local dev pin (`[tool.uv.sources]` in pyproject.toml) makes this work. Any user or CI installing the released SDK will get an `ImportError`. Either bump the `allora-sdk` dependency version to a release that includes PR #83, add a conditional fallback to the old internal path, or keep the `from allora_sdk.rpc_client.remote_signer import ForgeBackendClient` import until the SDK release ships.</comment>
<file context>
@@ -208,17 +208,16 @@ def _forge_client(self) -> ForgeClientProtocol:
+ # can't silently break this import.
try:
- from allora_sdk.rpc_client.remote_signer import ForgeBackendClient
+ from allora_sdk import ForgeBackendClient
except ImportError as e:
raise ValueError(
</file context>
…e epic branch Syncs allora_forge_builder_kit/wallet_link.py and tests/test_wallet_link.py to the reviewed state on the ENGN-8456 reconciliation branch (#36) so the wallet-linking CLI can ship ahead of the privy-delegation work. Brings the hardening done during epic review: HTTP(S) proxy + auth support for the keep-alive poller, monotonic-clock deadline, IPv6 ::1 loopback trust, no-redirect device POSTs, origin/port pinning, robust secrets parsing (skip non-string/non-object entries, relative key_file resolution, escape-path warnings), terminal-4xx poll stop, and early submit-rejection surfacing. Intentionally excluded as ENGN-8646 (managed custody / privy delegation), not wallet linking: worker_manager.py, worker_runtime.py, notebooks/, the [tool.uv.sources] local SDK pin in pyproject.toml, and the release.yml publish guard. cosmpy stays pinned ==0.11.1 (the loosening to >=0.11.1 was only to coexist with the local SDK pin, which this branch does not carry).
…e epic branch Syncs allora_forge_builder_kit/wallet_link.py and tests/test_wallet_link.py to the reviewed state on the ENGN-8456 reconciliation branch (#36) so the wallet-linking CLI can ship ahead of the privy-delegation work. Brings the hardening done during epic review: HTTP(S) proxy + auth support for the keep-alive poller, monotonic-clock deadline, IPv6 ::1 loopback trust, no-redirect device POSTs, origin/port pinning, robust secrets parsing (skip non-string/non-object entries, relative key_file resolution, escape-path warnings), terminal-4xx poll stop, and early submit-rejection surfacing. Intentionally excluded as ENGN-8646 (managed custody / privy delegation), not wallet linking: worker_manager.py, worker_runtime.py, notebooks/, the [tool.uv.sources] local SDK pin in pyproject.toml, and the release.yml publish guard. cosmpy stays pinned ==0.11.1 (the loosening to >=0.11.1 was only to coexist with the local SDK pin, which this branch does not carry).
…e epic branch Syncs allora_forge_builder_kit/wallet_link.py and tests/test_wallet_link.py to the reviewed state on the ENGN-8456 reconciliation branch (#36) so the wallet-linking CLI can ship ahead of the privy-delegation work. Brings the hardening done during epic review: HTTP(S) proxy + auth support for the keep-alive poller, monotonic-clock deadline, IPv6 ::1 loopback trust, no-redirect device POSTs, origin/port pinning, robust secrets parsing (skip non-string/non-object entries, relative key_file resolution, escape-path warnings), terminal-4xx poll stop, and early submit-rejection surfacing. Intentionally excluded as ENGN-8646 (managed custody / privy delegation), not wallet linking: worker_manager.py, worker_runtime.py, notebooks/, the [tool.uv.sources] local SDK pin in pyproject.toml, and the release.yml publish guard. cosmpy stays pinned ==0.11.1 (the loosening to >=0.11.1 was only to coexist with the local SDK pin, which this branch does not carry).
Automated Code Review — ENGN-8456 Identity & Wallet FoundationVerdict: ✅ Ready to merge Coverage note: This was a single-pass Deepseek V4 Pro review (report-only mode), not the full 3-pass × multi-provider matrix mandated by the Findings
Notable strengths
Refuted false positivesInvestigated and dismissed: Generated by |
Warn operators that managed-custody API keys grant wallet-level signing authority independently of Forge's transfer convenience route. Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Summary
Reconciles the builder-kit side of the ENGN-8456 Identity & Wallet Foundation epic into one branch off
main. Combines / supersedes:workerctl linkdevice-flow CLI for wallet ownership #32 (ENGN-8614) —workerctl link: gh-auth-login-style device flow CLI that proves worker-wallet ownership (ADR-036) against the forge-v2 backend.--custody managedswitch + local SDK pin so managed (Privy) runs don't fall back to the local.allora_keypath.WorkerManager(auto-provision + decommission).Linear stack (
orchestrator → custody → 8614 → main); each branch was brought current with its base and combined here.Validation
uv run pytest→ 93 passed, 23 skipped.🤖 Generated with Cursor
Summary by cubic
Adds
workerctl linkto securely link local worker wallets and completes managed‑custody (Privy) lifecycle with per‑topic auto‑provisioned wallets, hash‑aware redeploys, and hardened prechecks for ENGN‑8456. Also adds a publish guard to block releases that depend on a localallora-sdkand verifies the SDK pin resolves from PyPI.New Features
workerctl link: device flow with ADR‑036 signing (on‑disk keys viacosmpy), headless OK; requires HTTPS and same‑origin Forge URL (no path) unless localhost/--insecure; sanitizes output, bounds response reads; keep‑alive poller with HTTP(S) proxy + auth; clamps intervals, honors serverexpires_in(up to 30m), retries transient errors, stops on terminal 4xx; surfaces submit rejections early; fails if approval omits requested addresses; robust secrets parsing (skip non‑object/non‑string, resolve relativekey_file, warn on duplicates/escape‑paths); validates bech32 signer and safe verification URL.WorkerManager: per‑topic wallet provision/register (bounded, validated) with friendly SDK‑missing error; start injectsFORGE_API_KEY/FORGE_BACKEND_URL/FORGE_SIGNING_WALLET_ID, strips local‑key env, and prechecksALLORA_API_KEY; log files 0600; removal stops the process then clears association on a bounded thread; labels use topic names;status_allexposescustody/signing_wallet_id; Forge client typed via a Protocol, runtime‑checked, and built safely under the manager lock; rejects local‑key inputs whencustody='managed'.replace=True); different/unknown → requirereplace=True; rotations reportaction="replaced".worker_runtime:--custody {local,managed}guards; resolves wallet config before the event loop; managed requires non‑emptyFORGE_API_KEYand warns whenFORGE_BACKEND_URLor fee‑granter (FORGE_MASTER_GRANTER_ADDRESSorFEE_GRANTER) is unset; local requires--mnemonic-file; artifact caller adapts RunContext vs nonce (annotation‑first + safe probe); clearly distinguishesALLORA_API_KEYvsFORGE_API_KEY.[tool.uv.sources]redirectsallora-sdkto a local worktree and verifies the declaredallora-sdkpin resolves from PyPI; optional extra[wallet-link]addscosmpy>=0.11.1.Migration
FORGE_API_KEY(non‑empty) andFORGE_BACKEND_URL; set a fee granter viaFORGE_MASTER_GRANTER_ADDRESS(orFEE_GRANTER); ensureALLORA_API_KEYis present before starting workers.--mnemonic-filewith--custody local.workerctl link --forge-url https://forge.allora.network [--address ...] [--secrets-path ...]; installallora-forge-builder-kit[wallet-link]forcosmpy.Written for commit 4bd7028. Summary will update on new commits.