Repository navigation
Split agent instructions into a primer and topic guides - #244
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a86cf89af
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| ## How threads work | ||
|
|
||
| People comment on an exact passage of a plan, a table cell, or a diagram node or edge. Each comment starts a thread. A thread is `open` until someone resolves it. A person can reopen a resolved thread in the web UI. Resolved threads stay visible in the document with a dashed underline. |
There was a problem hiding this comment.
When an agent relies on this guide to explain the review UI, this sentence incorrectly promises that resolved threads remain visible with a dashed underline. The plan page starts with plan-layout--hide-resolved, and resolved discussions are only displayed after the viewer explicitly enables them, so the guide should distinguish that opt-in state from the default.
AGENTS.md reference: AGENTS.md:L111-L113
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d4cb755: the guide now says the document view hides resolved threads unless the reader chooses to show them.
| - Load `token` into `$TOKEN`. Use it only to mint your session token (step 1). | ||
| - If the file does not exist, ask your principal to create a token at **Settings → API Tokens** in CoPlan. Save it and the CoPlan URL to the file, then run `chmod 600` on it. | ||
| - If minting returns `401`, this token is not valid. Ask your principal for a new one, and update the file. | ||
| - Do not put the token itself in commands, files, or comments. Use the variable. |
There was a problem hiding this comment.
In the default bootstrap flow, the preceding instructions require saving the long-lived token in ~/.config/coplan/credentials.json, but this blanket rule then says never to put the token in any file. An agent cannot satisfy both requirements and may refuse to create or retain the credentials file; narrow this warning to command literals, comments, and files other than the protected credentials store.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d4cb755: the rule now says to keep the token only in the credentials file, and not in commands, other files, or comments.
| | State | Set by | Meaning | | ||
| |---|---|---| | ||
| | `watching` | you | You are attached and waiting. | | ||
| | `pending` | the server | An event arrived for you. If you do not change to `active` within 30 seconds, the pill shows that you did not respond. | |
There was a problem hiding this comment.
If an agent never transitions from pending to active, MarkStaleAgentSessionJob changes the session to stale, and AgentSession.visible excludes that state, so after 30 seconds the pill is removed rather than showing that the agent did not respond. This protocol description can make operators expect a persistent failure indicator that the UI never provides.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d4cb755: the table now says the session becomes stale after 30 seconds and the pill disappears.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4cb755780
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1,79 @@ | |||
| # API Reference | |||
|
|
|||
| Every endpoint, with the guide that explains it. All paths start with `<%= @base %>`. Every call except token minting needs `Authorization: Bearer $TOKEN` (see the primer at <<%= @base %>/agent-instructions>). Request bodies are JSON. | |||
There was a problem hiding this comment.
In the default configuration, token minting is not unauthenticated: TokensController skips require_api_token! but still runs authenticate_api!, and the primer's curl prefix sends the long-lived Bearer token. An agent following this sentence literally will omit authentication and receive 401 unless host request authentication is configured. Clarify that minting uses the long-lived/root credential (or configured host authentication), while subsequent calls use the minted session token.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3ab9ccf: the API guide now says minting uses the principal's long-lived token (or host request auth), and every other call uses the session token.
| ```json | ||
| { | ||
| "base_url": "<%= @base %>", | ||
| "token": "<long-lived token>", |
There was a problem hiding this comment.
Although the earlier credential wording was fixed, this new persistent-bridge example still directs operators to duplicate the long-lived root token in ~/.config/coplan/bridge.json, with no permission-hardening step. An ordinary editor-created file is commonly readable beyond its owner and would expose a credential capable of minting session tokens; load COPLAN_TOKEN from the protected credentials store, or explicitly require mode 0600, instead of embedding it in this config.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3ab9ccf: the bridge.json example no longer has a token. The guide says to pass COPLAN_TOKEN from the protected credentials file (mode 600) instead; the bridge already prefers the environment over the file.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ab9ccfb7f
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| ```bash | ||
| export COPLAN_BASE=<%= @base %> | ||
| export COPLAN_TOKEN=<your long-lived token from Settings → API Tokens> |
There was a problem hiding this comment.
When a principal follows this setup and substitutes the real long-lived token into the export command, the credential is recorded in shell history despite the primer requiring it to remain only in the mode-600 credentials file. Although the earlier bridge.json exposure was removed, this separate Amp command still exposes the root token; load COPLAN_TOKEN from ~/.config/coplan/credentials.json or another protected secret source instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1d882ba: both the Amp and bridge guides now load COPLAN_TOKEN with jq -r .token ~/.config/coplan/credentials.json, so the token never appears in a command.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d882ba984
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 1. It claims the session on each plan at start. | ||
| 2. When an event arrives, it sets the session to `active`, so people see a response within a second. | ||
| 3. It resumes the agent with the event and these steps: set `active`, reply, edit, then set `complete` or `awaiting_input`. | ||
| 4. It acknowledges the event after the agent's turn. If the turn fails, the event stays in the inbox. |
There was a problem hiding this comment.
When an adapter command starts but exits nonzero—for example because the harness session expired—system(*cmd) returns false rather than raising, so dispatch returns normally and the bridge immediately acknowledges the event at coplan-bridge:414-419. Contrary to this new guarantee, the failed event is therefore removed from the inbox and the user's comment is not retried; make a false/nil adapter result fail dispatch before acknowledging.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 69d072d: dispatch now raises when system returns false (nonzero exit) or nil (command could not start), so the loop's existing rescue leaves the event unacked for redelivery. Checked: system("false") → false/1, a missing command → nil/127.
| API paths use the plan `id`. If you have a plan URL, find the `id` in the list of plans you can see: | ||
|
|
||
| ```bash | ||
| <%= @curl %> \ | ||
| "<%= @base %>/api/v1/plans/$PLAN_ID/comments" | jq . | ||
| <%= @curl %> "<%= @base %>/api/v1/plans" | jq -r --arg url "$PLAN_URL" '.[] | select(.url == $url) | .id' |
There was a problem hiding this comment.
When an agent receives another person's draft URL, this lookup returns nothing: direct draft URLs are readable, but Plan.visible_to deliberately limits discovery to published plans and the caller's own drafts, as the following sentence acknowledges. Because canonical plan URLs contain a slug rather than the API ID, the agent then cannot call snapshot, comment, or edit endpoints for a shared draft; provide an authenticated URL-to-ID lookup or another way to obtain the ID.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed that this is a real API gap, but it is not a docs regression: the old instructions had no URL lookup at all. Adding a URL-to-id endpoint is an API change, so it is out of scope here. It is listed under Open questions in the PR description for the owner to decide. No change in this PR.
| export COPLAN_BASE=<%= @base %> | ||
| export COPLAN_TOKEN=$(jq -r .token ~/.config/coplan/credentials.json) |
There was a problem hiding this comment.
Although the prior storage exposure was fixed, this command still loads the principal's long-lived root credential directly into the bridge. Fresh evidence is that coplan-bridge assigns COPLAN_TOKEN directly to TOKEN and never calls POST /tokens, so every claim, poll, and inherited adapter process receives the non-expiring mint-capable secret; concurrent bridges using the same credential also share one inbox and presence identity. Mint a dedicated session token for the bridge instead of using the root credential for its API loop.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 69d072d: the bridge guide now mints a dedicated 30-day session token for the bridge (from the long-lived token in the credentials file) and exports that as COPLAN_TOKEN. It also says to run one bridge per token. The Amp guide now points to that install step instead of exporting the root token.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69d072dfd3
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| coplan-bridge --adapter claude --session <session-id> --plan $PLAN_ID | ||
| coplan-bridge --adapter amp --session T-<thread-id> --plan $PLAN_ID | ||
| coplan-bridge --adapter codex --session <session-id> --plan $PLAN_ID |
There was a problem hiding this comment.
On machines where ~/.coplan/bin is not already in PATH, the install block downloads the executable there but never updates PATH, so each bare coplan-bridge command fails with command not found. Use ~/.coplan/bin/coplan-bridge (as the live guide does for coplan-attach) or explicitly add the directory to PATH before these commands.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ee46d31: the install block now adds ~/.coplan/bin to PATH right after chmod, so the bare coplan-bridge commands below it work.
| - Copy `anchor_text` exactly from the plan. CoPlan highlights it for readers. Choose a short, unique phrase. If the phrase occurs more than once, add `anchor_occurrence` (1 for the first occurrence). | ||
| - Leave out `anchor_text` for a comment on the whole plan. | ||
| - `@username` in a comment notifies that person. Usernames that do not exist stay plain text. | ||
| - To delete one of your comments: `DELETE <%= @base %>/api/v1/plans/$PLAN_ID/comments/$COMMENT_ID/delete`. This uses the comment ID, not the thread ID. |
There was a problem hiding this comment.
When an agent follows this instruction for a comment it authored through the API, the comment has author_type: "local_agent", while CommentPolicy#delete? only permits comments whose author_type is human; the DELETE therefore returns 403. Narrow this to deleting the principal's own human comments, or add support for deleting comments authored by the calling agent token before advertising it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ee46d31: confirmed CommentPolicy#delete? only allows human comments. The comments guide now says an agent cannot delete its own comments and should reply to correct one; the API table says the endpoint is for the principal's own comments.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee46d316fc
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return if system(*cmd) | ||
|
|
||
| raise "#{adapter} adapter #{$?&.exitstatus ? "exited #{$?.exitstatus}" : "could not start"}" |
There was a problem hiding this comment.
When an adapter repeatedly exits nonzero or cannot start, the new raise leaves the event unacknowledged, but the next poll immediately redelivers it and dispatch first PATCHes the session to active again. Because this retry runs every three seconds, last_activity_at is continually refreshed and the five-minute stale window never removes the false “is reading your comment” pill, even though no agent turn is running. Transition the session out of active on dispatch failure before retrying.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 01340df: on dispatch failure the bridge now PATCHes the session to complete and backs off (3s, doubling, max 60s) before redelivery. Ran it against a local server with an adapter that exits 1: the event stayed unacked, retries went 3s → 6s → 12s, and the session ended at complete. With an adapter that exits 0, the event was acked.
| export COPLAN_TOKEN=$(curl -s -X POST \ | ||
| -H "Authorization: Bearer $(jq -r .token ~/.config/coplan/credentials.json)" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d '{"agent_name": "Bridge", "ttl_seconds": 2592000, "metadata": {"harness": "coplan-bridge"}}' \ |
There was a problem hiding this comment.
When an operator follows this setup for Claude, Amp, Codex, or ACP, the dedicated token is always minted with agent_name: "Bridge"; coplan-bridge and its resumed subprocess both use that token, while BaseController#api_agent_name derives attribution exclusively from the token. Consequently every presence pill, reply, edit, and history entry is labeled “Bridge” rather than the agent that performed the work, losing the provenance this session-token split is meant to preserve. Mint the token with the configured/resumed agent identity instead of a fixed transport name.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 01340df: the mint example now uses the resumed agent's identity (agent_name: "Claude", harness: claude-code), and the guide says to name the token for the agent the bridge resumes, not the bridge.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01340df316
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <%= @curl %> -X POST \ | ||
| -H "Content-Type: application/json" \ | ||
| -d '{"url": "https://github.com/org/repo", "key": "service-repo", "title": "Service Repo"}' \ | ||
| "<%= @base %>/api/v1/plans/$PLAN_ID/references" | jq . | ||
| ``` | ||
|
|
||
| **Find plans that reference a URL:** | ||
|
|
||
| ```bash | ||
| <%= @curl %> \ | ||
| "<%= @base %>/api/v1/references/search?url=https://github.com/org/repo" | jq . | ||
| ``` | ||
|
|
||
| **Guidelines:** | ||
| - **Always set a `key`** when adding explicit references — it gives the reference a stable semantic identity (e.g., `auth-repo`, `impl-pr`, `design-doc`). | ||
| - Add references to any GitHub repos, PRs, design docs, or related plans mentioned in your plan. | ||
| - Links in plan content are auto-extracted — you don't need to add those manually. | ||
| - Use explicit references for resources not linked in the content but still relevant. | ||
| - Reference titles should be human-readable (e.g., "Auth Service Repo", not the raw URL). | ||
| - Use markdown reference-style links (`[key]: url "title"`) in content to auto-extract keyed references. | ||
|
|
||
| ### Attachments | ||
|
|
||
| Plans can carry file attachments (diagrams, screenshots, PDFs, data files). Limits: **25 MB per file**; allowed types are images (png/jpg/gif/webp), pdf, txt/md/csv/json, and zip. Only the plan author can upload or delete; anyone who can see the plan can list and download. | ||
|
|
||
| **Upload a file (multipart form):** | ||
|
|
||
| ```bash | ||
| <%= @curl %> -X POST \ | ||
| -F "file=@./architecture-diagram.png" \ | ||
| "<%= @base %>/api/v1/plans/$PLAN_ID/attachments" | jq . | ||
| -d '{"agent_name": "Claude", "metadata": {"harness": "claude-code", "harness_version": "2.1.3", "model": "claude-fable-5"}}' \ | ||
| "<%= @base %>/api/v1/tokens" | jq . |
There was a problem hiding this comment.
$TOKEN
Under the default bootstrap flow, $TOKEN still contains the principal's long-lived credential when this command runs, and piping the response to jq . only prints the child token—it never replaces the variable. An agent executing the examples literally therefore performs every subsequent write with the root token, shares its inbox/identity across runs, and eventually revokes the root credential with the cleanup command on line 33. Capture .token into $TOKEN as part of this command before any later API call.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in da69a14: the mint example is now TOKEN=$(… | jq -r .token), and the primer says it replaces the long-lived token with the session token for the rest of the run (and to reload the long-lived one only to re-mint after a 401). Verified against a local server: after running the rendered command, $TOKEN held a session token, and a mint attempt with it returned "Session tokens cannot mint further tokens". The coplan-attach examples now pass --no-session so they use that session token as-is.
| coplan-bridge --adapter amp --session T-<thread-id> --plan <plan-id> | ||
| ``` | ||
|
|
||
| First, your principal installs the bridge and mints its session token: see "Install" in <<%= @base %>/agent-instructions/live/bridge>. The bridge claims the session and sets it to `active` for you. |
There was a problem hiding this comment.
When an Amp agent follows this bridge option, it has already claimed the plan with the run token in the block at lines 9–12, but the linked Install guide mints a different token and the bridge claims the same plan again. Because subscriptions, inboxes, and presence rows are keyed by token, both identities remain subscribed: the original inbox accumulates duplicate events and can process feedback again on a later wait=0, while an original watching session can also leave a second pill. Fresh evidence beyond the prior bridge-token review is this newly documented handoff from an already-claimed Amp session; it should transfer/revoke the original subscription or arrange for the bridge token to be the run's token from the start.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in da69a14: the Amp guide now tells the agent to DELETE its own agent session after the bridge starts, so only the bridge's token stays subscribed.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da69a14101
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export COPLAN_TOKEN=$(curl -s -X POST \ | ||
| -H "Authorization: Bearer $(jq -r .token ~/.config/coplan/credentials.json)" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d '{"agent_name": "Claude", "ttl_seconds": 2592000, "metadata": {"harness": "claude-code"}}' \ | ||
| "$COPLAN_BASE/api/v1/tokens" | jq -r .token) |
There was a problem hiding this comment.
On hosts that authenticate token minting through api_authenticate (for example mTLS) or customize agent_auth_instructions/agent_curl_prefix, there may be no ~/.config/coplan/credentials.json, so this hard-coded Authorization header makes the documented bridge installation fail even though TokensController explicitly supports host request authentication. Build this request from the configured authentication instructions/prefix instead of assuming the engine's default credential file.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed by the rebase onto #243: the bridge guide now uses the host's mint command (agent_mint_curl_prefix, or plain curl -s when api_authenticate is configured) and only reads credentials.json when there is no request authentication. Covered by the "uses the host's authenticated mint command" spec, which checks that the bridge guide has no credentials.json in that case.
da69a14 to
708c4f6
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 708c4f62ce
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <% end %> | ||
| -H "Content-Type: application/json" \ | ||
| -d '{"agent_name": "Claude", "ttl_seconds": 2592000, "metadata": {"harness": "claude-code"}}' \ | ||
| "$COPLAN_BASE/api/v1/tokens" | jq -r .token) |
There was a problem hiding this comment.
null
When the long-lived credential is invalid or host authentication fails, the token endpoint returns an error object, but jq -r .token exits successfully and exports the literal value null. coplan-bridge then treats each immediate 401 response as an empty event batch because its request helper only warns on non-2xx responses, causing a tight, delay-free polling loop that can hammer the host while appearing to be configured. Validate that the mint returned a nonempty token and make the bridge stop or back off on authentication failures.
AGENTS.md reference: AGENTS.md:L162-L168
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 702edcf. The mint commands now use jq -er '.token // error(.error // "mint failed")': the primer only replaces $TOKEN on success, and the bridge guide only exports COPLAN_TOKEN on success, so a failed mint never yields null. coplan-bridge now exits on any 401 with a message to mint a new token, sleeps 5s after a poll that returns no events key, and sleeps 3s on throttled. Checked against a local server: a bad credential left $TOKEN unchanged and exported nothing (the bridge then refused to start), and a revoked token made the bridge exit at its first call.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 702edcf241
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # It's strongly recommended that you check this file into your version control system. | ||
|
|
||
| ActiveRecord::Schema[8.1].define(version: 2026_09_25_171730) do | ||
| ActiveRecord::Schema[8.1].define(version: 2026_10_06_135151) do |
There was a problem hiding this comment.
The schema now declares migration version 2026_10_06_135151, but a repo-wide search of both db/migrate/ and engine/db/migrate/ finds no corresponding migration; the newly recorded theme_preference and voice_hotkey columns likewise have no creating migration. Consequently, databases loaded from schema.rb contain columns and a recorded version that databases upgraded through the committed migrations never receive, causing development/test and deployed schemas to diverge. Regenerate this file from the migrations or add the owning engine migration.
AGENTS.md reference: AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, my mistake: a local db:migrate against a shared dev database rewrote db/schema.rb, and it got swept into the commit. Fixed by rebasing onto current main (which now has the real move_user_settings_out_of_metadata migration) and dropping the change. This PR now changes no files under db/ or engine/db/.
/agent-instructions was one 50 KB document that every agent read in full. It now covers what every agent needs (token, read, edit, comments, create, live-session rules) and links topic guides at /agent-instructions/<guide> that agents fetch only when a task needs them: markdown, presentations, creating, editing, comments, organizing, api, and live sessions per harness. Also fixes coplan-bridge acking events whose adapter command failed. Co-authored-by: Amp <amp@ampcode.com> Signed-off-by: Hampton Lintorn-Catlin <hampton@squareup.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a0fd43-bf31-734a-8716-7b5b912f1164
Add an AGENTS.md section: a change agents can observe is not done until the served primer and guides match it. Co-authored-by: Amp <amp@ampcode.com> Signed-off-by: Hampton Lintorn-Catlin <hampton@squareup.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a0fd43-bf31-734a-8716-7b5b912f1164
A failed mint printed the literal string null into $TOKEN or COPLAN_TOKEN, and coplan-bridge then polled in a tight loop on 401s. The documented commands now keep the old value when minting fails, the bridge exits on 401, and it backs off on any other bad poll. Co-authored-by: Amp <amp@ampcode.com> Signed-off-by: Hampton Lintorn-Catlin <hampton@squareup.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a0fd43-bf31-734a-8716-7b5b912f1164
|
@codex review |
702edcf to
5f0e7fe
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f0e7feb98
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 3. Make your change on top of the current content. Do not send the content that you had before. It does not include the person's edit, so it deletes their words. | ||
| 4. If your task conflicts with their edit, keep their text and explain the conflict in a comment. | ||
|
|
||
| This rule applies to every way that an agent writes: content replacement and operations. |
There was a problem hiding this comment.
When an agent uses the existing edit-session workflow, POST /api/v1/plans/:id/sessions/:id/commit is also protected by SessionsController's guard_human_edits!, but this new “every way” statement only names content replacement and operations; the previous primer explicitly included session commits. Preserve that warning so an agent does not treat a session commit as exempt from re-reading after a hand edit.
AGENTS.md reference: AGENTS.md:L162-L168
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e5c7d4a: the editing guide now says the hand-edit fence covers content replacement, operations, and edit-session commits (POST …/sessions/$SESSION_ID/commit).
| # for redelivery instead of acking a turn that never happened. | ||
| return if system(*cmd) | ||
|
|
||
| raise "#{adapter} adapter #{$?&.exitstatus ? "exited #{$?.exitstatus}" : "could not start"}" |
There was a problem hiding this comment.
When a resumed CLI performs API side effects and then exits nonzero—for example, after posting a reply but during teardown—this new raise redelivers the same event with the original prompt, so the next turn can post the reply or edit again. The ACP restart path already adds a recovery instruction to inspect existing work, but exec-adapter retries do not; add equivalent recovery/deduplication context before redispatching a previously attempted event.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e5c7d4a: the bridge remembers which event ids it has handed to an exec adapter. On redelivery it prepends the same recovery note the ACP path uses (check the thread and content, do not duplicate replies or edits). Checked against a local server with an adapter that fails once: attempt 1 got the plain prompt, attempt 2 began with the NOTE, and the event was acked after attempt 2 succeeded.
Co-authored-by: Amp <amp@ampcode.com> Signed-off-by: Hampton Lintorn-Catlin <hampton@squareup.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a0fd43-bf31-734a-8716-7b5b912f1164
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5c7d4a1ab
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1,79 @@ | |||
| # API Reference | |||
|
|
|||
| Every endpoint, with the guide that explains it. All paths start with `<%= @base %>`. <% if @request_auth_available %>Mint a session token through the host's request authentication (`<%= @mint_curl %>`). Every other call needs `Authorization: Bearer $TOKEN` with that session token.<% else %>Every call needs `Authorization: Bearer $TOKEN`. To mint a session token, send your principal's long-lived token. For every other call, send the session token.<% end %> See the primer at <<%= @base %>/agent-instructions>. Request bodies are JSON. | |||
There was a problem hiding this comment.
The new reference promises to list every endpoint, but it omits the three routes defined in engine/config/routes.rb for creating, inspecting, and committing edit sessions (POST /plans/:id/sessions, GET /plans/:id/sessions/:session_id, and POST .../commit). An agent consulting this endpoint inventory cannot discover how to create the $SESSION_ID mentioned by the editing guide or how to apply operations through a session; add these routes and their required fields to the plans table.
AGENTS.md reference: AGENTS.md:L162-L162
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 891a859: the API reference now lists POST …/sessions, GET …/sessions/:session_id, and POST …/sessions/:session_id/commit with their fields, and the editing guide explains the flow (open, send session_id on operation requests, commit with change_summary, 10-minute expiry).
| prompt = "NOTE: an earlier attempt at this event failed and may have partially completed. " \ | ||
| "Check the plan's thread and content first, and do not duplicate replies or edits already made.\n\n#{prompt}" | ||
| end | ||
| ATTEMPTED << event["id"] |
There was a problem hiding this comment.
For an exec-adapter bridge that runs continuously, every dispatched event ID is added to ATTEMPTED, but successful acknowledgement never removes it. Contrary to the nearby boundedness comment, leaving the server inbox does not affect this in-process Set, so its memory usage grows for the lifetime of the always-on bridge; delete an ID after its ack succeeds or replace this with a bounded retry cache.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 891a859: an id is removed from ATTEMPTED once its ack succeeds, so the set only holds events still in flight. Updated the comment to match.
Co-authored-by: Amp <amp@ampcode.com> Signed-off-by: Hampton Lintorn-Catlin <hampton@squareup.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a0fd43-bf31-734a-8716-7b5b912f1164
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 891a859468
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| An operation that cannot apply returns `422` with the reason, and CoPlan saves nothing. | ||
|
|
||
| To collect operations from several requests into one version, open an edit session: `POST <%= @base %>/api/v1/plans/$PLAN_ID/sessions`. Send its `id` as `session_id` on each operation request; CoPlan holds the operations without saving them. Then `POST <%= @base %>/api/v1/plans/$PLAN_ID/sessions/$SESSION_ID/commit` with a `change_summary` saves them as one version. A session expires after 10 minutes. Most edits do not need one: a single operations request already saves as one version. |
There was a problem hiding this comment.
When an agent follows this edit-session workflow and sends only session_id with its operations, OperationsController#create rejects the request with 422 because it validates base_revision before dispatching to the session path. Tell agents to send the session's returned base_revision with every POST /operations request so the documented workflow is executable.
AGENTS.md reference: AGENTS.md:L160-L168
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5f18b91, and confirmed against a local server: without base_revision the session operation returned 422 base_revision is required. With it, two operations were held (revision stayed 1, operations_count: 2) and the commit saved them as revision 2 with the change summary. The editing guide and API reference now say to send both session_id and the session's base_revision.
Co-authored-by: Amp <amp@ampcode.com> Signed-off-by: Hampton Lintorn-Catlin <hampton@squareup.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a0fd43-bf31-734a-8716-7b5b912f1164
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f18b917cb
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| ## Edit locks | ||
|
|
||
| A person who opens the web editor takes the plan's edit lock. While anyone else holds the lock, CoPlan refuses every write, including content replacement and operations, with `409` and `"code": "edit_locked"`. The lock expires 5 minutes after the holder stops. Wait, read the plan again, and retry. Do not retry in a tight loop. |
There was a problem hiding this comment.
When a person has the web editor open, an agent following the no-lease operations example above can still create a version: OperationsController#apply_direct never calls EditLease.enforce!, unlike content replacement and edit-session commits. This makes the promise here false and leaves the person's eventual save facing a stale-revision conflict; either require/acquire a lease for this flow or enforce existing leases in the direct-operations path.
AGENTS.md reference: AGENTS.md:L162-L168
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
No change needed: direct operations already honor the edit lock. PlanVersion has before_create :enforce_edit_lease (engine/app/models/coplan/plan_version.rb:5, :48-51), which locks the plan and calls EditLease.enforce!(plan:, lease_token: edit_lease_token) for every version, whatever path created it. commit_version in apply_direct goes through PlanVersion.create! with edit_lease_token: params[:lease_token] (nil here), so an active lease raises EditLease::Conflict, rendered as 409 edit_locked by BaseController. The existing spec spec/requests/api/v1/operations_spec.rb "rejects bypassing an active lease by omitting its token" covers exactly this and passes.
| | `GET /api/v1/library/contents` | All plans in the library, with summaries. | organizing | | ||
| | `POST /api/v1/library/organize` | Batch folder and plan moves, with `dry_run`. | organizing | | ||
| | `GET /api/v1/library/events` | The library's change log. | organizing | | ||
| | `GET /api/v1/libraries`, `GET /api/v1/libraries/:id` | All libraries, or one library. | organizing | |
There was a problem hiding this comment.
When an agent needs to inspect or organize a library other than its principal-relative default, this endpoint inventory does not expose GET /api/v1/libraries/:id/contents, GET /api/v1/libraries/:id/events, or POST /api/v1/libraries/:id/organize, even though all three are defined by the member routes in engine/config/routes.rb. Listing only GET .../:id and the /library/... aliases prevents agents from discovering the bulk read, audit, and write operations for a selected library.
AGENTS.md reference: AGENTS.md:L162-L166
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in fc39678: the API reference now lists GET /api/v1/libraries/:id/contents, POST /api/v1/libraries/:id/organize, and GET /api/v1/libraries/:id/events, with who may call each.
Co-authored-by: Amp <amp@ampcode.com> Signed-off-by: Hampton Lintorn-Catlin <hampton@squareup.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a0fd43-bf31-734a-8716-7b5b912f1164
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc396783f8
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| -d '{"agent_name": "Claude", "ttl_seconds": 2592000, "metadata": {"harness": "claude-code"}}' \ | ||
| "$COPLAN_BASE/api/v1/tokens" | jq -er '.token // error(.error // "mint failed")') && export COPLAN_TOKEN |
There was a problem hiding this comment.
$TOKEN
When a principal follows this install block, the adapter subprocess inherits only COPLAN_TOKEN, while the served API examples and default agent_curl_prefix authenticate with $TOKEN; coplan-bridge also never aliases these variables before invoking system(*cmd). A resumed agent therefore sends an empty Authorization header or reuses a stale run token, and the bridge can still acknowledge the event after the CLI exits successfully. Export the minted value as TOKEN for adapter children or explicitly make the wake instructions use COPLAN_TOKEN.
AGENTS.md reference: AGENTS.md:L162-L168
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 281a58b: exec adapters now run with TOKEN and COPLAN_TOKEN set to the bridge's session token (system({ "TOKEN" => TOKEN, ... }, *cmd)), and the wake prompt says to authenticate with Bearer $TOKEN. Checked with a fake adapter while the parent shell had a stale TOKEN: the child saw the bridge token, and the prompt named $TOKEN. The bridge guide says this too.
| ```bash | ||
| <%= @curl %> "<%= @base %>/api/v1/library/contents" | jq . |
There was a problem hiding this comment.
When the requested reorganization targets a library other than the principal's, Step 1 allows selecting it with /libraries/$LIBRARY_ID and Step 3 allows writing to /libraries/$LIBRARY_ID/organize, but this Step 2 command still reads the principal-relative /library/contents. The agent can consequently classify the wrong plans and submit those IDs to the selected library's organizer, potentially moving plans across libraries. The newly added API-reference row exposes the scoped endpoint, but this workflow must use /libraries/$LIBRARY_ID/contents consistently.
AGENTS.md reference: AGENTS.md:L162-L168
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 281a58b: Step 2 now says to use GET /api/v1/libraries/$LIBRARY_ID/contents for another library, with the same $LIBRARY_ID in Step 3, and to keep to one library for the whole task.
Co-authored-by: Amp <amp@ampcode.com> Signed-off-by: Hampton Lintorn-Catlin <hampton@squareup.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a0fd43-bf31-734a-8716-7b5b912f1164
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 281a58b94c
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - `GET <%= @base %>/api/v1/library` returns your principal's folder tree. Each folder has a `description` of what belongs in it. Read the descriptions before you file a plan. Use the existing folders before you make new ones. | ||
| - To file or move a plan that your principal wrote, send `PATCH <%= @base %>/api/v1/plans/$PLAN_ID` with `{"folder_path": "Team EBT/Q3"}`. To remove it from its folder, send `{"folder_id": ""}`. | ||
| - Name folders like places (`Team EBT`, `Infra`, `Q3 Launch`). Give each new folder a one-line description. The next agent reads it. | ||
| - Folder endpoints: `GET`, `POST`, `PATCH`, and `DELETE` on `<%= @base %>/api/v1/folders`. You can delete only empty folders. |
There was a problem hiding this comment.
When an agent tries to rename or delete a folder from this guide, PATCH and DELETE against /api/v1/folders do not match the routes; both operations require /api/v1/folders/:id. Preserve the collection URL for GET/POST, but document the member URL for PATCH/DELETE so these instructions can be followed literally.
AGENTS.md reference: AGENTS.md:L162-L168
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c15cbcc: the creating guide now says GET/POST on /api/v1/folders and PATCH/DELETE on /api/v1/folders/$FOLDER_ID. I also checked every method + path in the guides and coplan-bridge (39 curl/request commands plus prose references) against rails routes; this was the only mismatch.
Co-authored-by: Amp <amp@ampcode.com> Signed-off-by: Hampton Lintorn-Catlin <hampton@squareup.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a0fd43-bf31-734a-8716-7b5b912f1164
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c15cbcc5b8
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| Respond as the plan's agent, over the CoPlan API at #{BASE} (full instructions: #{BASE}/agent-instructions): | ||
| Respond as the plan's agent, over the CoPlan API at #{BASE} (full instructions: #{BASE}/agent-instructions). | ||
| Authenticate every call with `Authorization: Bearer $TOKEN`: $TOKEN in your environment is this bridge's session token. |
There was a problem hiding this comment.
$TOKEN into ACP subprocesses
When the documented --acp mode is used, the install flow exports only COPLAN_TOKEN, while AcpAgent#boot starts the ACP command with Open3.popen2(*@command) and never creates the $TOKEN alias that the exec-adapter branch supplies. This prompt therefore tells the ACP agent to authenticate with an unset or stale variable, so its API calls fail even though the bridge itself can poll successfully; inject both token variables into the ACP subprocess or direct ACP prompts to the exported variable.
AGENTS.md reference: AGENTS.md:L162-L168
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a23571e: AcpAgent#boot now starts the ACP command with Open3.popen2({ "TOKEN" => TOKEN, "COPLAN_TOKEN" => TOKEN }, *@command), matching the exec adapters, so the prompt's $TOKEN is the bridge token.
|
|
||
| - Take it: `POST <%= @base %>/api/v1/plans/$PLAN_ID/lease` with `{"lease_token": "<random string>"}`. | ||
| - Send the same `lease_token` with each content or operation request. | ||
| - Renew it with `PATCH` before 5 minutes pass. Release it with `DELETE` as soon as you finish. While you hold it, people cannot save in the editor. |
There was a problem hiding this comment.
When an agent follows these lock instructions, a bare PATCH or DELETE cannot renew or release the lease: LeasesController passes params[:lease_token] to renew!/release!, and a missing token produces a conflict. The preceding sentence only requires the token on content or operation requests, so document that both lease calls also need {"lease_token":"..."} (preferably with literal commands).
AGENTS.md reference: AGENTS.md:L162-L168
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a23571e: the editing guide now says renew and release need the same lease_token, with literal commands. Ran them against a local server: renew returned a new expires_at, release returned 204, and a following DELETE reported no active lease.
Co-authored-by: Amp <amp@ampcode.com> Signed-off-by: Hampton Lintorn-Catlin <hampton@squareup.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a0fd43-bf31-734a-8716-7b5b912f1164
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
What changed and why
/agent-instructionswas one 50 KB document. Every agent read all of it, including the live-session protocol and library organizing, which most tasks never use. Basics such as tables and diagrams were buried in the middle.Now
/agent-instructionsis a primer of about 10 KB. It says how CoPlan works in practice (the agent acts for one principal, a hand edit outranks agent work, give people theurl), then six steps: mint a session token, read, edit, answer comments, create a plan, and stay for feedback. It ends with writing rules and a table of guides.Guides live at
/agent-instructions/<guide>, and agents fetch them only when a task needs them:markdown: tables, diagrams, code blocks, footnotes, section links, task lists, imagespresentations:::: {.presentation}deck regions and slide layout rulescreating,editing,comments,organizing,apilive(the protocol), pluslive/claude-code,live/amp,live/codex,live/hosted,live/bridgeThe live guides keep the current rules: show a pill only when something can wake the agent, wait in the foreground only when asked and with a time limit, and do not count an ACP agent as attachment. Without a wake, the agent claims the session, sets it to
complete, and reads its inbox withwait=0.The rewrite also fixes facts that had drifted: the old doc said a lease was needed for operations, that a plan can sit in many libraries, and that anyone can upload attachments. None of these is true now. It also documents the
edit_locked409 from the web editor's lock, which was missing.It fixes one bug in
coplan-bridge: it acked an event even when the adapter command failed, becausesystemreturns false instead of raising. A failed turn now stays in the inbox for redelivery.The old
/agent-instructions/organizingURL still works; library API responses link it. A spec checks that the primer links every guide and nothing else.This builds on the setup page from #243: a browser that opens
/agent-instructionsstill goes to/_/agent/setup, and/_/agent/instructionsrenders the primer. Guides render as HTML in place, with a link back to that page. The primer, API guide, and bridge guide use the host's mint command (agent_mint_curl_prefixor request auth) when one is configured.AGENTS.mdgets a new section, "Agent Instructions Stay Current": a change that agents can observe is not done until the primer and guides match it. It says where each kind of detail goes and how to add a guide.Evidence
Primer, light and dark:
Presentations guide, light and dark:
How to try it
Open
/agent-instructionsin a browser and follow a link in the Guides table. Each guide has a back link and a Copy URL button.Testing
ContentRegions::SplitandSlideshows::Classify. It parses as onepresentationregion (#q3-review, themecoplan) with atitleslide and acontentslide.bundle exec rspecafter rebasing ontomain: 2460 examples, 1 failure. The failure isspec/system/editor_code_scope_spec.rb:74, which passed twice when run alone; this PR does not touch the editor.Open questions
urlto find the id. Someone else's private plan is never in that list, so the agent cannot find it.PUT /contenthas no author check, butPATCH /plans/:idis author-only. The primer tells agents to edit someone else's plan only when asked. Is open content editing intended?Generated with Amp