Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions context/knowledge/gotchas/misc.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@
- **SGR sequences must strip to empty; cursor movement must strip to space.** Raw PTY logs contain cursor movement codes (`\x1b[5C`, `\x1b[1B`) that position text on different screen locations. Stripping these to empty merges unrelated text into URLs. `stripANSI` uses `ReplaceAllStringFunc` to distinguish: CSI ending in `m` → empty (preserves mid-URL colors), all else → space (prevents merging).
- **`bareLinkRe` must exclude `"`, backtick, `{`, `}`, `<`.** These are never valid in URLs per RFC 3986. Without exclusion, the regex matches through quoted/JSON URL data producing garbage entries in the link picker.
- **`hasValidHost` filters in-progress URLs (`https://gi`) by requiring a host with a real TLD.** The regex matches greedily as the agent streams output, so partial typing like `https://gi` would otherwise leak into the picker. Acceptance rule: IP literal, the literal `localhost`, or a DNS name whose final label is ≥2 chars starting with a letter (covers `.co`, `.museum`, and `xn--*` punycode). Don't tighten further with a curated IANA list — keeping the check syntactic avoids shipping (and updating) a 1500-entry blob.
- **`links.IsPR` is the single source of truth for the PR indicator; `Extract` stamps `Link.IsPR` so clients never re-derive it.** Both TUI pickers read `link.IsPR` and the web client reads the JSON `isPR` field — the JS must NOT pattern-match URLs itself, or TUI and web could disagree. `IsPR` matches on the path segment (`/pull/<n>` or `/merge_requests/<n>`), not the host, so it also catches GitHub Enterprise / self-hosted GitLab.
- **Both link pickers reserve a fixed `prLinkGutter` (2-col) left margin on every item row, and the modal-width calc must add it.** The PR glyph is painted into the gutter only for PR rows, but the gutter is reserved for all rows so PR and non-PR links stay column-aligned. If you compute `modalW` from the link display width without adding `prLinkGutter`, the widest PR link truncates a char early.

## Settings UI

Expand Down Expand Up @@ -217,4 +219,5 @@
- **`openspec archive <name> --yes` ABORTS but still exits 0 on a delta-vs-base mismatch.** It prints `… MODIFIED failed for header "…" - not found` / `… ADDED … - already exists` then `Aborted. No files were changed.` and returns rc=0 — a scripted `for` loop checking `$?` will silently skip the change. Detect failure by re-checking whether the folder actually moved (`test -d openspec/changes/<name>`) or grepping the output for `Aborted`, not by exit code. The abort is atomic (nothing partial), so just fix the delta and re-run.
- **Archive a backlog of un-archived changes in AUTHORING/merge order, not alphabetically.** `MODIFIED` replaces the whole requirement block and `REMOVED` deletes it, so the last-applied delta wins and every `REMOVED`/`MODIFIED` needs its target already present. Out-of-order folding either errors (target missing) or silently leaves the wrong generation in the base spec. Derive order from when each change first landed: `git log --diff-filter=A --format=%cI -- openspec/changes/<name>/proposal.md | tail -1`, sorted ascending.
- **Folding a backlog can yield an internally-contradictory base spec — audit for orphaned superseded requirements after archiving.** When a later change supersedes an earlier one by `MODIFY`-ing a *different* requirement (e.g. re-defining "Rail keybindings") instead of writing a `REMOVE` for the now-stale standalone requirement the earlier change `ADDED`, both survive the fold and contradict each other. `openspec validate --strict` does NOT catch this (it checks structure, not cross-requirement consistency). After folding, diff the base spec against current code behavior and delete the orphaned requirements by hand (this is what reconciled the stale `R`/`C`/`Ctrl+R`/`Ctrl+D`-bridging hera-view requirements vs the EOL-redesign nuke model).
- **A requirement's statement is parsed as the text up to the first blank line, and `validate` requires SHALL/MUST on that *first physical line*.** If you hard-wrap the statement so "SHALL" lands on line 2 (e.g. a lead-in clause ending in an em-dash on line 1), `openspec validate` fails with `ADDED "<name>" must contain SHALL or MUST` even though SHALL is present in the paragraph. Keep the SHALL on the first line — don't wrap before it.
- **Authoring drift to expect when folding old deltas:** a `MODIFIED` whose target was renamed/retired by later-merged work → drop or rewrite it; a `MODIFIED` for a requirement that is actually new (absent from base) → it's really an `ADDED`; an `ADDED` whose requirement a later change already put in base → drop the duplicate (keep base's newer version). `openspec archive` auto-creates new-capability specs with a `## Purpose\nTBD - created by archiving change <name>. Update Purpose after archive.` stub — fill it in the same PR.
2 changes: 1 addition & 1 deletion context/knowledge/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Non-obvious invariants and gotchas, split by topic. Read the relevant file when
| [gotchas/keybindings.md](gotchas/keybindings.md) | Key routing, ctrl sequences, tcell modifier quirks, agent view nav, plugin-view full surrender + double-Ctrl+Q failsafe, plugin help overlay, keyenc single-source + mod-7 Cmd+arrow round-trip, control-frame read-pump threading, plugin resize-envelope reconciliation, ctrl+r Claude session switcher, ctrl+k task switcher (grouped folder mode), plugin-view reconnect (backoff, laidOut survives resume) | 35 |
| [gotchas/tasklist-ui.md](gotchas/tasklist-ui.md) | Task list cursor, modals, focus guards, filter, archive, pinned, spinner, row rendering, form done-flag reset, needs-input icon, agent attention bar, sticky needs-input across idle gate, ❯+NBSP prompt anchor, decoration-line skip, PR indicator cell, filter-picker narrow-terminal width clamp (shared taskswitcher/sessionpicker/fuzzylinkpicker), shared TaskStatusIcon classifier (task list + switcher) | 53 |
| [gotchas/web-remote.md](gotchas/web-remote.md) | SPA + REST API + service worker, Tailscale-only binding, EventSource auth, xterm.js, HTML escaping, virtual keys, detail/files views, prompt modal, input history, link picker, new-task defaults, file uploads, offline view, Web Push, per-device tokens, settings endpoints, share target + iOS Shortcut, test harness, idle status, daemon-side status flip on exit, static-output replay, mobile compose + key bar, skill autocomplete, task-list search, diff wrap + stacked panels, full-history scrollback, global hotkeys, session artifacts (manifest-scoped serving, X-Frame-Options, blob+sandbox isolation), PR badge from cached pr_state, compose split text→CR send, artifact Open overlay vs paneled, Hera tab (read-only /api/hera roster), Settings System panel (sysmetrics cached snapshot, visible-only poller, *_avail "—" fallback, collector started in ListenAndServe) | 191 |
| [gotchas/misc.md](gotchas/misc.md) | DB patterns, Go idioms, Codex, Pi/ollama prelaunch, MCP, PRs, file explorer, quick-add, scheduled tasks, links, task auto-naming, settings two-pane UI, plugin-scoped token tagging, task_meta sidecar, plugin settings registry, Claude session discovery (folder encoding, JSONL parse), config.toml override layer (precedence, partial-map zeroing, mtime live-reload), hera plugin contract (POST/DELETE /notify), model selection (--model injection, codex-resume re-append), per-schedule model override (TaskCreator taskModel → HeadlessInput.Model), wall-clock duration clamp, openspec archive workflow (rc0-on-abort, authoring-order fold, orphaned-superseded-requirement audit, authoring-drift fixes) | 148 |
| [gotchas/misc.md](gotchas/misc.md) | DB patterns, Go idioms, Codex, Pi/ollama prelaunch, MCP, PRs, file explorer, quick-add, scheduled tasks, links, task auto-naming, settings two-pane UI, plugin-scoped token tagging, task_meta sidecar, plugin settings registry, Claude session discovery (folder encoding, JSONL parse), config.toml override layer (precedence, partial-map zeroing, mtime live-reload), hera plugin contract (POST/DELETE /notify), model selection (--model injection, codex-resume re-append), per-schedule model override (TaskCreator taskModel → HeadlessInput.Model), wall-clock duration clamp, link PR indicator (IsPR single-source + prLinkGutter), openspec archive workflow (rc0-on-abort, authoring-order fold, orphaned-superseded-requirement audit, authoring-drift fixes) | 150 |
| [gotchas/orchestration.md](gotchas/orchestration.md) | depends_on DAG / depswatcher / link-unlink-halt / plan_slug all RETIRED (Hera is the single model; base_branch kept; tasks start immediately), task_set_result opaque-to-daemon, ARGUS_TASK_ID env export, (name,project) idempotency, schema column ordering, hera schema/store M1 (FK-cascade, NULL partial-index, per-(task,orchestrator) multi-binding + ErrHeraAmbiguous, transactional role+binding), hera M4 (born-bound spawn + AfterPersist hook; auto-adopt removed; ReconcileBindings keyed on task-row), hera M5 (subtree TLDR roll-up: SubtreeOrchIDs BFS + cycle guard, tree_read_cursors cascade, advisory per-role cursor), hera plan-DAG substrate (planned node = role w/ no binding, gate on role-status done, failed-blocker → HOLD + ping coordinator, in-tx DFS cycle check, check-in pulled via inbox) | 41 |
| [gotchas/dag-rendering.md](gotchas/dag-rendering.md) | dagview is now a LAYOUT LIBRARY (dagview.Compute longest-path stage placement) consumed by internal/tui/planview — standalone widget no longer mounted; PLAN-DAG widget (planview) replaced the retired orchestration-tree graph, heraTreeNodes/tree.go DELETED. Sugiyama-lite layer math, rune-vs-byte truncation, single-line edges, branch-change log-only. planview node State/colour from RoleView.TaskStatus/TaskResult ({"failed":true}→red ✕); planview surfaces OnEnter/OnDrillIn/OnDrillOut/OnBranchChange (read-only nav, no edit callbacks); branchShape folds cursor+fanned-group+orch-title | 18 |
| [gotchas/hera-view.md](gotchas/hera-view.md) | Native Hera view (`internal/tui/hera`): 2nd tab always native (cfg.Hera.Enabled gates only daemon MCP tools, not tick refresh); structural multi-binding fan-out; freelance section; ready_to_close from task_meta; goroutine-free debounced Refresher on UI thread; rail nav j/k/Up/Down + Space (tab nav 1/2/3 only); no Sync + full-rect coverage. Panes fed from in-process runner ring (poll, not SSE); coord-vs-agent session rule; multi-binding disambiguated by role.OrchID; PTY align via ForceResyncPTY + off-thread SyncPanes. Thin mutation layer (ops.go over M1; shared agent.SpawnHeraWorker); rail keyset via OnXxx callbacks; multi-binding isolation; s/S step hera ROLE status; modal.ConfirmModal + NewInputForm; remote=nil ⇒ inert. Details 2 modes (worker→terminal, coordinator→stacked roster-over-PLAN — same geometry as roster-over-tree); embedded PLAN-DAG graph via heraPlanNodesWithBridge(orch, bridgeIndex) — coordinators not plan nodes, Drillable needs WithBridge form; planview↔hera import one-way (hera imports planview); OnDrillIn page-owned (drillIntoChild→bridgeIndex→PushOrch), OnEnter App-owned; handleDetailsKey Esc-at-root escapes pane; node colour from TaskStatus/TaskResult. Ctrl+Z→fullscreen (closes SIGTSTP footgun); Enter revives dead (startSession) or suspended worker (reviveHeraWorker→KickRerender, idle+not-blocked gated; live coordinator navigate-only); jumpToLeaf expands ancestor coordinators (EnsureAncestorsExpanded via canonicalParents + OrchIDsForTask) before SelectByTaskID so a folded coord doesn't swallow the join (BUG-007) | 39 |
Expand Down
4 changes: 4 additions & 0 deletions internal/api/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -734,12 +734,16 @@ func TestHandleGetLinks(t *testing.T) {
Links []struct {
Label string `json:"label"`
URL string `json:"url"`
IsPR bool `json:"isPR"`
} `json:"links"`
}
testutil.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp))
testutil.Equal(t, len(resp.Links), 2)
testutil.Equal(t, resp.Links[0].URL, "https://example.com/page")
testutil.Equal(t, resp.Links[0].IsPR, false)
testutil.Equal(t, resp.Links[1].URL, "https://github.com/org/repo/pull/42")
// PR URLs are flagged so the web client can mark them without re-deriving.
testutil.Equal(t, resp.Links[1].IsPR, true)
})
}

Expand Down
29 changes: 26 additions & 3 deletions internal/api/static/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -851,13 +851,23 @@
}
.links-empty { padding: 12px; color: var(--text-dim); font-size: 13px; text-align: center; }
.links-item {
display: flex; flex-direction: column; gap: 2px;
display: flex; flex-direction: row; align-items: center; gap: 8px;
width: 100%; padding: 8px 10px; min-height: 44px;
background: transparent; border: 0; border-radius: 0;
color: var(--text); text-align: left; cursor: pointer;
font: inherit;
}
.links-item:hover, .links-item:focus { background: var(--surface); outline: none; }
/* Label/URL stack. min-width:0 lets the children's ellipsis kick in inside
the flex row instead of overflowing it. */
.links-item .links-content { display: flex; flex-direction: column; gap: 2px; flex: 1; min-width: 0; }
/* PR indicator pill. Purple matches the TUI's StylePRLink (ColorPRAwaiting)
and the task list's awaiting-review badge, so "this link is a PR" reads
consistently across surfaces. */
.links-item .links-pr-badge {
flex: none; font-size: 11px; font-weight: 600; padding: 2px 7px;
border-radius: 10px; background: #251b3a; color: #b294fa; white-space: nowrap;
}
.links-item .links-label {
font-size: 13px; line-height: 1.3;
overflow: hidden; text-overflow: ellipsis; white-space: nowrap;
Expand Down Expand Up @@ -3931,17 +3941,30 @@ <h3>Keyboard shortcuts</h3>
const url = l.url || '';
const label = l.label || url;
if (label === url) btn.classList.add('url-only');
// PR indicator. l.isPR is server-classified by the shared links package
// (single source of truth), so the client never re-derives it. Built via
// the DOM API — never innerHTML — like everything else in this row.
if (l.isPR) {
const badge = document.createElement('span');
badge.className = 'links-pr-badge';
badge.textContent = 'PR';
badge.title = 'Pull request';
btn.appendChild(badge);
}
// textContent (not innerHTML) is required — labels and URLs are
// attacker-controlled (they came out of the agent's stdout). textContent
// escapes HTML, so a label of `<img src=x onerror=...>` renders as text.
const content = document.createElement('div');
content.className = 'links-content';
const labelEl = document.createElement('span');
labelEl.className = 'links-label';
labelEl.textContent = label;
const urlEl = document.createElement('span');
urlEl.className = 'links-url';
urlEl.textContent = url;
btn.appendChild(labelEl);
btn.appendChild(urlEl);
content.appendChild(labelEl);
content.appendChild(urlEl);
btn.appendChild(content);
btn.addEventListener('click', () => {
closeLinksModal();
openExternalURL(url);
Expand Down
2 changes: 1 addition & 1 deletion internal/api/static/sw.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//
// Increment SW_VERSION on any shell change to bust the cache.

const SW_VERSION = 'argus-shell-v62';
const SW_VERSION = 'argus-shell-v63';
const SHELL_ASSETS = [
'/',
'/manifest.webmanifest',
Expand Down
24 changes: 22 additions & 2 deletions internal/links/links.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,26 @@ import (
type Link struct {
Label string `json:"label"` // markdown link text, or the URL itself for bare URLs
URL string `json:"url"`
IsPR bool `json:"isPR"` // URL points at a GitHub PR / GitLab merge request
}

// prPathRe matches the path of a pull-request / merge-request URL on a
// code-forge host: GitHub's "/<owner>/<repo>/pull/<n>" or GitLab's
// "/<group>/<repo>/-/merge_requests/<n>". Matching on the path segment (rather
// than the host) means it also recognizes GitHub Enterprise and self-hosted
// GitLab instances. The trailing "(?:/|$)" keeps "/pull/123" and
// "/pull/123/files" matching while a stray "/pull/abc" does not.
var prPathRe = regexp.MustCompile(`/(?:pull|merge_requests)/\d+(?:/|$)`)

// IsPR reports whether the URL points at a pull request or merge request. It is
// the single source of truth for the PR indicator shown by the TUI link pickers
// and the web client's Open Link modal.
func IsPR(raw string) bool {
u, err := url.Parse(raw)
if err != nil || u.Host == "" {
return false
}
return prPathRe.MatchString(u.Path)
}

// mdLinkRe matches markdown links: [text](url)
Expand Down Expand Up @@ -160,7 +180,7 @@ func Extract(content string) []Link {
continue
}
seen[u] = true
out = append(out, Link{Label: m[1], URL: u})
out = append(out, Link{Label: m[1], URL: u, IsPR: IsPR(u)})
}

// Second pass: bare URLs not already captured
Expand All @@ -173,7 +193,7 @@ func Extract(content string) []Link {
continue
}
seen[u] = true
out = append(out, Link{Label: u, URL: u})
out = append(out, Link{Label: u, URL: u, IsPR: IsPR(u)})
}

return out
Expand Down
43 changes: 39 additions & 4 deletions internal/links/links_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestExtract(t *testing.T) {
{
name: "github PR URL",
content: "PR: https://github.com/org/repo/pull/123",
want: []Link{{Label: "https://github.com/org/repo/pull/123", URL: "https://github.com/org/repo/pull/123"}},
want: []Link{{Label: "https://github.com/org/repo/pull/123", URL: "https://github.com/org/repo/pull/123", IsPR: true}},
},
{
name: "URL with ANSI escape sequences",
Expand Down Expand Up @@ -105,7 +105,7 @@ func TestExtract(t *testing.T) {
name: "multiple OSC 8 hyperlinks",
content: "\x1b]8;;https://github.com/org/repo/pull/1\x1b\\PR #1\x1b]8;;\x1b\\ and \x1b]8;;https://circleci.com/gh/org/repo/99\x1b\\CI\x1b]8;;\x1b\\",
want: []Link{
{Label: "https://github.com/org/repo/pull/1", URL: "https://github.com/org/repo/pull/1"},
{Label: "https://github.com/org/repo/pull/1", URL: "https://github.com/org/repo/pull/1", IsPR: true},
{Label: "https://circleci.com/gh/org/repo/99", URL: "https://circleci.com/gh/org/repo/99"},
},
},
Expand All @@ -117,12 +117,12 @@ func TestExtract(t *testing.T) {
{
name: "cursor movement prevents text merging",
content: "https://github.com/org/repo/pull/123\x1b[5C\x1b[1Bpublished",
want: []Link{{Label: "https://github.com/org/repo/pull/123", URL: "https://github.com/org/repo/pull/123"}},
want: []Link{{Label: "https://github.com/org/repo/pull/123", URL: "https://github.com/org/repo/pull/123", IsPR: true}},
},
{
name: "quoted URL stops at double quote",
content: `"https://github.com/org/repo/pull/123",URL,"https://github.com/org/repo/pull/123")`,
want: []Link{{Label: "https://github.com/org/repo/pull/123", URL: "https://github.com/org/repo/pull/123"}},
want: []Link{{Label: "https://github.com/org/repo/pull/123", URL: "https://github.com/org/repo/pull/123", IsPR: true}},
},
{
name: "backtick-wrapped URL cleaned",
Expand Down Expand Up @@ -242,7 +242,42 @@ func TestExtract(t *testing.T) {
for i := range tt.want {
testutil.Equal(t, got[i].Label, tt.want[i].Label)
testutil.Equal(t, got[i].URL, tt.want[i].URL)
testutil.Equal(t, got[i].IsPR, tt.want[i].IsPR)
}
})
}
}

func TestIsPR(t *testing.T) {
tests := []struct {
name string
url string
want bool
}{
{"github pr", "https://github.com/org/repo/pull/123", true},
{"github pr with files suffix", "https://github.com/org/repo/pull/123/files", true},
{"github enterprise pr", "https://github.example.com/org/repo/pull/9", true},
{"gitlab merge request", "https://gitlab.com/group/repo/-/merge_requests/42", true},
{"github repo root not a pr", "https://github.com/org/repo", false},
{"github issue not a pr", "https://github.com/org/repo/issues/5", false},
{"github compare range not a pr", "https://github.com/org/repo/compare/v1.0...v1.1", false},
{"pull without number not a pr", "https://github.com/org/repo/pull/abc", false},
{"unrelated url", "https://example.com/page", false},
{"empty", "", false},
{"no host", "https:///pull/1", false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
testutil.Equal(t, IsPR(tt.url), tt.want)
})
}
}

func TestExtractStampsIsPR(t *testing.T) {
got := Extract("PR: https://github.com/org/repo/pull/7 and docs https://example.com/guide")
testutil.Equal(t, len(got), 2)
testutil.Equal(t, got[0].URL, "https://github.com/org/repo/pull/7")
testutil.Equal(t, got[0].IsPR, true)
testutil.Equal(t, got[1].URL, "https://example.com/guide")
testutil.Equal(t, got[1].IsPR, false)
}
Loading
Loading