fix(gl): report quickstart registration failure when the node returns no UCAN (#355)

[beardthelion]

Summary

gl quickstart printed "Registered successfully" and "UCAN saved to ..." on any 2xx from /api/register, including a non-JSON body where resp.json() falls back to Value::Null. Nothing was written, and a stale ucan.json from another node stayed in place while the output claimed success.

Motivation & context

Closes #355

Kind of change

• Bug fix
• Feature
• Security fix
• Docs
• Tests / CI
• Refactor (no behavior change)
• Breaking or protocol change (issue required first)

What changed

• crates/gl/src/quickstart.rs: the success lines moved inside the !ucan.is_empty() guard that already covered the write. A 2xx with no usable ucan now prints a failure row with a retry hint instead.

How a reviewer can verify

Ran the built binary against a stub node returning 200 text/html on /api/register:

• before: Registered successfully / UCAN saved to <path> with no ucan.json written
• after: Registration returned no UCAN (unexpected response body), still no ucan.json, exit continues to step 3 as before

cargo test -p gl

Before you request review

• Scope is one logical change; no unrelated churn
• cargo test -p gl passes locally
• New behavior is covered by tests where a seam exists; the only observable change here is the printed line, which the binary run above verifies directly
• cargo fmt --all and cargo clippy -p gl --all-targets -- -D warnings are clean
• Commit titles use Conventional Commits (feat(...), fix(...), docs(...))
• Docs / .env.example updated if behavior or config changed (or N/A)
• Checked existing PRs so this isn't a duplicate

Protocol & signing impact

• Touches DID / did:key, Ed25519 / RFC 9421 signatures, UCAN, ref certs, or P2P wire formats
• Discussed in an issue before implementation
• Backward-compatible with existing nodes and previously signed history

Notes for reviewers

Overlaps the #354 fix PR in quickstart.rs's register arm: whichever merges second needs a small rebase.

Summary by CodeRabbit

• Bug Fixes
  • Improved gl quickstart handling when registration succeeds without a usable UCAN.
  • Displays a failure message and retry command instead of reporting success.
  • Shows trust score and UCAN-saved messages only when a valid UCAN is received.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Essentials

Run ID: 3a79727d-4365-467e-805b-28756a8a7142

📥 Commits

Reviewing files that changed from the base of the PR and between bfc44f9 and a418c28.

📒 Files selected for processing (1)

• crates/gl/src/quickstart.rs

Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.

---

📝 Walkthrough

Walkthrough

gl quickstart now treats a registration response without a non-empty UCAN as unexpected. It reports failure and provides a retry command. Trust and success messages appear only when a usable UCAN is returned.

Changes

Quickstart registration

Layer / File(s)	Summary
Registration response validation crates/gl/src/quickstart.rs	Success output, trust score output, and UCAN path output now require a non-empty UCAN. Missing UCAN responses report an unexpected response and provide a retry command.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to a418c

Quickstart now reports registration failure for unusable responses while preserving the existing continuation behavior, with no identified merge-blocking impact.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	Issue #355 requires quickstart to report registration failure when a 2xx response has no usable UCAN and to avoid claiming that a token was saved. quickstart.rs now writes the record before printing…	When a 2xx registration response has no usable UCAN, remove or invalidate an existing ucan.json for another node, or otherwise prevent later commands from consuming it. Add an automated regression test for this stale-token case, in additi…
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the fix: gl quickstart must report registration failure when the node returns no UCAN. It is specific and relevant to the main change.
Description check	✅ Passed	The description covers the summary, motivation, bug-fix classification, concrete changes, verification steps, test results, review checklist, compatibility, and reviewer notes. It provides sufficient …
Out of Scope Changes check	✅ Passed	The reviewed change is limited to the registration response handling in crates/gl/src/quickstart.rs. It changes success output, trust-score output, and retry failure output that directly implement i…

Full details: Linked Issues check

Explanation

Issue #355 requires quickstart to report registration failure when a 2xx response has no usable UCAN and to avoid claiming that a token was saved. quickstart.rs now writes the record before printing success, gates the trust score and success messages on a non-empty UCAN, prints a failure with a retry command, and continues to step 3. However, when ucan.json already belongs to another node, the no-UCAN branch leaves that stale file in place. Later commands can still use the stale token, so the misleading registration state remains. The summary reports stub verification and cargo test -p gl; no separate automated regression test is shown in the reviewed changes.

Resolution

When a 2xx registration response has no usable UCAN, remove or invalidate an existing ucan.json for another node, or otherwise prevent later commands from consuming it. Add an automated regression test for this stale-token case, in addition to the no-UCAN response cases.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR corrects gl quickstart output for successful HTTP responses that contain no usable UCAN.

• Success and saved-path messages are now emitted only after a non-empty UCAN is persisted.
• Empty, missing, or non-JSON UCAN responses now produce a registration-failure message and retry guidance.

Confidence Score: 5/5

The PR appears safe to merge and correctly prevents a no-UCAN response from being reported as successful registration.

The new branch accurately reflects whether a usable UCAN was saved and preserves the command’s established behavior of continuing after registration failures; no actionable regression was identified.

Important Files Changed

Filename	Overview
crates/gl/src/quickstart.rs	Aligns registration status output with the existing UCAN persistence guard while preserving quickstart’s intentionally non-fatal control flow.

_{Reviews (1): Last reviewed commit: "fix(gl): report quickstart registration ..." | Re-trigger Greptile}