Skip to content

fix: Rename coder to sshd on Windows for VS Code Remote support - #974

Merged
kylecarbs merged 1 commit into
mainfrom
windowcode
Apr 12, 2022
Merged

fix: Rename coder to sshd on Windows for VS Code Remote support#974
kylecarbs merged 1 commit into
mainfrom
windowcode

Conversation

@kylecarbs

Copy link
Copy Markdown
Member

On Windows, VS Code Remote requires a parent process of the
executing shell to be named sshd, otherwise it fails. See:
microsoft/vscode-remote-release#5699

On Windows, VS Code Remote requires a parent process of the
executing shell to be named sshd, otherwise it fails. See:
microsoft/vscode-remote-release#5699
@kylecarbs
kylecarbs requested a review from code-asher April 12, 2022 02:07
@kylecarbs kylecarbs self-assigned this Apr 12, 2022
@codecov

codecov Bot commented Apr 12, 2022

Copy link
Copy Markdown

Codecov Report

Merging #974 (6a32af4) into main (e8b1a57) will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #974      +/-   ##
==========================================
+ Coverage   66.70%   66.76%   +0.06%     
==========================================
  Files         241      241              
  Lines       14577    14577              
  Branches      115      115              
==========================================
+ Hits         9724     9733       +9     
+ Misses       3867     3865       -2     
+ Partials      986      979       -7     
Flag Coverage Δ
unittest-go-macos-latest 53.61% <ø> (-0.17%) ⬇️
unittest-go-postgres- 66.42% <ø> (+0.11%) ⬆️
unittest-go-ubuntu-latest 56.26% <ø> (+0.03%) ⬆️
unittest-go-windows-2022 53.03% <ø> (+0.04%) ⬆️
unittest-js 61.56% <ø> (ø)
Impacted Files Coverage Δ
provisionersdk/agent.go 100.00% <ø> (ø)
pty/start_other.go 64.70% <0.00%> (-23.53%) ⬇️
peerbroker/dial.go 77.04% <0.00%> (-6.56%) ⬇️
peer/channel.go 83.23% <0.00%> (-1.74%) ⬇️
peer/conn.go 80.96% <0.00%> (-0.51%) ⬇️
coderd/provisionerdaemons.go 63.00% <0.00%> (-0.51%) ⬇️
coderd/database/queries.sql.go 83.67% <0.00%> (+0.21%) ⬆️
provisioner/terraform/provision.go 68.54% <0.00%> (+0.43%) ⬆️
provisionerd/provisionerd.go 81.17% <0.00%> (+1.17%) ⬆️
coderd/workspaceagents.go 61.77% <0.00%> (+1.77%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8b1a57...6a32af4. Read the comment docs.

@code-asher code-asher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the best jank

@kylecarbs

Copy link
Copy Markdown
Member Author

Waiting for us to be open-sourced and someone to ask why the heck we call ourselves sshd...

@kylecarbs
kylecarbs merged commit db9d5b7 into main Apr 12, 2022
@kylecarbs
kylecarbs deleted the windowcode branch April 12, 2022 02:14
nickvigilante added a commit that referenced this pull request Aug 11, 2026
#27246)

## Summary

Phase 2 of the H1 → front-matter migration (`DOCS-483`; parent
`DOCS-477`). Makes the two reference-doc generators emit per-page
metadata as YAML front matter instead of a leading `# H1`, so generated
pages are self-describing and `make gen` stops reverting migrated pages
(Phase 3).

Phase 1 (`DOCS-482`) made the coder.com renderers prefer a front-matter
`title` (manifest fallback).

> [!NOTE]
> Rebased onto `main` and fully regenerated, and updated across two
rounds of Coder Agents Review — see **Review follow-ups** below.

## Changes

- **`scripts/clidocgen/command.tpl` + `gen.go` + `main.go`** — front
matter now carries `title` (from `fullName`) and `description` (from the
command's `Short`), and the leading `# H1` is dropped. The CLI index
page's front matter is taken from the manifest `Command Line` route
(title/description/icon_path).
- **`scripts/apidocgen/postprocess/main.go`** — reads the manifest and,
at write time, injects front matter carrying each section's `title` plus
any curated `description`, `state`, and `icon_path`. The API index
page's front matter is taken from the manifest `REST API` route.
- **`scripts/docgenenv`** (new shared code) — one `YAMLScalar`
front-matter escaper, one `Route`/`Manifest` schema +
`LoadManifest`/`FindRoute`, and one `FrontMatter(Route)` emitter, all
imported by both generators (no duplicated helpers, types, or emitters).
- Regenerated all **166 CLI + 31 API** reference pages.

### Metadata → front matter, and what stays in the manifest

Every *per-page* manifest field is mirrored into the page's front
matter: `title`, `description`, `state`, `icon_path`. The **structural**
fields stay in `manifest.json`:

- `children` — the nav tree (explicitly out of scope).
- `path` — the manifest's pointer to the file; a page carrying its own
path is redundant/error-prone, so it's treated like `children`.

The fields are **duplicated** into front matter and **`manifest.json` is
left unchanged**, so this is a **no-op for rendering today** (coder.com
strips front matter for `llms`, and Algolia + the renderer read only
`title`). Removing the fields from the manifest is the natural
follow-up, gated on the renderer reading them from front matter first.

### Why the API side changes the postprocessor, not the `.dot` templates

The issue text suggested editing
`scripts/apidocgen/markdown-template/*`. I deliberately did **not**,
because the postprocessor derives each page's **filename, section title,
and manifest route** from the leading `# {name}` line
(`extractSectionName`). Emitting front matter from the template would
break that extraction. Instead the widdershins templates still emit `#
{name}`, the postprocessor reads it (and now verifies it), and then
swaps the heading for a front-matter block as each section is written.

## Review follow-ups (Coder Agents Review)

### Round 1 — addressed in `e53d5e03` (all threads resolved)

- **CRF-1 / CRF-4** — de-duplicated the escaper and the
`route`/`manifest` schema + traversal into `scripts/docgenenv` (shared
by both generators).
- **CRF-2** — `YAMLScalar` now quotes YAML-reserved scalars
(`true/false/null/…`, numbers); no current value is affected.
- **CRF-3** — added unit tests: a `YAMLScalar` round-trip, `FindRoute`,
and `prependFrontMatter`.
- **CRF-5** — the CLI and API **index** pages now mirror their manifest
route's title/description/icon_path instead of a hardcoded
`coder`/`API`, fixing a rendered-heading regression (`REST API`/`Command
Line` were being overwritten).
- **CRF-6** — dropped the dead `#login` anchor in
`docs/support/support-bundle.md` (the migrated `login.md` no longer
mints that heading anchor).
- **CRF-7 / CRF-8 / CRF-11** — renamed to `prependFrontMatter`, switched
to `bytes.Cut`, and it now strips the first line only when it is the `#
{name}` heading (`extractSectionName` errors otherwise).
- **CRF-9** — removed the orphan `docs/reference/api/chat.md` (not in
the manifest, not linked; the real page is `chats.md`).
- **CRF-10** — the metadata read and the manifest rewrite now share one
`FindRoute` traversal.
- **CRF-13** — moot under squash-merge; this branch is a single
scopeless commit.
- **CRF-15** — the pre-existing `sort.Slice`/`slices.IsSorted`
comparator is left as-is per the review (out of scope; safe today
because section names are unique).

### Round 2 — addressed in `ee796e7107` (all threads resolved)

- **CRF-16** (P1) — removed three em-dashes from new doc comments (the
only `make lint` failure on the prior head); the emdash gate is green.
- **CRF-17 / CRF-18** — unified front-matter emission into one shared
`docgenenv.FrontMatter(Route)`, used by the API postprocessor directly
and by `command.tpl` via a `frontMatter` template func. This retires the
hand-written template YAML and the
`indexTitle`/`indexDescription`/`indexIconPath` closures, so a new
front-matter field is wired in one place, and it gives the CLI index the
`state` arm it previously lacked. Verified byte-identical: a full CLI +
API regen produces zero page changes.
- **CRF-19** — CLI child sort switched to `slices.SortFunc` +
`cmp.Compare` (typed comparator).
- **CRF-20** — reworded the `prependFrontMatter` comment:
`extractSectionName`'s fail-fast is the load-bearing guard; the prefix
check is a defensive backstop.
- **CRF-21** — added `icon_path`/`state` coverage in `docgenenv`'s
`TestFrontMatter/AllFields` (the branch the index page relies on,
previously at 0%).
- **CRF-22** — `YAMLScalar` no longer emits a trailing-space value as a
bare scalar (YAML strips it on read, so it would not round-trip); added
test coverage.
- **CRF-24** — the shared emitter removed the duplicated `cliIndexRoute`
doc comment; the rationale now lives in one place.
- **CRF-23** (Phase 3, out of scope here) — noted: the API generator
wipes and regenerates `reference/api/` from the manifest, so removing
curated metadata from the manifest in Phase 3 needs another source first
(a generator that preserves existing front matter, or metadata carried
alongside the swagger annotations).
- **Process (Mafu-san)** — the verification set below now leads with
`make lint`, the mandatory CI gate that the earlier list omitted.

## Cross-repo dependency

**Resolved — this PR no longer has a hard merge-ordering gate** (CRF-14
was right; the earlier "must merge after #968" note was stale).

The coder.com surfaces that would otherwise leak raw front matter from
`coder/coder` `main` are already front-matter-aware on merged PRs:

- **coder.com#964** (`DOCS-554`, llms-full.txt corpus + Algolia) —
**merged**.
- **coder.com#974** (`DOCS-574`, the `.md` proxy twin + `llms.txt` index
titles) — **merged**.

coder.com#968 (`DOCS-577`) was re-scoped to only the renderer
route-metadata generalization; it's a no-op on today's corpus and its
own description confirms the "deploy before the generators" constraint
no longer applies (that was driven by the llms corpus, now in #964).
Worth a final confirmation that #964/#974 are **deployed** before merge,
but there's no branch/PR ordering blocker left.

## Verification & evidence

AI was the primary author of this PR (see disclosure below); per the [AI
Contribution
Guidelines](https://coder.com/docs/about/contributing/AI_CONTRIBUTING)
here is manual verification.

- `make lint` (golangci-lint + the emdash gate) passes; `go build` / `go
vet` / `go test` are clean for the generators + `scripts/docgenenv`;
`pnpm check-docs` passes.
- `swagger.json`, `docs.go`, and `manifest.json` are **unchanged** —
metadata is duplicated into front matter; command/section names and
routes did not move.
- The diff is purely additive front matter
(`title`/`description`/`state`/`icon_path`) + the leading H1 removal; no
body reflow. A full CLI + API regen produces **zero** page changes
beyond the two index pages.

<details>
<summary>Terminal evidence</summary>

CLI `description` from the command's `Short` (`YAMLScalar` quotes when
needed, e.g. a `Short` with a colon):

```md
---
title: server
description: Start a Coder server
---
```

API pages inherit curated manifest metadata (only Agents/Chats have any
today):

```md
---
title: Chats
description: "REST endpoints for Coder Agents Chats API (programmatic agent sessions)."
state:
  - early access
---
```

Diff scope + "no body changes" proof (uses an explicit `base..HEAD`
range, so it actually tests the claim):

```
$ git diff --shortstat origin/main
 210 files changed, 1447 insertions(+), 344 deletions(-)
# = 166 CLI + 31 API reference pages + generators + scripts/docgenenv
# swagger.json / docs.go / manifest.json: NOT modified

# Every removed line under docs/reference is a leading "# H1"; nothing else:
$ git diff origin/main..HEAD -- docs/reference/ | grep '^-' | grep -v '^---' | grep -v '^-# '
(empty)

$ pnpm check-docs
Summary: 0 error(s)
```

</details>

Linear: DOCS-483

> This PR was created with AI assistance (Coder Agents).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants