Skip to content

Usage is very high #18

Description

@ableinc

Ensure that we are only prompting the AI with the minimal data it needs. Upon auditing i'm finding a single session taking upwards of 1 millions tokens collectively between tokens up and tokens down. This is way too much and its causing claude rate limits.

Activity

  1. ableinc commented on Aug 29, 2026

    @ableinc
    OwnerAuthor

    Plan

    Cut per-run token usage: trim the harness prompt, stop the CLI loading ambient operator context, cap per-run spend, and report cache reads separately from fresh input

    Context

    Issue #18 reports single sessions totalling ~1M tokens "up and down", causing Claude rate limits, and asks that we prompt the model with only the minimal data it needs.

    Auditing the code turns up two separate things, and both need addressing:

    1. The 1M number is mostly cache reads. claude.Result.TokensIn() (internal/claude/runner.go:110) sums input_tokens + cache_creation_input_tokens + cache_read_input_tokens into one figure, which is stored as runs.tokens_in and rendered by the web console as plain "Tokens in" (internal/web/assets/app.js:649,685) and by Discord (internal/discord/notifier.go:269). On an agentic run the whole conversation prefix is re-read from cache on every turn, so a 40-turn run with a 25K prefix reports ~1M input tokens while only tens of thousands were ever processed fresh. Cache reads are billed at a fraction of fresh input and are not what drives a rate limit. The number is real but it is not the number the operator thinks it is, and today there is no way to see the split.
    2. There is genuine, fixable bloat, in two places: what the harness puts into the prompt, and what the Claude Code CLI loads into every turn's system prompt because we never tell it not to. Neither is 1M tokens on its own, but both are paid on every turn of every run.

    The intended outcome: the same work, with a materially smaller context re-sent per turn, a hard per-run spend ceiling the daemon can enforce, and a dashboard that distinguishes cache traffic from fresh input so "usage is very high" can be judged from data rather than a conflated total.

    Approach

    Part A — Report cache reads separately from fresh input

    Nothing here changes what is sent to Claude; it changes what the operator sees, and it is the part that actually answers "is 1M tokens a problem?".

    • internal/claude/runner.go: keep TokensIn() as the total (so existing runs.tokens_in rows keep their meaning) and add three accessors alongside it, mirroring the existing TokensOut() style: FreshTokensIn() (Usage.InputTokens), CacheWriteTokens() (CacheCreationInputTokens), CacheReadTokens() (CacheReadInputTokens). The Usage struct already parses all four fields.
    • internal/store/store.go: append a third entry to the migrations slice — the file already establishes this pattern at line 285 (ALTER TABLE runs ADD COLUMN kind ...) and the migration loop bumps PRAGMA user_version per entry, so existing databases upgrade in place:
      ALTER TABLE runs ADD COLUMN tokens_cache_read  INTEGER NOT NULL DEFAULT 0;
      ALTER TABLE runs ADD COLUMN tokens_cache_write INTEGER NOT NULL DEFAULT 0;
      Add the two fields to store.Run, to the runColumns const, and to scanRun.
    • RecordUsage already takes seven positional arguments; adding two more makes it unreadable. Replace the tail arguments with a small store.RunUsage{ModelID, SessionID, CostUSD, TokensIn, TokensOut, CacheRead, CacheWrite, Turns} value and update the four call sites (internal/orchestrator/loop.go:660, :740, internal/orchestrator/prcomments.go:385, :435). Keep the existing "empty model/session does not overwrite" CASE WHEN semantics — the pre-record call at loop.go:660 depends on it.
    • Surface the split:
      • internal/web/assets/app.js — runs table cell (line 649) and detail grid (lines 685-686): render 1.0M in (18K new · 24K written · 950K cached) / 42K out, reusing the existing fmtTokens helper (line 132). No CSS or server-side change is needed: /runs already marshals the whole store.Run.
      • internal/discord/notifier.go:269 — same breakdown in the Tokens embed field.
      • internal/orchestrator/loop.go:794 — extend the claude_done event detail (and the equivalent in prcomments.go:479) with fresh_in=… cached_in=… out=… so the per-run audit trail carries it.

    Part B — Trim what the harness puts in the prompt

    All in internal/orchestrator/prompt.go. The current worst case for issueContext is ~36K chars (12K body + 12 × 2K comments) and implementTaskPrompt appends the approved plan untruncated on top of that; prCommentTaskPrompt is unbounded in the number of comments it renders.

    1. Stop feeding the harness's own comments back to the model. issueContext (line 190) renders every comment, including the plan comment, the PR announcement, and every failure comment the daemon itself wrote. Filter with the existing isAgentComment helper (phase.go:37) before the maxCommentsInclu window is applied. This is the single largest and safest win: on a re-plan or implement run the plan is currently sent twice (truncated inside the discussion, in full under "Approved plan"), and failure comments are pure noise. The re-plan path's wording ("see the newest comment in the discussion above") still holds — reviewer feedback is a human comment and survives the filter. Optionally also drop bare implement approvals via isApproval (phase.go:51); they carry no information the "Approved plan" header does not already state.
    2. Tighten the constants (lines 11-15), which are the only knobs the truncation uses: maxBodyChars 12000 → 6000, maxCommentChars 2000 → 1200, maxCommentsInclu 12 → 6. Every one already routes through the existing truncate helper, which appends truncationSuffix — and extractPlan (phase.go:134) keys off that suffix to refuse a truncated plan, so the plan cap below must stay well clear of any real plan length.
    3. Cap the plan. Add maxPlanChars = 20000 and apply truncate to the plan in implementTaskPrompt (line 265) and to previousPlan in planTaskPrompt (line 244). 20K chars is roughly 5K tokens and comfortably above a normal plan, so TestImplementTaskPromptCarriesTheApprovedPlanVerbatim keeps passing; the cap only bites on a runaway plan.
    4. Bound prCommentTaskPrompt (line 140), which today has no limit on comment count: add maxPRCommentsInclu = 10 (keep the newest, state how many were dropped, mirroring the issueContext "showing the last N of M" line), give diff hunks their own smaller maxDiffHunkChars = 800, and cap review summaries at 3. pendingMentions (prcomments.go:106) can legitimately return dozens of inline comments after a big review pass.

    Note that the system prompts themselves (systemPrompt, planSystemPrompt, prCommentSystemPrompt) are ~1.5K chars each and are not worth touching — they are the cheapest part of the prompt and each line in them is load-bearing for a safety rule.

    Part C — Stop the CLI loading the operator's ambient context

    This is the lever with the largest per-turn effect and it is currently entirely unmanaged. The daemon shells out to claude with the operator's own environment (internal/claude/runner.go:198-222), so every run inherits their user-level MCP servers, plugins, skills, and settings. MCP tool schemas and the skills listing sit in the system prompt on every turn — this is exactly the "prompting the AI with data it does not need" the issue describes, and it is invisible in the harness's own code.

    In internal/claude/runner.go, extend Options and the args construction (after the existing --permission-mode block, before opts.ExtraArgs so an operator can still override):

    Flag Why Default
    --strict-mcp-config With no --mcp-config, this loads zero MCP servers. The harness's runs need none — they get the built-in file/bash tools. on
    --disable-slash-commands Drops the skills listing from the system prompt; the harness drives the model from stdin and never types a slash command. on
    --exclude-dynamic-system-prompt-sections Moves per-machine sections (cwd, env, git status) out of the system prompt, improving prompt-cache reuse across runs. Applies here because we use --append-system-prompt, not --system-prompt. on
    --autocompact <tokens> Caps how large the conversation grows before it is compacted. This is what bounds the per-turn cache-read cost that produced the 1M figure. Accepts 100k–1M. 200000
    --setting-sources <list> Controls whether user/project/local settings load at all. leave unset — see Risks

    Expose these as first-class config.ClaudeConfig fields (StrictMCPConfig, DisableSlashCommands, ExcludeDynamicSystemPrompt, AutocompactTokens, SettingSources) rather than asking operators to know the flags, with the defaults above set in config.Default() (internal/config/config.go:244). config.Migrate is Default()-driven and generic, so existing config.json files pick the new fields up through -migrate-config with no change to internal/config/migrate.go.

    Validate AutocompactTokens in Config.Validate(): zero means "don't pass the flag", otherwise it must be within 100000–1000000, matching what the CLI accepts.

    Part D — A hard per-run spend ceiling

    The README states as a principle that there is "no cost budget", and the gate only reacts after Claude reports a limit. The CLI offers --max-budget-usd <amount> (print mode only), which stops a run at a dollar figure instead of letting a pathological run consume an hour of usage before the run.timeout fires.

    • Add Options.MaxBudgetUSD float64 to internal/claude/runner.go and pass --max-budget-usd when it is > 0.
    • Add run.max_budget_usd to config.RunConfig and set it from execute (loop.go:700) and executePRComments (prcomments.go:403).
    • Default: 0 (off), so behaviour is unchanged unless the operator opts in. config.example.json should ship a non-zero suggestion in the documented reference so the knob is discoverable.
    • A budget-terminated run surfaces through the existing failure path: diagnose (runner.go:293) already renders subtype, terminal_reason and the CLI's own message into the error, so the run is recorded as failed with an explanatory reason and backs off normally.

    There is no --max-turns in the installed CLI (2.1.x) — do not reach for it.

    Files touched

    • internal/claude/runner.go — new usage accessors; new Options fields; new args.
    • internal/orchestrator/prompt.go — comment filtering, tightened constants, plan and PR-comment caps.
    • internal/orchestrator/loop.go, internal/orchestrator/prcomments.go — RecordUsage call sites, new claude Options, richer claude_done events.
    • internal/store/store.go — third migration entry, two Run fields, runColumns, scanRun, RecordUsage signature.
    • internal/config/config.go — new ClaudeConfig and RunConfig fields, defaults, validation.
    • internal/web/assets/app.js, internal/discord/notifier.go — token breakdown display.
    • config.example.json (also the embedded default via embedded.go); models.json untouched.
    • README.md — "How Claude Code is invoked" (line 319, the claude invocation block is verbatim and must be updated), the "Prompts" section's description of what goes into the prompt, the configuration reference, and the "Usage-limit-aware, not schedule-aware" principle at line 86, which no longer holds literally once run.max_budget_usd exists.
    • Tests: internal/orchestrator/prompt_test.go (add cases for agent-comment filtering and the PR-comment cap), internal/store/store_test.go (a migration test in the shape of TestMigrationAddsKindAndPRCommentTasks), internal/claude/runner_test.go (assert the new flags appear, in the style of the existing arg assertions).

    Verification

    1. go build ./... && go test ./... — the store migration test and the prompt tests are the ones that should visibly change.
    2. Prompt size, measured rather than assumed: add a temporary test (or a -run one-off) that builds implementTaskPrompt from a fixture issue with a long body, 20 comments (half of them agent comments) and a long plan, and prints len() before and after. Expect roughly a 2-3× reduction.
    3. ./coding-agent-loop -dry-run -once — exercises discovery, phase decision and model selection without spending anything; confirms the config additions load and validate.
    4. A real single run against a scratch issue: ./coding-agent-loop -no-mutate -once (runs Claude for real, pushes nothing). Then inspect the transcript:
      jq -c 'select(.type=="system") | {tools: (.tools|length), mcp: (.mcp_servers//[]|length)}' ~/.agent-loop/logs/<run>.jsonl | head -1
      jq  'select(.type=="result") | .usage' ~/.agent-loop/logs/<run>.jsonl
      The first line confirms --strict-mcp-config took effect (no MCP servers); the second gives the four-way usage split to compare against a pre-change run on the same issue.
    5. Open /ui, check the run row and detail page show the in (new · written · cached) / out breakdown, and confirm a Discord notification (if enabled) carries the same.

    Risks and decisions for the reviewer

    • --setting-sources is deliberately left unset. Restricting it would cut the most ambient context, but user-level settings can carry auth helpers, permission rules and env that a bypassPermissions daemon run may depend on. Turning it off could break runs on the operator's machine in a way that is hard to diagnose. It is exposed as config, defaulted to unchanged behaviour. Decide whether to default it to project instead — that is the aggressive choice and would need a manual run to confirm auth still works.
    • --disable-slash-commands also disables skills. If a target repository ships .claude/skills that the agent is expected to use, this would silently remove them. The harness has never invoked a skill explicitly, so this is judged safe, but it is a behaviour change worth a conscious yes.
    • --autocompact 200000 changes behaviour on long runs: the model's earlier context gets summarized rather than carried verbatim. This is the trade the issue is asking for (fewer tokens per turn) but it can cost quality on a long implement run. 200K is a conservative starting point — the CLI's own default is higher.
    • Redefining tokens_in was considered and rejected. Making tokens_in mean "fresh input only" would give a more honest headline number, but silently changes the meaning of every existing row. The plan keeps the total and adds the breakdown instead. Say so if you would rather have the cleaner semantics and accept the discontinuity.
    • Retry behaviour is deliberately untouched. retryDelay (loop.go:384) caps back-off at retry_backoff_max (24h default) and never gives up, so a permanently-broken issue costs a full plan-or-implement run every day forever. Capping that would contradict the documented "the trigger label is the only thing that decides whether it is worked" principle, so it is flagged rather than changed. If reducing usage matters more than that principle, the cheap version is: after N consecutive failures, drop the trigger label and say so in the failure comment.
    • The plan/implement split inherently pays for exploration twice — the implement run cannot see the plan run's session (--no-session-persistence, and a human gate sits between them). Nothing in this plan changes that; a shared-session design would be a much larger change and would weaken the "the plan a human approved is the one on the issue" guarantee.

    Reply with exactly implement to approve this plan and start the change. Reply with anything else and the plan will be revised to address it.

    coding-agent-loop run ac782780-2909-4afc-a4ef-a8d3f1dcc03b, model claude-opus-5, cost $2.8418

  2. ableinc commented on Aug 30, 2026

    @ableinc
    OwnerAuthor

    Every solution doesn't require you extend the config.json. This tool needs to be slim and great at what its intended for out of the box. We don't need a configuration tweak for every little fix. Revise your plan such that all of this is baked in my default - no configuration overrides.

  3. ableinc commented on Aug 30, 2026

    @ableinc
    OwnerAuthor

    Plan

    Cut per-run token usage with baked-in defaults: trim the harness prompt, stop the CLI loading ambient operator context, and split cache reads out of the reported "tokens in"

    What changed since the previous plan

    The reviewer's feedback was: "Every solution doesn't require you extend the config.json. This tool needs to be slim and great at what its intended for out of the box... all of this is baked in by default — no configuration overrides."

    Accordingly:

    • No new config.json fields. The previous plan added five claude.* fields and one run.* field. All are gone. internal/config/config.go, config.example.json, embedded.go and internal/config/migrate.go are untouched, and no -migrate-config step is required to get the benefit.
    • No new claude.Options fields either. The CLI flags are appended unconditionally inside Runner.Run, next to the --print / --no-session-persistence flags that are already hardcoded there. That is the existing precedent in the file for "this is how the harness invokes Claude, full stop."
    • The per-run dollar budget (--max-budget-usd) is dropped entirely. It only made sense as a number an operator picks; a hardcoded dollar figure would either be so high it never fires or would kill legitimate long runs with a confusing error, and on an OAuth subscription the reported cost is imputed rather than billed. run.timeout (45m) already bounds a run in wall-clock, and --autocompact below bounds what each turn costs. Dropping it also means the README's "no time-of-day window and no cost budget" principle (line 86) stays true and needs no rewrite.
    • Everything else — the prompt trimming and the token-reporting split — was already constants-and-code with no configuration surface, and survives as-is.

    The one operator escape hatch that remains is the one that already exists: claude.extra_args is still appended last, so --mcp-config … can be added back by anyone who genuinely needs an MCP server in agent runs.

    Context

    Issue #18 reports single sessions totalling ~1M tokens "up and down", hitting Claude rate limits, and asks that the model be prompted with only the minimal data it needs.

    The audit turns up two separate things:

    1. The 1M number is mostly cache reads. claude.Result.TokensIn() (internal/claude/runner.go:110) sums input_tokens + cache_creation_input_tokens + cache_read_input_tokens into one figure, stored as runs.tokens_in and rendered as plain "Tokens in" by the web console (internal/web/assets/app.js:649,685) and Discord (internal/discord/notifier.go:269). On an agentic run the whole conversation prefix is re-read from cache every turn, so a 40-turn run with a 25K prefix reports ~1M input tokens while only tens of thousands were ever processed fresh. Cache reads are billed at a fraction of fresh input and are not what drives a rate limit. The number is real, but it is not the number the operator thinks it is, and today there is no way to see the split.
    2. There is genuine, fixable bloat, in two places: what the harness puts in the prompt, and what the Claude Code CLI loads into every turn's system prompt because we never tell it not to. Neither is 1M tokens on its own, but both are paid on every turn of every run.

    Intended outcome: the same work, with a materially smaller context re-sent per turn, and a dashboard that distinguishes cache traffic from fresh input so "usage is very high" can be judged from data. All of it on by default, with nothing to configure.


    Part A — Report cache reads separately from fresh input

    Changes what the operator sees, not what is sent to Claude. It is the part that actually answers "is 1M tokens a problem?".

    internal/claude/runner.go — keep TokensIn() as the total (so existing runs.tokens_in rows keep their meaning) and add three accessors alongside it in the style of the existing TokensOut(): FreshTokensIn() (Usage.InputTokens), CacheWriteTokens() (CacheCreationInputTokens), CacheReadTokens() (CacheReadInputTokens). The Usage struct already parses all four fields.

    internal/store/store.go — append a third entry to the migrations slice. The file already establishes this pattern (entry 2, ALTER TABLE runs ADD COLUMN kind …, line 285) and the migration loop bumps PRAGMA user_version per entry, so existing databases upgrade in place rather than being deleted:

    ALTER TABLE runs ADD COLUMN tokens_cache_read  INTEGER NOT NULL DEFAULT 0;
    ALTER TABLE runs ADD COLUMN tokens_cache_write INTEGER NOT NULL DEFAULT 0;

    Add the two fields to store.Run (line 83), to the runColumns const (line 579), and to scanRun (line 582) in the same positions.

    RecordUsage signature — it already takes seven positional args (store.go:453); two more makes it unreadable. Replace the tail with a store.RunUsage{ModelID, SessionID, CostUSD, TokensIn, TokensOut, CacheRead, CacheWrite, Turns} value and update the four call sites: internal/orchestrator/loop.go:660 and :740, internal/orchestrator/prcomments.go:385 and :435. Keep the existing "empty model/session does not overwrite" CASE WHEN semantics — the pre-record call at loop.go:660 depends on it.

    Surface the split:

    • internal/web/assets/app.js — runs table cell (line 649) and detail grid (lines 685-686): render 1.0M in (18k new · 24k written · 950k cached) / 42k out, reusing the existing fmtTokens helper (line 132). No server-side or CSS change: /runs already marshals the whole store.Run.
    • internal/discord/notifier.go:269 — same breakdown in the Tokens embed field.
    • internal/orchestrator/loop.go:794 and prcomments.go:479 — extend the claude_done event detail with fresh_in=… cached_in=… out=… so the per-run audit trail carries it.

    Part B — Trim what the harness puts in the prompt

    All in internal/orchestrator/prompt.go. Worst case today for issueContext is ~36K chars (12K body + 12 × 2K comments), and implementTaskPrompt appends the approved plan untruncated on top of that; prCommentTaskPrompt has no cap at all on comment count.

    1. Stop feeding the harness's own comments back to the model. issueContext (line 190) renders every comment, including the plan comment, the PR announcement, and every failure comment the daemon itself wrote. Filter with the existing isAgentComment helper (phase.go:37) — and also drop bare approvals via isApproval (phase.go:51), which carry no information the "Approved plan" header doesn't already state — before the maxCommentsInclu window is applied, and report "showing the last N of M" against the filtered count.

      This is the single largest and safest win. On a re-plan or implement run the plan is currently sent twice (truncated inside the discussion, in full under "Approved plan"), and failure comments are pure noise. The re-plan wording ("see the newest comment in the discussion above") still holds: reviewer feedback is a human comment and survives the filter.

      Edge case to handle: when filtering leaves zero comments (e.g. the first implement run, whose only comments are the plan and the word implement), omit the ### Discussion header entirely rather than emitting an empty section.

    2. Tighten the constants (lines 11-15), which are the only knobs the truncation uses: maxBodyChars 12000 → 6000, maxCommentChars 2000 → 1200, maxCommentsInclu 12 → 6. Everything already routes through the existing truncate helper.

    3. Cap the plan. Add maxPlanChars = 20000 and apply truncate to the plan in implementTaskPrompt (line 265) and to previousPlan in planTaskPrompt (line 244). Note the coupling: truncate appends truncationSuffix, and extractPlan (phase.go:134) deliberately refuses to recover a plan ending in that suffix — 20K chars (~5K tokens) is comfortably above any real plan, so the cap only bites on a runaway one and TestImplementTaskPromptCarriesTheApprovedPlanVerbatim keeps passing.

    4. Bound prCommentTaskPrompt (line 140), which today renders every comment pendingMentions (prcomments.go:106) returns — that can be dozens after a big review pass. Add maxPRCommentsInclu = 10 (keep the newest, state how many were dropped, mirroring the issueContext line), give diff hunks their own smaller maxDiffHunkChars = 800 instead of reusing maxCommentChars, and cap review summaries at 3. TestPRCommentTaskPromptIncludesEveryCommentAndDiffHunk uses two comments and one review, so it is unaffected.

    Not touched: systemPrompt, planSystemPrompt, prCommentSystemPrompt are ~1.5K chars each, are the cheapest part of the prompt, and each line is load-bearing for a safety rule.


    Part C — Stop the CLI loading the operator's ambient context

    The lever with the largest per-turn effect, and currently entirely unmanaged. The daemon shells out to claude with the operator's own environment (internal/claude/runner.go:198-222), so every run inherits their user-level MCP servers, plugins and skills. MCP tool schemas and the skills listing sit in the system prompt on every turn — exactly the "prompting the AI with data it does not need" the issue describes, and invisible from inside the harness's own code.

    In internal/claude/runner.go, add these to the hardcoded args literal (line ~178), alongside --print / --output-format / --verbose / --no-session-persistence, i.e. before opts.ExtraArgs is appended:

    Flag Why Verified on 2.1.240
    --strict-mcp-config With no --mcp-config, loads zero MCP servers. Harness runs need none — they use the built-in file/bash tools. Cuts every inherited MCP server's tool schemas out of the system prompt. ✅ accepted
    --disable-slash-commands "Disable all skills" — drops the skills listing from the system prompt. The harness drives the model from stdin and never types a slash command. ✅ accepted
    --exclude-dynamic-system-prompt-sections Moves per-machine sections (cwd, env, git status) out of the system prompt into the first user message. This is a prompt-cache-reuse win, not a size reduction — be honest about that in the commit message. Applies here because we use --append-system-prompt, not --system-prompt. ✅ accepted
    --autocompact 200000 Caps how large the conversation grows before compaction. This is what bounds the per-turn cache-read cost that produced the 1M figure. ✅ accepted (CLI validates the range: bogus is rejected with "must be 'auto', or between 100k and 1M")

    Deliberately not passed: --setting-sources (see Risks), --safe-mode, --bare, --tools.

    Since these are literals rather than Options fields, no validation code is needed — the CLI validates --autocompact itself at parse time, before any API call.


    Approaches considered and rejected for Part C

    • --safe-mode looks like the perfect "slim by default, one flag" answer: it disables CLAUDE.md, skills, plugins, hooks, MCP servers, custom commands, agents and output styles in one go, while leaving auth, model selection, built-in tools and permissions working. It is rejected because it also disables the target repository's CLAUDE.md, which is genuine project context the harness's own system prompt asks the agent to honour ("Follow the conventions already present in the repository"). The four targeted flags get most of the saving without giving that up.
    • --bare is rejected outright: it forces Anthropic auth to ANTHROPIC_API_KEY/apiKeyHelper and never reads OAuth or the keychain. The daemon authenticates via ~/.claude/.credentials.json, so this would break every run.
    • --tools <subset> would shave the built-in tool definitions, but the agent legitimately needs the full file/bash/search set and losing web access silently would be a real capability regression. Not worth the tokens.

    Files touched

    • internal/claude/runner.go — three new usage accessors; four new hardcoded args.
    • internal/orchestrator/prompt.go — agent-comment filtering, tightened constants, plan cap, PR-comment caps.
    • internal/orchestrator/loop.go, internal/orchestrator/prcomments.go — RecordUsage call sites, richer claude_done events.
    • internal/store/store.go — third migration entry, two Run fields, runColumns, scanRun, RunUsage struct + RecordUsage.
    • internal/web/assets/app.js, internal/discord/notifier.go — token breakdown display.
    • README.md — the claude … invocation block at line 327 is verbatim and must gain the four flags plus a sentence on why; the Prompts section (line ~312) describes issueContext truncation and should mention that harness-authored comments are filtered out. No change to the Configuration reference, the principles list, or models.json.
    • Not touched: internal/config/config.go, internal/config/migrate.go, config.example.json, embedded.go.
    • Tests: internal/orchestrator/prompt_test.go (agent-comment filtering; the empty-discussion edge case; the PR-comment cap), internal/store/store_test.go (migration test shaped like the existing TestMigrationAddsKindAndPRCommentTasks; the two existing RecordUsage call sites at lines 154 and 444 need updating for the new signature), internal/claude/runner_test.go (assert the four flags appear in the invocation).

    Verification

    1. go build ./... && go test ./... — the store migration test and the prompt tests are the ones that should visibly change.
    2. Measure the prompt, don't assume it. Add a test in prompt_test.go that builds implementTaskPrompt from a fixture issue with a long body, 20 comments (half of them carrying markerPrefix) and a long plan, and asserts len(p) is under a fixed ceiling. Expect roughly a 2-3× reduction versus today; keeping it as a real assertion stops the constants drifting back up.
    3. ./coding-agent-loop -dry-run -once — exercises discovery, phase decision and model selection without spending anything.
    4. A real single run against a scratch issue: ./coding-agent-loop -no-mutate -once (runs Claude for real, pushes nothing), then inspect the transcript:
      jq -c 'select(.type=="system") | {tools: (.tools|length), mcp: (.mcp_servers//[]|length)}' ~/.agent-loop/logs/<run>.jsonl | head -1
      jq  'select(.type=="result") | .usage' ~/.agent-loop/logs/<run>.jsonl
      The first line confirms --strict-mcp-config took effect (zero MCP servers); the second gives the four-way usage split to compare against a pre-change run on the same issue.
    5. Open /ui: confirm the run row and detail page show the in (new · written · cached) / out breakdown, and that an existing run recorded before the migration renders 0 new · 0 written · 0 cached rather than breaking.

    Risks and decisions for the reviewer

    • --disable-slash-commands also disables skills. If a target repository ships .claude/skills the agent is expected to use, this silently removes them. The harness has never invoked a skill explicitly, so this is judged safe — but with no config knob it is now unconditional, so it is worth a conscious yes.
    • These flags are unconditional and boolean flags cannot be un-set from extra_args. --autocompact is a value flag and a second occurrence in extra_args will very likely win (commander is last-wins), but --strict-mcp-config and --disable-slash-commands cannot be turned back off. An operator who needs MCP in agent runs adds --mcp-config <file> to extra_args, which --strict-mcp-config then honours. This is the accepted cost of "baked in, no overrides."
    • --autocompact 200000 changes behaviour on long runs: earlier context gets summarized rather than carried verbatim. That is the trade the issue asks for, but it can cost quality on a long implement run. 200K is conservative relative to the CLI's own default; it is a one-token edit in runner.go if it proves too tight.
    • --setting-sources is deliberately not passed. Restricting it would cut the most ambient context of all, but user-level settings can carry auth helpers, permission rules and env that a bypassPermissions daemon run may depend on. With no config escape hatch, turning it off would be an unconditional risk of breaking every run on the operator's machine in a hard-to-diagnose way. Left at CLI default.
    • Filtering agent comments also hides prior failure comments from the model. A retry no longer sees why the last attempt failed. I judge that an improvement (a clean retry beats one anchored on a stale failure), but it is a real behaviour change.
    • Redefining tokens_in was considered and rejected. Making it mean "fresh input only" would give a more honest headline number but silently changes the meaning of every existing row. The plan keeps the total and adds the breakdown. Say so if you would rather have the cleaner semantics and accept the discontinuity.
    • Retry behaviour is deliberately untouched. retryDelay (loop.go:384) caps back-off at retry_backoff_max (24h) and never gives up, so a permanently-broken issue costs a full run every day forever. Capping that would contradict the documented "the trigger label is the only thing that decides whether it is worked" principle, so it is flagged rather than changed. If usage matters more than that principle, the baked-in version is: after N consecutive failures, drop the trigger label and say so in the failure comment.
    • The plan/implement split inherently pays for exploration twice — the implement run cannot see the plan run's session (--no-session-persistence, and a human gate sits between them). Nothing here changes that; a shared-session design would be much larger and would weaken the "the plan a human approved is the one on the issue" guarantee.
    • Minor, pre-existing: truncate (prompt.go:288) slices by bytes and can split a UTF-8 rune. Harmless today and out of scope, but the constants being lowered makes it fire more often; a for !utf8.ValidString(...) back-off is a two-line fix if you want it folded in.

    Reply with exactly implement to approve this plan and start the change. Reply with anything else and the plan will be revised to address it.

    coding-agent-loop run 5a7f4f64-e22a-45a8-9379-84bfd3c1a05b, model claude-opus-5, cost $1.5158

  4. ableinc commented on Aug 30, 2026

    @ableinc
    OwnerAuthor

    implement

  5. ableinc commented on Aug 30, 2026

    @ableinc
    OwnerAuthor

    Opened a draft pull request for this issue: #19

    Tests failed (make test) — see the PR for output.

    Comment implement again if you want another attempt at this issue.

    coding-agent-loop run b328b7bb-cc7a-451c-b698-00fbcad54205

  6. added a commit that references this issue on Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions