feat(jtk): add remotelinks subcommand for issue web links

[piekstra]

Closes #413

Summary

Adds a top-level jtk remotelinks command (aliases: remotelink, rl) to manage an issue's Jira Remote Links / Web Links — external URLs shown in the issue's links sidebar — via the Jira REST API (GET/POST/DELETE /rest/api/3/issue/{key}/remotelink).

Subcommands deliver the CRUD surface from the issue, following the existing issue-scoped command conventions (comments/links/attachments) and the jtk command-surface guardrails:

• remotelinks list <issue-key> — list remote links. Supports --id, --extended (adds RELATIONSHIP + SUMMARY columns), and --fields projection.
• remotelinks add <issue-key> --url <url> --title <title> [--summary --relationship] — attach a web link. Per the guardrails verb table, add = attach a child to a parent. --title defaults to the URL when omitted; --id emits only the new link ID; emits a get-style post-state detail block otherwise.
• remotelinks delete <issue-key> <link-id> — delete a remote link record from the issue. Emits a confirmation line.

Note on command placement

The issue requested jtk issue remotelink ..., but this CLI exposes resource commands at the top level (jtk links, jtk comments, jtk attachments) rather than under a jtk issue parent — there is no jtk issue command. The feature is therefore delivered as jtk remotelinks, matching repo convention while preserving the requested issue-scoped remote-link management and flags (--url, --title).

Architecture

Layered per ARCHITECTURE.md (domain → presenter → renderer; commands orchestrate):

• tools/jtk/api/remotelinks.go — Client.GetRemoteLinks / AddRemoteLink / DeleteRemoteLink + DTOs; new ErrRemoteLinkIDRequired / ErrRemoteLinkURLRequired / ErrRemoteLinkTitleRequired sentinels in api/errors.go.
• tools/jtk/internal/present/remotelink.go — pure RemoteLinkPresenter (list table, added-detail, deleted/empty messages) + a projection Registry.
• tools/jtk/internal/cmd/remotelinks/remotelinks.go — command wiring; registered in cmd/jtk/main.go.
• Docs: new remotelinks section in tools/jtk/internal/cmd/OUTPUT_SPEC.md and an example block in README.md.

Tests added at every layer: API client (httptest), presenter (exact presentation-model assertions), and command (list/add/delete incl. --id/--extended/--fields, title-defaults-to-URL, empty-state, non-numeric delete IDs).

Testing

All commands run from the repo root (GOFLAGS=-tags=keyring_no1password,keyring_nopassage, per the Makefile).

• go build -v -o bin/jtk ./tools/jtk/cmd/jtk — pass
• go test -race ./tools/jtk/... — pass
• cd tools/jtk && golangci-lint run — pass
• cd tools/jtk && go mod tidy + git diff --exit-code go.mod go.sum — clean

Remaining work / follow-ups

• --relationship and --summary are supported on add but optional; an update/edit-in-place path (re-POST with a matching globalId) was not added.
• No name/ID cache resolution is needed here (remote links are issue-scoped and addressed by their numeric ID from list).

[piekstra-dev]

Automated PR Review

Reviewed commit: a809723109ad
Profile: reviewer - Posting as: piekstra-dev

Summary

Reviewer	Findings
documentation:docs	0
go:implementation-tests	2
policies:conventions	0
structure:repo-health	1

go:implementation-tests (2 findings)

Minor - tools/jtk/api/remotelinks.go:78

DeleteRemoteLink accepts linkID string while RemoteLink.ID is int. The Jira remote-link ID is an integer domain type: AddRemoteLink returns *RemoteLink with ID int, and GetRemoteLinks also returns []RemoteLink with ID int. A caller performing a common list-then-delete flow must strconv.Itoa(l.ID) to bridge the mismatch, and an invalid non-numeric string (e.g. from a typo) passes the == "" guard, reaches Jira, and surfaces as a server-side 404 rather than a typed Go error.

Prefer linkID int in the signature to stay consistent with the domain type:

func (c *Client) DeleteRemoteLink(ctx context.Context, issueKey string, linkID int) error {
    if issueKey == "" {
        return ErrIssueKeyRequired
    }
    if linkID <= 0 {
        return ErrRemoteLinkIDRequired
    }
    urlStr := fmt.Sprintf("%s/issue/%s/remotelink/%d", c.BaseURL, url.PathEscape(issueKey), linkID)
    ...
}

The command layer (runRemove) receives the ID as args[1] (string) and can parse it with strconv.Atoi, returning a user-facing error for non-numeric input before reaching the API call. The API-layer test TestDeleteRemoteLink passes "10001" as a string today; update it to pass 10001 as an int and add a linkID <= 0 guard case.

Minor - tools/jtk/api/remotelinks.go:70

AddRemoteLink validates req.Object.URL but not req.Object.Title. Jira's remote-link schema marks object.title as required; submitting an empty title results in a Jira 400 rather than a typed sentinel error, making it harder for callers to handle programmatically.

The command layer always supplies a default title (the URL), so the gap only affects direct API-client callers. Add a sentinel and validate at the seam:

// in errors.go
ErrRemoteLinkTitleRequired = errors.New("remote link title is required")

// in AddRemoteLink, after the URL check
if req.Object.Title == "" {
    return nil, ErrRemoteLinkTitleRequired
}

Add a corresponding TestAddRemoteLink_EmptyTitle case alongside the existing TestAddRemoteLink_EmptyURL test.

structure:repo-health (1 finding)

Minor - tools/jtk/internal/present/remotelink.go:38

The extended branch in PresentList duplicates column selection logic that is already declared in RemoteLinkListSpec. The spec exists precisely so that callers don't need to re-enumerate columns; the method bypasses this by hard-coding two parallel headers/rows paths instead of driving from the registry. If a column is added or reordered in RemoteLinkListSpec, PresentList silently diverges. The other presenter types in this package (links, comments) drive their table rows from a single spec-driven path via projection.ApplyToTableInModel rather than forking on extended inside the presenter itself. Fix: remove the in-method fork and build a single row with all columns (ID, RELATIONSHIP, TITLE, URL, SUMMARY), then let the projection layer strip non-extended columns via the existing Extended: true registry flags, matching the pattern in the rest of the codebase.

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 4m 08s | ~$0.61 (est.) | claude-sonnet-4-6 | cr 0.4.161

Field	Value
Model	claude-sonnet-4-6
Reviewers	documentation:docs, go:implementation-tests, policies:conventions, structure:repo-health
Engine	claude_cli · claude-sonnet-4-6
Reviewed by	cr · piekstra-dev
Duration	4m 08s wall · 4m 06s compute
Cost	~$0.61 (est.)
Tokens	25 in / 11.4k out

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost	Duration
orchestrator-selection	claude-sonnet-4-6	4	1.8k	7.7k	12.6k	~$0.08 (est.)	29s
documentation:docs	claude-sonnet-4-6	4	1.5k	7.2k	12.6k	~$0.07 (est.)	30s
go:implementation-tests	claude-sonnet-4-6	5	5.8k	34.7k	34.0k	~$0.22 (est.)	1m 54s
policies:conventions	claude-sonnet-4-6	4	1.5k	7.7k	15.4k	~$0.08 (est.)	31s
structure:repo-health	claude-sonnet-4-6	4	555	7.2k	18.9k	~$0.08 (est.)	15s
orchestrator-rollup	claude-sonnet-4-6	4	432	20.7k	15.6k	~$0.07 (est.)	25s

[piekstra-dev]

Automated PR review completed with outcome: approved.

[piekstra-dev]

Automated PR Review

Reviewed commit: 2ef19efe088d
Profile: reviewer - Posting as: piekstra-dev

Summary

Reviewer	Findings
go:implementation-tests	1
documentation:docs	0
policies:conventions	0

go:implementation-tests (1 finding)

Minor - tools/jtk/internal/present/remotelink.go:107

PresentRemoved accepts linkID string even though the domain type for a remote link ID is int (as established by RemoteLink.ID int and DeleteRemoteLink(ctx, issueKey string, linkID int)). The caller in runRemove already holds the parsed int after strconv.Atoi, so passing it to the presenter forces a string → int → string round-trip:

// runRemove
linkID, err := strconv.Atoi(linkIDArg)   // linkID is int
// ...
PresentRemoved(strconv.Itoa(linkID), issueKey)  // needless re-stringify

This discards the typed value at the view boundary and makes the presenter's signature inconsistent with PresentAddedDetail, which receives *api.RemoteLink (carrying the int ID). If this pattern spreads to other remove-type presenters it erodes the convention of keeping typed values through the call chain.

Fix: change the signature to accept int and format inside the presenter:

func (RemoteLinkPresenter) PresentRemoved(linkID int, issueKey string) *present.OutputModel {
    // ...
    Message: fmt.Sprintf("Removed remote link %d from %s", linkID, issueKey),
}

The caller then passes linkID directly without strconv.Itoa.

3 PR discussion threads considered. 3 summarized; 3 resolved.

---

Completed in 3m 10s | ~$0.33 (est.) | claude-sonnet-4-6 | cr 0.4.161

Field	Value
Model	claude-sonnet-4-6
Reviewers	go:implementation-tests, documentation:docs, policies:conventions
Engine	claude_cli · claude-sonnet-4-6
Reviewed by	cr · piekstra-dev
Duration	3m 10s wall · 3m 03s compute
Cost	~$0.33 (est.)
Tokens	18 in / 5.5k out

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost	Duration
orchestrator-selection	claude-sonnet-4-6	3	124	3.3k	4.8k	~$0.02 (est.)	53s
go:implementation-tests	claude-sonnet-4-6	4	3.5k	7.2k	29.2k	~$0.16 (est.)	1m 08s
documentation:docs	claude-sonnet-4-6	4	1.3k	7.2k	12.6k	~$0.07 (est.)	27s
policies:conventions	claude-sonnet-4-6	3	124	3.3k	4.8k	~$0.02 (est.)	24s
orchestrator-rollup	claude-sonnet-4-6	4	372	7.2k	11.7k	~$0.05 (est.)	10s

[piekstra-dev]

Automated PR review completed with outcome: approved.

[rianjs]

• No blocking architecture/product issues found in the supplied metadata/diff. The command placement at top-level jtk remotelinks is justified by the existing surface, and the verb change to delete now matches the command guardrails.

• Minor: the implementation appears to add --relationship and --summary on add, plus --extended/--fields projection on list. Those are useful, but they go beyond the ticket’s requested list/add/delete-by-id surface. Since tests and output specs cover them, I would not block, but it slightly increases maintenance and docs surface.

• Minor: the testing described is strong for unit/httptest coverage, but there is no mention of verification against a real Jira remote-link object shape or sandbox tenant. Given Jira’s remote-link API has create/update semantics around globalId, a one-time manual or recorded integration check would reduce risk.

[rianjs]

• Major: The command tests never execute the actual Cobra surface (jtk remotelinks ...); they call helpers like newListCmd and runList directly. That means a broken Register() call, bad alias wiring (remotelink/rl), or missing main.go registration could still pass the suite.
• Major: I don’t see coverage that the old public delete verb (remove) is rejected or absent. Since this PR explicitly changes the user-facing contract to delete, a stale alias would be a real regression that these tests would miss.
• Major: The add-path tests only prove --url and --title round-trip. The optional --summary and --relationship behavior advertised in the PR is untested, so the implementation could silently drop those fields and still pass.
• Minor: Most CLI assertions are substring checks on stdout, not exact output/model checks. That leaves room for formatting, column-order, and --extended/--fields renderer regressions to slip through.

[monit-reviewer]

Automated PR Review

Reviewed commit: 5023be4

Approved with 2 non-blocking suggestions below. Address at your discretion.

Summary

Reviewer	Findings
documentation:docs-reviewer	1
harness-engineering:harness-knowledge-reviewer	1

documentation:docs-reviewer (1 findings)

💡 Suggestion - tools/jtk/internal/cmd/OUTPUT_SPEC.md:490

The --summary and --relationship flags for remotelinks add are implemented and referenced in the PR description but absent from OUTPUT_SPEC.md and README. Users have no way to discover these flags from the docs. The spec entry should be extended to document all optional flags with their behavior.

harness-engineering:harness-knowledge-reviewer (1 findings)

💡 Suggestion - tools/jtk/internal/cmd/OUTPUT_SPEC.md:463

The rationale for placing resource commands at the top level (e.g., jtk remotelinks rather than jtk issue remotelinks) is explained only in the PR description. If ARCHITECTURE.md doesn't already document the 'all resource commands are top-level' convention and the reasoning behind it, this decision will be invisible to future contributors facing the same structural choice.

1 info-level observations excluded. Run with --verbose to include.

9 PR discussion threads considered.

---

Completed in 3m 16s | $2.72 | sonnet | daemon 0.2.133 | Glorfindel

Field	Value
Model	sonnet
Reviewers	hybrid-synthesis, documentation:docs-reviewer, harness-engineering:harness-architecture-reviewer, harness-engineering:harness-enforcement-reviewer, harness-engineering:harness-knowledge-reviewer, harness-engineering:harness-self-documenting-code-reviewer, security:security-code-auditor
Engine	claude · sonnet
Reviewed by	pr-review-daemon · monit-pr-reviewer
Duration	3m 16s wall · 2m 47s compute (Reviewers: 2m 03s · Synthesis: 44s)
Cost	$2.72 (estimated)
Tokens	476.7k in / 23.8k out
Turns	14

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost
hybrid-synthesis	sonnet	40.9k	2.0k	13.1k	27.8k (1h)	$0.20
documentation:docs-reviewer	sonnet	39.0k	3.3k	13.1k	25.9k (1h)	$0.21
harness-engineering:harness-architecture-reviewer	sonnet	67.6k	1.1k	13.1k	54.5k (1h)	$0.35
harness-engineering:harness-enforcement-reviewer	sonnet	72.2k	5.3k	13.1k	59.1k (1h)	$0.44
harness-engineering:harness-knowledge-reviewer	sonnet	69.5k	2.7k	13.1k	56.3k (1h)	$0.38
harness-engineering:harness-self-documenting-code-reviewer	sonnet	72.2k	6.6k	13.1k	59.0k (1h)	$0.46
security:security-code-auditor	sonnet	66.3k	1.7k	13.1k	53.1k (1h)	$0.35
discussion-summarizer	—	49.0k	1.1k	18.5k	30.5k (1h)	$0.34

_{Re-reviews only run when @monit-reviewer is re-requested as a reviewer — push as many commits as you need, then re-request when ready. PRs targeting branches other than main, master are skipped, even when @monit-reviewer is re-requested.}

[monit-reviewer]

🔵 Low (documentation:docs-reviewer): The --summary and --relationship flags for remotelinks add are implemented and referenced in the PR description but absent from OUTPUT_SPEC.md and README. Users have no way to discover these flags from the docs. The spec entry should be extended to document all optional flags with their behavior.

Reply to this thread when addressed.

[monit-reviewer]

🔵 Low (harness-engineering:harness-knowledge-reviewer): The rationale for placing resource commands at the top level (e.g., jtk remotelinks rather than jtk issue remotelinks) is explained only in the PR description. If ARCHITECTURE.md doesn't already document the 'all resource commands are top-level' convention and the reasoning behind it, this decision will be invisible to future contributors facing the same structural choice.

Reply to this thread when addressed.