feat(get trail): add --output markdown format

[meekrosoft]

What

Adds markdown as an output format for kosli get trail:

kosli get trail $TRAIL --flow $FLOW --output markdown

This is the first thin slice of an explicit, opt-in approach to #904 (CI summaries), proposed as an alternative to implicitly summarising every command in CI. See the discussion comment for the full rationale.

The idea: output formatting is already a format → renderer registry, so markdown slots in next to table/json. The user redirects it wherever they want — which works identically for both CIs and sidesteps GitLab's lack of a native job summary:

# GitHub Actions
kosli get trail $TRAIL --flow $FLOW --output markdown >> "$GITHUB_STEP_SUMMARY"

# GitLab (then expose summary.md as an artifact)
kosli get trail $TRAIL --flow $FLOW --output markdown > summary.md

How

• printTrailAsMarkdown renders trail metadata, an optional git-commit block, and the events list as GitHub-Flavored Markdown tables.
• mdCell escapes |/newlines so cell values can't break the table layout.
• Extracted eventFields from eventRow so the table and markdown renderers share the event-description logic — table output is byte-identical.
• Registered markdown in the get trail output map and updated its --output help text.

Tests

• Golden test: get trail --output markdown → testdata/output/get/get-trail-markdown.txt (runs in the integration suite against the local server).
• A server-free renderer unit test (TestPrintTrailAsMarkdown) feeds a fixture through printTrailAsMarkdown and asserts it matches the golden byte-for-byte — passes locally without the test server.

Scope / open questions

• Draft to work out the approach before fanning out.
• This slice covers get trail only. If the approach lands, next slices extend to the other read/detail commands (get artifact, get snapshot, diff snapshots, assert artifact), then docs with the CI usage examples above.
• Intentionally does not touch write commands (attest *, report artifact) — they emit a one-line message, not structured data worth rendering.

Refs #904

🤖 Generated with Claude Code

[claude]

Claude finished @meekrosoft's task in 3m 16s —— View job

---

Claude Code Review

Static analysis: go build ./... ✅ · go vet ./cmd/kosli/ ✅ · go mod tidy → no diff ✅ (golangci-lint not available in this runner, so the full lint suite wasn't run.)

Overall this is a clean, well-scoped slice that grew nicely since the first draft. The format → renderer registry fit is the right call, the eventFields extraction keeps table output byte-identical while sharing the description/link logic, and the test suite is genuinely thorough — escaping, nil cells, empty events, git-commit block, attestation statuses, and every link variant each have a server-free unit test. The earlier review notes (the <nil> cell rendering and the markdown-branch coverage gap) have both been addressed. Nice work.

Findings

Two low-priority items posted inline:

1. Panic hardening on the shared eventFields path (getTrail.go:443, also :424/:455) — sha1[0:7] is length-unguarded, and ["timestamp"].(float64) / ["type"].(string) are unchecked assertions. All pre-existing (lifted from eventRow), but the extraction now makes them reachable from the markdown renderer too, so this is a natural moment to harden. Fix this →

2. Empty attestation table for a zero-attestation artifact (getTrail.go:245-253) — cosmetic; an artifact with no attestations still emits a header + empty table when another artifact keeps the section's total > 0.

A couple of things to consider (no change requested)

• Markdown link injection. mdCell escapes &/</>/|/newlines (good — kills raw-HTML injection), but not [ ] ( ). A value containing [x](url) survives into a rendered link, and a ] inside a trail/attestation name could also break the surrounding [name](url) construct. It's the user's own data rendered in their own CI summary, and GitHub's step-summary renderer sanitizes javascript:/HTML, so the risk is low — but worth a mention since markdown is a brand-new output surface.
• Link insertion via strings.Replace in mdEventDescription is coupled to the exact server-side description wording ('…' for …, in '…'). It's well-tested today, but a future server-side phrasing change would silently drop the links rather than fail a test. Fine to leave; just noting the coupling.

These are all minor — nothing blocking. 👍

[claude]

Coverage gap: the new markdown branches aren't exercised. This fixture (and the shared golden) only covers a trail with no git_commit_info and a single trail_reported event. So three new code paths in printTrailAsMarkdown are never tested:

• the ### Git commit block (getTrail.go:101-116)
• the _No events._ empty-events path (getTrail.go:130-132)
• mdCell's |/newline escaping (no value in the fixture contains either character)

Since the integration golden mirrors what SetupTest produces, those branches won't be covered there either. Consider adding a second unit case with a git_commit_info block (including a message containing | and a newline) and one with events: [] to lock in the escaping and the empty path.

[claude]

Minor: for a missing/null field, mdCell renders the literal string <nil> (e.g. a trail with no description → | Description | <nil> |). The table renderer has the same behavior, so this is consistent — but markdown is a fresh, user-facing surface (a rendered CI summary), and <nil> reads worse there than an empty cell. Consider normalizing nil to "":

func mdCell(v interface{}) string {
	if v == nil {
		return ""
	}
	s := fmt.Sprintf("%v", v)
	...
}

Low priority — flagging since this is new output, not a regression.

[claude]

Two unchecked operations on this shared path can panic on malformed/partial payloads:

• eventMap["timestamp"].(float64) (line 305) panics if timestamp is missing or not a number.
• sha1[0:7] (line 324) panics if the sha is shorter than 7 chars.

Both are pre-existing (lifted verbatim from eventRow), so this PR doesn't regress anything — but extracting eventFields is a natural moment to harden them, since the markdown path now depends on it too. Low priority; flagging since the code moved.

[meekrosoft]

Adding some screenshots:

[claude]

sha1[0:7] panics if the sha is shorter than 7 characters. It's guarded for type but not length. This is pre-existing (lifted from eventRow), but extracting eventFields makes it a shared path that the new markdown renderer also depends on, so it's worth hardening here:

Suggested change

	eventCommit = sha1[0:7]
	if sha1, ok := commitInfo["sha1"].(string); ok && len(sha1) >= 7 {
	eventCommit = sha1[0:7]
	}

Same class of issue a couple of lines up: eventMap["timestamp"].(float64) (line 424) and eventMap["type"].(string) (line 455) are unchecked type assertions that panic on a missing/partial payload. Low priority — flagging since the code moved and is now reachable from two renderers.

[claude]

Minor/cosmetic: the artifact header (**name** — status) and its table are emitted unconditionally for every artifact in artifacts_statuses. If one artifact has zero attestations but another has some (so the overall total > 0 guard passes), that artifact still renders a header followed by an empty table (just the | | | / | --- | --- | header rows, no data). Consider skipping the table — or the whole block — when atts is empty:

atts, _ := artifact["attestations_statuses"].([]interface{})
if len(atts) == 0 {
    continue
}
fmt.Fprintf(b, "\n**%s** — %s\n\n", mdCell(name), mdComplianceState(artifact["status"]))
writeAttestationTable(b, atts, trailURL)

Low priority — depends on whether a zero-attestation artifact can appear alongside non-empty ones in practice.