GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix(cli): close coderd on server startup errors by ibetitsmike · Pull Request #30280 · coder/coder

Conversation

@ibetitsmike

@ibetitsmike ibetitsmike commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

coder server closes the coderd API only in its graceful shutdown sequence. Every startup error returned after newAPI (creating aibridged, registering Prometheus metrics, writing the config URL, creating provisioner daemons, notifying systemd) skipped that close and leaked the whole coderd instance: tailnet and wireguard-go, gvisor, metricscache, dbrollup, cryptokeys, chatd, and pubsub queues. TestServer/Logging hits this when test cleanup cancels startup while aibridged is being created. The test helper ignores the resulting context canceled error, so only the package-level goleak check in cli fails.

The fix closes the API in a deferred sync.OnceValue registered right after newAPI, and the shutdown sequence calls the same function. The guard is needed because the closer is not safe to call twice: the AGPL API returns "API already closed", and the enterprise closer closes each component again. The normal shutdown path is unchanged.

Testing

The new TestServer/StartupErrorNoLeak fails startup after coderd is created by putting a directory at the config URL path.

  • Red (test only): go test ./cli -run '^TestServer$/^StartupErrorNoLeak$' -count=1 exited 1 in 3 of 3 runs with goleak: Errors on successful test run: found unexpected goroutines (coderd frames) while the subtest passed.
  • Green (with the fix): the same command passed in 3 of 3 runs.
  • TestServer/Logging alone, 20 runs from one test binary: 20 of 20 passed with no goleak failure, including 3 runs that hit the create aibridged startup error.
  • go test ./cli -count=1 passed in 4 of 4 runs, and go vet ./cli is clean.

Found in the CI of #30272 (test-go-pg (ubuntu-latest)).

Xum, an AI coding agent, implemented, tested, and opened this PR on behalf of @ibetitsmike.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

chatgpt-codex-connector[bot] reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@ibetitsmike
fix(cli): close coderd on server startup errors
9b2476d
`coder server` closed the coderd API only in its shutdown sequence,
so every startup error returned after the API was created leaked the
whole instance (tailnet, chatd, metricscache, pubsub queues). In CI,
TestServer/Logging hits this when cleanup cancels startup while
aibridged is being created, and the cli package goleak check fails.

Close the API in a defer right after it is created and reuse the same
call in the shutdown sequence. A sync.OnceValue guards it because the
closer is not safe to call twice.
@github-actions github-actions Bot assigned ibetitsmike Oct 2, 2026
@ibetitsmike
ibetitsmike marked this pull request as ready for review October 2, 2026 12:34
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 2, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-02T12:37:09.642736Z 9b2476d Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pr-queue-manager
pr-queue-manager Bot requested a review from jscottmiller October 2, 2026 12:36
@ibetitsmike ibetitsmike mentioned this pull request Oct 2, 2026
feat: add organization-scoped Coder Agents instructions #30272
Merged
jscottmiller
jscottmiller approved these changes Oct 2, 2026
View reviewed changes
ibetitsmike added a commit that referenced this pull request Oct 6, 2026
@ibetitsmike
feat: add organization-scoped Coder Agents instructions (#30272)
7993c73
Adds organization-scoped Coder Agents instructions. Organization admins
set them in **Admin settings** > **AI** > **Coder Agents** >
**Organization settings**, or with `GET`/`PUT
/api/v2/organizations/{organization}/chats/config/system-prompt`.
Implements [CODAGT-984](https://linear.app/codercom/issue/CODAGT-984).

The instructions add to the deployment system prompt and never replace
it. When a chat is created, chatd stores the organization prompt as its
own system row right after the deployment row, only for chats in that
organization. Subagents resolve their parent organization's current
prompt when they spawn, and existing chats keep their stored rows.
Access reuses the organization `chat_model_config` permission: owners
and organization admins edit, auditors read, members get 404 on read and
403 on write. Writes reuse the deployment prompt's sanitization, size
limit (128 KiB by default), and audit behavior, with a new
`chat_organization_system_prompt` audit resource type. Migration
`000613` (the next number after `000612` on `main`) adds the
`chat_organization_system_prompts` table.

## Testing

- Go: enterprise authorization (including an update-only custom role),
validation, and audit tests; chatd row order and subagent tests.
- Frontend: Vitest interaction tests and stories for the card (loading,
load error, refetch error over the cached value, save error, read-only).
- Remote dogfood UAT, [evidence
chat](https://dogfood.cdr.dev/agents/b595a709-0709-44e6-9535-96a922974d61):
round 1 **PASS** on `b027cb5ca67` (permission matrix, validation, UI,
row order, cross-organization isolation, audit, subagents, regression).
Round 2 **PASS** on `618bff67c75` (loading and load-error states,
refetch error over the cached value with unsaved edits kept, update-only
custom role saves over the API, row order, the documented size limit,
rapid organization switching, double-click save).
- CI: rebased onto `main` (`98332c739b0`) as `3850fbaa922`; other commit
SHAs in this description are from before the rebase. Every check passed
on `3850fbaa922`, with `test-go-pg` green on the first attempt. After
the review fixes in item 7, `main` was merged in as `006c4951749` to
resolve new conflicts in generated files and renumber the migration to
`000613`. Every check passes on `006c4951749`. Before the rebase, every
check passed on `b4e76543df2`. `test-go-pg (ubuntu-latest)` passed on
its fourth attempt; the first three failed only the `cli` package
goroutine leak check, with every test passing. Two of those leaked a
coderd instance, which also happens intermittently on `main`:
`TestServer/Logging` cancels `coder server` during startup, and an early
return in `cli/server.go` skips closing coderd. That fix is in #30280.
The first attempt leaked an idle HTTP connection instead (not
investigated).

## Screenshots

From remote UAT on a test deployment (test data only).

| Organization admin saves instructions (`b027cb5ca67`) | Auditor sees
read-only instructions, no save actions (`b027cb5ca67`) |
|---|---|
| ![Organization admin saves organization instructions and sees a
success
toast](https://gist.githubusercontent.com/ibetitsmike/1027eba97c252d6f3e921d2f46debd01/raw/5a70872eae4170b4e9330c07696727071f0962e8/r1-admin-saved.png)
| ![Organization auditor sees the instructions
read-only](https://gist.githubusercontent.com/ibetitsmike/1027eba97c252d6f3e921d2f46debd01/raw/5a70872eae4170b4e9330c07696727071f0962e8/r1-auditor-readonly.png)
|
| **Loading state while the first fetch is pending (`618bff67c75`)** |
**Refetch error shown above the field; the unsaved edit is kept and Save
stays enabled (`618bff67c75`)** |
| ![Loading spinner with no textarea or
buttons](https://gist.githubusercontent.com/ibetitsmike/1027eba97c252d6f3e921d2f46debd01/raw/5a70872eae4170b4e9330c07696727071f0962e8/r2-loading.png)
| ![Error alert above the instructions field with the unsaved edit
preserved](https://gist.githubusercontent.com/ibetitsmike/1027eba97c252d6f3e921d2f46debd01/raw/5a70872eae4170b4e9330c07696727071f0962e8/r2-refetch-error-edit-kept.png)
|

## Review record

1. Codex,
[review](#30272 (review))
on `c42b5f9e05c`: 6 findings. Fixed the missing loading state, refetch
failures hiding cached instructions, and two docs style issues
(`58eab5603a4`, `cc73b714630`). Two were first declined in error, then
corrected: the read-only test now checks that typing leaves the value
unchanged (`cc73b714630`), and update-only custom roles no longer get a
500 on save (`618bff67c75`).
2. Independent security review by a clean-context agent,
[comment](#30272 (comment))
on `c42b5f9e05c`: no P0 to P2 findings. A stale P3 comment was fixed in
`34c17aadb7f`; returning `err.Error()` in 500 details was kept to match
the deployment prompt handlers.
3. Codex,
[review](#30272 (review))
on `58eab5603a4`: 2 findings (refetch errors not surfaced, docs line
breaks), both fixed in `cc73b714630`.
4. Codex, blockers only,
[review](#30272 (review))
on `618bff67c75`: 1 P2 (a failed refresh right after a successful save
shows the previous value). Not a blocker, deferred.
5. Final fresh advisory by a clean-context agent,
[comment](#30272 (comment))
on `618bff67c75`: **READY**, no blockers and nothing to cut. It
independently checked authorization, audit, the migration, chatd row
insertion and subagent inheritance, the tests, and soft-deleted
organization handling, and agreed with every deferral. No code changes
followed: the snapshot behavior it suggested documenting is already
documented, and its docs wording note joins the deferred docs item.
6. Codex,
[comment](#30272 (comment))
on `b4e76543df2` (the docs-only follow-up below): no major issues. Mike
asked for a passing Codex review loop after item 5, so this review goes
past the planned stop at the final advisory. No code changes followed.
7. ethanndickson,
[review](#30272 (review))
on `3850fbaa922`: approved with 4 comments. Fixed 3: the audit target is
now empty (`91e77d3fa8c`), the docs no longer re-explain audit logs
(`a0fb6891af5`), and the organization instructions stories went from 9
to 5 (`e836097f93f`). Not fixed: the organization prompt read fails open
on a database error, like the deployment prompt, and ethanndickson is
changing both after merge
([thread](#30272 (comment))).
UAT did not rerun for these changes.

Not counted as reviews: the pre-PR frontend self-review, simplify pass,
and test audit (`de8377e94e1`), and a second test audit of
`de8377e94e1..cc73b71` (pass with nits; the update-only test case
was added after it).

Docs-only follow-up after the advisory, requested by Mike: `b4e76543df2`
adds audit log guidance for prompt changes to the platform controls page
and an update-only custom role note to the organizations page. Codex
reviewed it (item 6). No UAT covers it, and the code is unchanged since
`618bff67c75`. The deferred claim that the Instructions page path is
outdated was wrong, so it's dropped. Item 7 removed the audit log
guidance again; the custom role note stays.

## Soft-deleted organizations

UAT found that a soft-deleted organization's prompt stays readable and
writable by UUID. This is inherited behavior, not new exposure:
`ExtractOrganizationParam` resolves UUIDs without a deleted filter, so
the sibling organization chat routes (MCP servers, model overrides,
models) behave the same. The same `chat_model_config` RBAC still
applies, nobody gains access to another organization, and writes are
audited.

## Deferred (owner: Mike)

- A failed refresh right after a successful save shows the previous
value with an error alert, so a further edit from that view starts from
the old text (Codex round 4, UAT round 2). Trigger: reports of stale
values after saving on unreliable networks.
- Update-only custom roles can save over the API, but the card stays
hidden without read access. Trigger: a customer uses such a role.
- Soft-deleted organization access by UUID, above. Trigger: the platform
starts rejecting deleted organizations in `ExtractOrganizationParam` or
purging their configuration.
- A contrived "code word" instruction was refused as possible prompt
injection; neutral instructions work. Trigger: reports of ignored
organization instructions.
- UX polish: there is no retry action after a failed first load, the
save toast can cover Cancel, there is no unsaved-changes guard, the
oversize error omits the limit, and whitespace-only text clears the
prompt without notice. Trigger: the next settings UX pass.
- Docs: the 413 response and organization name paths are undocumented.
Trigger: the next Coder Agents docs pass.
- Test audit nits: a redundant `waitFor` and an optional read-only plus
refetch story. Trigger: the next change to these tests.

> Xum, an AI coding agent, implemented, tested, and opened this PR on
behalf of @ibetitsmike.
@ibetitsmike
ibetitsmike merged commit 7c39819 into main Oct 7, 2026
77 of 81 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@ibetitsmike
ibetitsmike deleted the mike/cli-server-startup-leak branch October 7, 2026 11:39
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@jscottmiller jscottmiller jscottmiller approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@ibetitsmike ibetitsmike

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@ibetitsmike @jscottmiller
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.