Skip to content

[design] "Which repo does this agent work on" is stored twice — the field you can edit is read by nothing, and for a single-repo agent there is no supported way to fix a wrong path #411

Closed

Description

@serge-ivo

The control you can edit is not the value the agent reads

Reported as "I updated it, but it is still using the old one." Both halves are true, and the reason
is that "which repo does this agent work on" is stored in two places, only one of which is
editable, and the editable one is read by nothing.

Measured on Chess coder (26f71cd8, surfaceOptions.coding.repos: "single")

GET /v1/instances/26f71cd8…/settings
  settings.repo  =  ~/dev/stores/pas/platform/apps/chess-academy     ← the owner edited THIS
  field "repo"   =  { label: "Repository", type: "text",
                      description: "The repo this agent owns — a local path (~/dev/my-repo)…" }

GET /v1/instances/26f71cd8…/coding/repos
  workdir        =  ~/dev/pas/platform/apps/chess-academy             ← every tool reads THIS
  cloneStatus    =  ready
  updatedAt      =  2026-08-07 23:23:15                               ← untouched by the edit

grep across workers/api/src finds no reader of settings.repo at all. It is a
settingsSchema field seeded on the coder-repo agent (migrations/0063_seed_coder2_agents.sql),
stored faithfully, and consumed by nothing. The owner changed the one thing the console offers, the
platform saved it, and the agent kept reading a different column.

Both values happen to point at empty directories; the real checkout is
~/dev/stores/pas/apps/chess-academy (21,039 files). That is incidental — the bug is that no edit
to the visible field could ever have worked.

Why there is no way out from the UI

  • RepoSettingsModal.tsx:86 shows Folder through Detail — a read-only div, styled with the same
    border/radius/font-mono language as every input in the app (bg-paper vs bg-panel, i.e.
    #0a0a0a vs #141414 on a dark-only theme).
  • ReposList.tsx:192 hides the add-repo input entirely when singleRepo.
  • The empty state (:216) sends the user to "Settings → Agent settings" — the field that does
    nothing.

So for a single-repo agent the repo path is: not editable in repo settings, not re-addable in the
repo list, and editable only through a setting no code reads. There is no supported way to correct
it.
The only working route is POST /coding/repos by hand.

The design

1. One source of truth, and it is the setting

For repos: "single" the settings field is the right home: declarative, subscriber-owned, already
rendered, already documented in the empty state, and consistent with how every other per-instance
choice works (#41's typed settings). Make it authoritative:

  • PUT /v1/instances/:id/settings with a changed repo reconciles the coding_repos row —
    updating workdir (local path) or the remote coordinates (owner/name), creating the row if
    absent, and never silently diverging again.
  • The row remains the internal representation. Nothing outside the reconciler needs to know.

For multi-repo agents the list stays the source of truth and the setting is absent — which is
already true, since only the coder-repo agent declares it.

2. Validate where the value is entered

Apply #405's check on the settings write, not only on POST /coding/repos: does the path exist, is
it non-empty, is it a git work tree — asked of the runner that will use it. A rejected value must
not be stored, and the reason must be shown next to the field. This is the moment the owner can
actually act on it.

3. Make the repo path editable in the repo settings modal too

PUT /coding/repos/:repoId currently accepts name | urls | mergePolicy only. Add workdir, with
the same validation. Two entry points are acceptable if they write the same place; that is the
whole lesson of this ticket.

4. Stop rendering read-only values as inputs

Detail is visually a text field. Give read-only rows their own treatment (no border, or an
explicit lock affordance) so a value that cannot be changed does not invite a change. This is
general — every Detail in the app has the problem — and belongs with the design-system primitives
that landed in #366.

5. Where this is going: a checkout is per MACHINE, not per repo

Recorded as direction, not proposed for this ticket. workdir is a property of (repo, machine),
not of a repo. This account has two connected machines; the same repository is at
~/dev/stores/pas/apps/chess-academy on one and may be anywhere or nowhere on the other. Today a
single workdir column means an agent bound to a second machine is silently wrong — the identical
failure this ticket describes, arriving invisibly. coding_sessions already carries runner_node;
coding_repos has no such dimension.

The end state: a repo row holds identity (name, provider, remote, merge policy, URLs,
instructions); a checkout row holds location per machine, validated on the machine it names, so
the platform can say "ready on Mac, missing on Mac-mini" — a sentence it cannot form today. Do this
when a second machine actually runs the same repo, not before.

Acceptance

  • Editing Repository in Agent settings changes what the agent reads, in one step.
  • A path that does not exist / is empty / is not a git repo is refused at the point of entry,
    with the reason beside the field.
  • The repo settings modal can also change the folder, writing the same place.
  • A single-repo agent whose repo is wrong can be corrected without deleting anything.
  • settings.repo and coding_repos.workdir can no longer disagree — with a test that asserts it.
  • Read-only detail rows are visually distinguishable from inputs.

Files: workers/api/src/routes/instances.ts (settings PUT), workers/api/src/lib/instance-settings.ts,
workers/api/src/routes/coding-repos.ts:146,500, workers/api/src/lib/coding-store.ts,
agents/coder/web/src/RepoSettingsModal.tsx:86,121, agents/coder/web/src/ReposList.tsx:192,216,
workers/api/migrations/0063_seed_coder2_agents.sql. Supersedes the framing of #410; extends
#405.

Activity

  1. added
    bugSomething isn't working
    backendBackend / Worker / API work
    on Aug 8, 2026
  2. serge-ivo commented on Aug 8, 2026

    @serge-ivo
    ContributorAuthor

    Self-review. Four findings, and one of them says the ticket is wrong.

    First, the question this came from: is the address immutable by decision?

    No. There is no recorded decision anywhere.

    • PUT /:instanceId/coding/repos/:repoId was introduced by 50a9746 — a refactor ("the Coder
      control plane, split where a reader has to stop", [standards][tracking] Split worst large files into testable modules #305). Its {name, urls, mergePolicy} payload
      is what the pre-split code happened to accept, not a considered list of mutable fields.
    • grep for any rationale — "immutable", "cannot change", "never change" near workdir — returns
      nothing in workers/api/src, agents/coder/web/src or docs/.
    • The agent's own welcome message says "Point me at a repo in the Coding tab" and its settings
      field is labelled "The repo this agent owns". Both promise configurability.

    It is an omission, not a decision. But asking the question surfaces the argument nobody made,
    and that argument is good — see (2).

    1. "Make the setting authoritative" is the wrong fix. Withdrawing it.

    settingsSchema is for typed, declarative, subscriber-set VALUES (target_language, photo_limit).
    Making one of them create and mutate rows in coding_repos — a table with foreign keys from
    coding_sessions and coding_timeline — puts relational work behind a text field and hides a
    substantial side effect inside a settings save. It inverts the dependency: the settings write path
    would need to know about cloning, runner probes, and live sessions.

    The smaller and more honest fix is to remove the concept, not wire it. settings.repo is
    declared on exactly one agent (migrations/0063_seed_coder2_agents.sql) and read by nothing.
    Delete it, and make the repo row's address editable — one control, one destination, one less
    concept. An acceptance criterion in the original body ("settings.repo and workdir can no longer
    disagree") evaporates rather than needing a test.

    The cost of deleting: the empty-state copy at ReposList.tsx:216 must change, and the add-repo form
    must be shown for a single-repo agent that has zero repos. Both are small and both are needed
    anyway.

    2. The ticket ignores live sessions — the real argument for immutability

    This is the omission that matters. A repo's address is not just a string:

    • coding_sessions and coding_timeline hang off repo_id. Past work recorded as "in this repo"
      silently re-points at a different directory.
    • An active session has an engine process whose cwd is the old path. Changing the row does not
      move the process — the platform would believe path B while the engine works in path A. That is a
      split brain, and it is exactly the class of defect this board has spent the week on.
    • Claude Code's persisted conversation is keyed to that session and would --resume a conversation
      about different code.

    So the right rule is not "editable" and not "immutable" — it is:

    Identity is immutable; address is not. Which repository this is stays fixed (that is what
    sessions hang off). Where its working copy lives is a fact about the world that changes, and must
    be changeable.

    …with the safety expressed as when, not whether:

    • refuse the change while a session is active on that repo, naming the session and offering to end
      it — or end-and-relaunch explicitly as part of the change, never silently;
    • write the change to the timeline, so earlier work is not retroactively re-attributed.

    3. Validation-on-write assumes the runner is online. It often is not.

    The ticket says a bad path "must not be stored". But the owner may be configuring from a phone, with
    the laptop closed — blocking the save then makes the product unusable for the case it is meant to
    serve. Correct behaviour: store it, mark cloneStatus: needs_attention, and validate on the next
    runner contact.
    That is also the missing half of #405, whose check only runs at add time and never
    re-examines an existing row — which is why Chess coder's row still reads ready today.

    4. The per-machine section overstates its evidence

    Two machines are connected, but nothing shows the SAME repository being worked from both. That makes
    per-machine checkouts a plausible direction, not a demonstrated need. The staging note was right;
    the wording ("this account has two connected machines" implying the need) was not.

    What the ticket should say

    1. Delete settings.repo; show the add form for a single-repo agent at zero repos.
    2. Accept workdir on PUT /coding/repos/:repoId; make the Folder row an input.
    3. Refuse (or explicitly end) an active session before the address changes; record it in the timeline.
    4. Store-then-validate, with needs_attention, and re-validate existing rows on runner contact.
    5. Read-only rows stop looking like inputs.
    6. Per-machine checkouts: recorded as direction, gated on a repo actually running from two machines.
  3. serge-ivo commented on Aug 8, 2026

    @serge-ivo
    ContributorAuthor

    Owner decision (2026-08-08): delete settings.repo.

    Not kept as a read-only mirror. The repo row is the single source of truth for a repo's address.

    What that entails

    • Migration removing the repo field from the coder-repo agent's settingsSchema
      (seeded in 0063_seed_coder2_agents.sql). It is declared on that one agent and nowhere else.
    • Stored values are orphaned, not migrated. Instances carry
      agent_instances.config.settings.repo today (Chess coder's reads
      ~/dev/stores/pas/platform/apps/chess-academy). Once the field is gone the console stops
      rendering it and nothing reads it — the value is inert. Sweep it or leave it, but do not
      "helpfully" copy it into coding_repos.workdir: these values are unvalidated and at least
      one of them is wrong
      , so a migration that adopts them would install a broken path as though
      someone had chosen it.
    • ReposList.tsx:216 — the empty state currently says "No repository set yet — add its
      path in Settings → Agent settings."
      That sentence points at the field being deleted. It
      becomes the add affordance itself.
    • ReposList.tsx:192 — {showAddRepo && !singleRepo && …} must show the add form for a
      single-repo agent when it has zero repos. Hiding it at one repo stays correct; hiding it
      at zero is what makes a deleted repo unrecoverable.

    Why not a read-only mirror

    A mirror is a second place the same fact is written down, which is the defect this whole ticket
    documents. It would also need its own sync, and a sync that can fail is a disagreement waiting to
    happen — exactly what produced "I updated it and it is still using the old one".

    Deleting removes a concept. Mirroring adds one.

  4. serge-ivo commented on Aug 8, 2026

    @serge-ivo
    ContributorAuthor

    Implementation notes — worked out against HEAD so nobody re-derives them

    #405 already built every primitive this needs. lib/coding-workdir.ts exports
    checkWorkdirVia(conn, workDir) → WorkdirVerdict, cloneStatusForVerdict, isWorkdirBroken, and
    routes/coding-repos.ts:105 has the private verifyLocalWorkdir(env, repo, conn, {onUnverified})
    that the create path and the list already share. The edit path should use the same one, so all three
    entry points give one verdict.

    API — PUT /:instanceId/coding/repos/:repoId (coding-repos.ts:500)

    1. Accept workdir?: string in the body; trim it. Reject "" with 400 — blanking the address of a
      repo every tool addresses by it is not an intent anyone has; deleting the repo is.
    2. updateRepo cannot currently write it. lib/coding-store.ts:174 takes
      {name, urls, mergePolicy} and its UPDATE has no workdir column. Either add it there
      (workdir = COALESCE(?n, workdir), matching the pattern updateRepoClone:164 already uses) or
      call updateRepoClone for that one field. Adding it to updateRepo keeps one owner-scoped write.
    3. 409 when a session is live and the path actually changes. getActiveSessionForRepo(env, instanceId, uid, repoId) — return the session id in the body so the console can offer "end it".
      Only when existing.workdir !== workdir: a no-op save must not be refused.
    4. Store, then verify. After the write, re-read the repo and run
      verifyLocalWorkdir(env, moved, conn, { onUnverified: null }), returning the verdict in the
      response. onUnverified: null is deliberate — an offline runner must not condemn a path nobody
      has looked at.

    Console

    • RepoSettingsModal.tsx:86 — Folder becomes an input, saved with the existing PUT. Render the
      returned verdict inline (missing / empty / not_a_git_repo / unverified); this is the moment
      the owner can act on it. On 409, say a session is running and offer to end it.
    • RepoSettingsModal.tsx:121 Detail — read-only rows currently use
      bg-paper border border-line rounded-lg p-2 + font-mono, which is the app's text-INPUT costume
      (bg-panel border border-line rounded-xl px-3 py-2). #0a0a0a vs #141414 on a dark-only theme is
      not a visible distinction. Give read-only rows their own treatment — this is what made the owner
      believe the folder was editable.
    • ReposList.tsx:192 — {showAddRepo && !singleRepo && …} must also show at zero repos for
      a single-repo agent. Hiding it at one repo stays right; hiding it at zero is what makes a deleted
      repo unrecoverable.
    • ReposList.tsx:216 — the empty state points at "Settings → Agent settings", the field being
      deleted. It becomes the add affordance.

    Migration

    Remove the repo field from the coder-repo agent's settingsSchema
    (0063_seed_coder2_agents.sql). Declared on that one agent, read by nothing. Do not adopt stored
    settings.repo values into workdir
    — they were never validated and at least one is wrong.

    Tests worth having

    • PUT changes workdir; a live session makes it 409; a no-op save does not.
    • A broken path is stored AND marked, not rejected.
    • updateRepo with only workdir still returns 404 for another user's repo (owner scoping).
  5. serge-ivo commented on Aug 8, 2026

    @serge-ivo
    ContributorAuthor

    Delivered — fc01c17

    Implemented to the comments, not the body: the self-review withdrawing "make the setting authoritative", and the owner decision of 2026-08-08 to delete settings.repo rather than mirror it.

    One correction to the ticket's central claim

    grep across workers/api/src finds no reader of settings.repo at all.

    This was stale at HEAD. routes/instances.ts carried attachSettingRepo (added deliberately by f5f521a, #157/#182), which creates a coding_repos row from settings.repo — but only on create, and only at zero repos. So the field worked exactly once and then diverged silently forever, which is precisely why it looked configurable.

    That strengthens the delete decision rather than weakening it. It also means removing the schema field while keeping the function would have made the next settings save adopt the orphaned, unvalidated value into workdir — the one thing the decision explicitly forbids. attachSettingRepo is deleted.

    What shipped

    The second home is gone. Migration 0102_coder_repo_drop_repo_setting.sql rebuilds $.settingsSchema minus repo, scoped to coder-repo, idempotent, with no coding_repos write. Verified against real SQLite loaded from 0063's own seed: field gone, order and all other fields intact, coder-lead untouched, re-run a no-op. Stored settings.repo values are left orphaned and inert, not adopted — as instructed, since they were never validated and at least one is wrong.

    The address became editable. PUT /coding/repos/:repoId accepts workdir, written through updateRepo's owner-scoped statement. "" → 400 (blanking is not an intent; deleting is). 409 when a session is live and the path actually changes, returning sessionId so the console can offer to end it — a no-op resave is not refused. Store-then-verify through #405's existing verifyLocalWorkdir/checkWorkdirVia, reused rather than reinvented, so all three entry points give one verdict. A broken path is stored and marked, never rejected — the phone-with-the-laptop-shut case from comment 1 §3.

    One deliberate deviation from the implementation notes. The notes specify onUnverified: null; I used "unknown". null leaves the old path's ready sitting on the new path — the exact false claim #405 removed, and it fails #410's criterion "cloneStatus reflects the new path immediately". unknown does not condemn the path either; it says nobody has looked. Reasoned in the code comment and the commit body. Flagging it explicitly in case you disagree — it is a one-line change back.

    The affordances the deleted field stood in for. Add form now shows for a single-repo agent at zero repos (hiding at one stays right; hiding at zero is what made a deleted repo unrecoverable). Both empty states stopped pointing at "Settings → Agent settings". Folder is a real input in RepoSettingsModal, with the server's verdict inline, keeping the sheet open on a warning. Detail no longer wears the input costume — no border, no bg-paper, dimmed value, padlock.

    One file outside the brief: CodingTab.tsx returns its solo branch at singleRepo && repos.length <= 1, so ReposList is never rendered for a single-repo agent — its empty state was unreachable and the solo view's own empty state was the sign pointing at the deleted field. Without that 4-line hunk, deleting settings.repo would have been a net regression in recoverability.

    Verified in production

    GET /v1/instances/bfc76603…/settings
      fields: engine · autonomy · merge_policy        ← `repo` is GONE (0102 applied)
    
    PUT /v1/instances/bfc76603…/coding/repos/repo_a452fe0f… {"workdir":""}
      → HTTP 400 {"error":"A folder is required. To remove this repo, delete it."}
    

    That 400 is the proof the parameter is now read: before fc01c17, workdir was not in the payload type, so the same request was a silent 200 no-op. No production data was mutated by the check.

    pnpm typecheck clean · pnpm vitest run 6354 passed · biome clean · design tokens clean · migration hygiene OK · file-size ratchet OK (not raised) · control-shapes held at 48 (not raised — the new Add button was resolved by extracting a shared AddRepoForm used by both surfaces, which is also the better design) · Playwright Chromium + WebKit, including the mobile 320px/390px cases and console.spec.ts:3032 "a repo whose path is unusable says so (#405)", which exercises the changed component · CI green · Deploy API + Host success.

    Not verified / deliberately left

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    backendBackend / Worker / API workbugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions