fix(cli): surface TLSA for on-chain domains, clearer verify output - #669
fix(cli): surface TLSA for on-chain domains, clearer verify output#669pcfreak30 wants to merge 7 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
- websites domains dns-requirements for on-chain managed domains now states the domain is held on-chain and puts the TLSA record in front of the user; without one in the bundle it points at dane republish - websites domains verify human output becomes an outcome: verified confirmation pointing at the served site, or explicit not-verified with retry guidance and the dns-requirements command (also when the backend can't resolve the domain) - add OpWebsitesDomainsVerify/DNSRequirements constants in catalogops and use them in the websites wiring
2d8f57b to
6ecb840
Compare
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
| if result != nil && isNilPointerResult(result) { | ||
| if op.Name() == catalogops.OpWebsitesDomainsVerify { | ||
| renderDomainVerifyResult(output, nil) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
The typed-nil verify branch calls renderDomainVerifyResult(output, nil) without an output.IsJSON() guard, writing human-readable '⏳ not verified yet' text even in --json mode. Under --json, jsonFormatter.Printfln emits the text as a single non-JSON line, so pinner websites domains verify <d> --json on an unresolvable (nil,nil) result produces broken output instead of a machine JSON document, while all sibling branches honor IsJSON. Guard with if output.IsJSON() { return output.PrintJSON(map[string]any{"verified": false, "message": "not verified yet"}) } before calling renderDomainVerifyResult.
if op.Name() == catalogops.OpWebsitesDomainsVerify {
if output.IsJSON() {
return output.PrintJSON(map[string]any{"verified": false, "message": "not verified yet"})
}
renderDomainVerifyResult(output, nil)
return nil
}Prompt for LLM
File internal/cli/catalog_websites_wiring.go:
Line 385 to 389:
The typed-nil verify branch calls renderDomainVerifyResult(output, nil) without an output.IsJSON() guard, writing human-readable '⏳ not verified yet' text even in --json mode. Under --json, jsonFormatter.Printfln emits the text as a single non-JSON line, so `pinner websites domains verify <d> --json` on an unresolvable (nil,nil) result produces broken output instead of a machine JSON document, while all sibling branches honor IsJSON. Guard with `if output.IsJSON() { return output.PrintJSON(map[string]any{"verified": false, "message": "not verified yet"}) }` before calling renderDomainVerifyResult.
Suggested Code:
if op.Name() == catalogops.OpWebsitesDomainsVerify {
if output.IsJSON() {
return output.PrintJSON(map[string]any{"verified": false, "message": "not verified yet"})
}
renderDomainVerifyResult(output, nil)
return nil
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Code Coverage ReportTotal Coverage: 50.0% Generated from commit: 34f5f6e |
- bump go.lumeweb.com/ipfs-sdk to v0.1.96 (swagger sync: tlsa_record -> tlsa_rdata, adds published_to_managed_zone + owner_name) - renderOnchainTLSA falls back to the response's tlsa_rdata when the delegation bundle carries no TLSA record, removing the backend TODO gap for on-chain bindings - dane republish output reports TLSA Published (published_to_managed_zone) - resync mcptest spec/generators to the synced swagger
This comment has been minimized.
This comment has been minimized.
| // published_to_managed_zone is a required field and always echoed; a | ||
| // false value means the republished TLSA is NOT live in the managed | ||
| // zone yet, so the user must not treat the command as a success. | ||
| {"TLSA Published", fmt.Sprintf("%t", r.PublishedToManagedZone)}, | ||
| } | ||
| if r.TlsaRdata != nil && *r.TlsaRdata != "" { | ||
| fields = append(fields, Field{"TLSA Record", *r.TlsaRdata}) | ||
| } | ||
| output.PrintFields(FieldGroup{Fields: fields}) |
There was a problem hiding this comment.
The renderDomainDANEResponse function silently reports success when published_to_managed_zone is false, contradicting the code's own comment (lines 628-631) that a false value means the TLSA is NOT live. Return an error from the renderer/caller path or propagate failure through the exit code so a failed zone publication exits non-zero.
// published_to_managed_zone=false means the TLSA did NOT land in the
// managed zone; surface it as a failure so the command does not exit 0.
fields := []Field{
{"ID", fmt.Sprintf("%d", r.Id)},
{"Domain", r.Domain},
{"Namespace", string(r.Namespace)},
{"Status", status},
{"Owner Name", ownerName},
{"TLSA Published", fmt.Sprintf("%t", r.PublishedToManagedZone)},
}
if r.TlsaRdata != nil && *r.TlsaRdata != "" {
fields = append(fields, Field{"TLSA Record", *r.TlsaRdata})
}
output.PrintFields(FieldGroup{Fields: fields})
if !r.PublishedToManagedZone {
return fmt.Errorf("TLSA was not published to the managed zone")
}
return nil
}Prompt for LLM
File internal/cli/catalog_websites_wiring.go:
Line 628 to 636:
The renderDomainDANEResponse function silently reports success when published_to_managed_zone is false, contradicting the code's own comment (lines 628-631) that a false value means the TLSA is NOT live. Return an error from the renderer/caller path or propagate failure through the exit code so a failed zone publication exits non-zero.
Suggested Code:
// published_to_managed_zone=false means the TLSA did NOT land in the
// managed zone; surface it as a failure so the command does not exit 0.
fields := []Field{
{"ID", fmt.Sprintf("%d", r.Id)},
{"Domain", r.Domain},
{"Namespace", string(r.Namespace)},
{"Status", status},
{"Owner Name", ownerName},
{"TLSA Published", fmt.Sprintf("%t", r.PublishedToManagedZone)},
}
if r.TlsaRdata != nil && *r.TlsaRdata != "" {
fields = append(fields, Field{"TLSA Record", *r.TlsaRdata})
}
output.PrintFields(FieldGroup{Fields: fields})
if !r.PublishedToManagedZone {
return fmt.Errorf("TLSA was not published to the managed zone")
}
return nil
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
- bump go.lumeweb.com/ipfs-sdk to v0.1.97 (adds ValidationCheck on DomainResponse and WebsiteValidateResponse) and resync the mcptest spec/generators to the new swagger - dns-requirements, verify, and websites validate now turn the backend per-record checks (validation token, dnslink, TLSA, delegation) into an actionable to-do list: a passing/failing summary plus, for each failing record, a verbatim copyable expected value - the dnslink record a managed HNS site needs is no longer missing from the output - check values are rendered as plain lines, never a table, so long record values are not hard-wrapped and stay copyable
This comment has been minimized.
This comment has been minimized.
- the backend no longer ships a delegation bundle (or checks) for on-chain bindings, so the dns-requirements handler now attaches the owning website and the HNS driver derives the authoritative record set client-side: the _dnslink TXT (name/type/value from the website's target) plus the server-served TLSA (owner_name/tlsa_rdata) - renderDomainDelegation passes the owning website through the delegation driver registry; drivers that do not need it ignore it - dns-requirements --json keeps the historical domain-response shape
This comment has been minimized.
This comment has been minimized.
- the nil-Delegation early return in the domains wizard's delegation step bypassed renderDomainDelegation, so on-chain managed bindings - which carry no bundle - never got the derived dnslink/TLSA records; only a nil response is nothing-available now
This comment has been minimized.
This comment has been minimized.
| // A nil Delegation is still worth routing through the renderer: on-chain | ||
| // managed bindings carry none, and the on-chain branch renders the | ||
| // derived records (dnslink, TLSA) instead of a delegation bundle. Only a | ||
| // nil overall response is "nothing available". | ||
| if delegResult == nil { | ||
| w.output.Printfln("No delegation records are available for %s.", result.Domain) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
The newly added block (lines 182-189) is unreachable dead code: it checks delegResult == nil again, but the pre-existing guard at line 177 (delegResult == nil || delegResult.Delegation == nil) has already returned for an overall nil response. This defeats the intent of routing a nil Delegation (on-chain managed binding) through renderDomainDelegation so its derived dnslink/TLSA records are surfaced, leaving on-chain managed domains stuck at "No delegation records are available" and the promised TLSA record hidden. Relax line 177 to return only when the whole response is nil (if delegResult == nil) and have the new block check delegResult.Delegation == nil, or remove the dead block since renderer already handles nil Delegation per websites_domains.go:88-92 and websites_domains_test.go:320-335.
if delegResult == nil {
w.output.Printfln("No delegation records are available for %s.", result.Domain)
return nil
}
// N.B. a nil Delegation is intentionally routed through renderDomainDelegation,
// which renders on-chain derived records (dnslink, TLSA) — see websites_domains.go.Prompt for LLM
File internal/cli/websites_domains_wizard.go:
Line 182 to 189:
The newly added block (lines 182-189) is unreachable dead code: it checks `delegResult == nil` again, but the pre-existing guard at line 177 (`delegResult == nil || delegResult.Delegation == nil`) has already returned for an overall nil response. This defeats the intent of routing a nil `Delegation` (on-chain managed binding) through renderDomainDelegation so its derived dnslink/TLSA records are surfaced, leaving on-chain managed domains stuck at "No delegation records are available" and the promised TLSA record hidden. Relax line 177 to return only when the whole response is nil (`if delegResult == nil`) and have the new block check `delegResult.Delegation == nil`, or remove the dead block since renderer already handles nil Delegation per websites_domains.go:88-92 and websites_domains_test.go:320-335.
Suggested Code:
if delegResult == nil {
w.output.Printfln("No delegation records are available for %s.", result.Domain)
return nil
}
// N.B. a nil Delegation is intentionally routed through renderDomainDelegation,
// which renders on-chain derived records (dnslink, TLSA) — see websites_domains.go.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
- bare tlsa_rdata tells the user what to publish but not where; the record lives at the TCP port 443 service of the domain, so the owner name (_443._tcp.<domain>, or the server-provided owner_name when present) is now in both the copy line and derivable - kept the TYPE/VALUE table layout: the no-hang wrap rule for long digest values keys off TYPE in the first column
This comment has been minimized.
This comment has been minimized.
- the on-chain TLSA table is NAME/TYPE/VALUE now, so the copyable record includes where it goes (_443._tcp.<domain>) - keepWholeValue keeps the digest in the VALUE column of this layout whole-token, extending the no-wrap guarantee to the new shape
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
1 participant
Reworks
websites domains dns-requirementsfor on-chain managed domains: states the domain is held on-chain with its DNS records set on-chain, and puts the TLSA record in front of the user (rendered from the bundle when the backend returns it; otherwise points atdane republishfor now). Replaces the old copy that claimed no DNS setup was needed.Reworks
websites domains verifyhuman output into an outcome: a verified confirmation pointing at the site the domain serves (with a TLSA reminder for on-chain managed domains, where verification doesn't prove the TLSA is published), or an explicit not-verified-yet message with retry guidance and thedns-requirementscommand to fix records (including when the backend can't resolve the domain yet). JSON output is unchanged. Operation name constants are added in catalogops and used by the websites wiring.