A time in no particular zone, and a preview missing what it previews
The first audit pass: everything from 0.8.1 to 0.9.8 read as a whole rather than one feature at a time, starting with what a model is actually told. Four of these had shipped as correct. The date line carried a timezone variable that resolves to nothing until somebody chooses one -- so every default account was told times were "in unless they say otherwise", while two comments asserted the line disappeared instead. The prompt preview built its variables without a chat, which is what eleven fragments are gated on, so the whole agent surface was absent from it whatever was ticked. Plan mode was instructed to keep its plan current with a tool that mode withdraws. And knowledge_get returned a document whole where every sibling reader caps and says so, its description promising exactly that. The subagent guidance was wrong in both directions at once: it denied a documented parameter and named seven of twenty-three allowed commands. Both halves are pinned by tests against the real list and the real schema now, because prose and a constant drift the moment one is edited alone. docs/notes/audit-0.9.md carries the findings that are not fixed here, with why -- the ones whose fix would change what a feature does are the user's call, not this pass's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,231 @@
|
||||
# The 0.9 audit
|
||||
|
||||
A working document, written during the pass over everything that landed between
|
||||
`0.8.1` and `0.9.8` -- 16 commits, 163 files, +16,795 / −1,608. It exists so the
|
||||
findings survive the session that produced them, and so the security and testing
|
||||
stages have something to work from rather than re-deriving it.
|
||||
|
||||
**This file is temporary.** Everything in it either becomes a `CHANGELOG.md`
|
||||
line, a paragraph in `CLAUDE.md`, or an entry under "After 1.0.0" in `PLAN.md`.
|
||||
It is deleted before the release.
|
||||
|
||||
Each finding carries a disposition:
|
||||
|
||||
- **fixed** -- landed, with a test
|
||||
- **later** -- real, but the fix changes what a feature does, so it is the
|
||||
user's call and it is written up rather than done
|
||||
- **stated** -- correct as it stands, and recorded so the next audit does not
|
||||
spend an hour rediscovering that it is deliberate
|
||||
|
||||
---
|
||||
|
||||
## The harness and the instruction prompts
|
||||
|
||||
### {{timezone}} shipped as a hole — *fixed*
|
||||
|
||||
`clock.name_for` returns `""` for anybody who has never chosen a zone, which is
|
||||
the default state of every account. `substitute` drops a line only when the
|
||||
whole line is **blank** after expansion, and this variable sits inside a
|
||||
sentence, so every such request carried:
|
||||
|
||||
```
|
||||
- Times the person gives you are in unless they say otherwise.
|
||||
```
|
||||
|
||||
Both the comment at `harness.py:191` and the fragment's own hint asserted the
|
||||
line disappeared. Neither had ever been true. The sibling call site
|
||||
(`schedule/compile.py:102`) has always had the fallback.
|
||||
|
||||
Worth noting *why* the fallback is right rather than merely convenient: `stamp`
|
||||
is `clock.now_for(user)`, which already falls back to the server's zone, and
|
||||
`{{now}}` already prints its offset. The times were already in that zone.
|
||||
Withholding the label from a value the model had been given was not restraint.
|
||||
|
||||
### The admin preview could not show most of what it previews — *fixed*
|
||||
|
||||
`admin_prompts.py` built its variables with `context_variables(db, user, [], None)`
|
||||
— no tools, no chat — and backfilled six names. Every other `requires` gate
|
||||
stayed empty, so **eleven fragments could never appear in a preview whatever an
|
||||
administrator ticked**: the whole agent surface, both scheduling fragments and
|
||||
the helper warning. Editing `tool.agent` and pressing preview showed a system
|
||||
message with `tool.agent` missing from it, silently.
|
||||
|
||||
Fixed with sample values, which is what `SAMPLE_DOCUMENTS` already did for
|
||||
attachments, plus two controls the "Tools offered" row could not reach — "This
|
||||
chat is" (ordinary / scheduled task / helper) and an agent-mode select. The
|
||||
samples are gated **exactly as `context_variables` gates the real values**,
|
||||
because a preview that admits a fragment the real request would not is worse
|
||||
than one that omits it.
|
||||
|
||||
### Fragments named tools that had been withdrawn — *fixed for Plan mode, later for the rest*
|
||||
|
||||
`resolve_tools` filters per **tool**; `harness._families` re-derives gates per
|
||||
**family**. So a family survives on its readers while its writers are gone, and
|
||||
the family-gated fragment still names them — under `core.tool_list` saying
|
||||
"Anything not named there does not exist here — calling it costs a round and
|
||||
returns nothing."
|
||||
|
||||
**Fixed:** Plan mode. `agent/tools.py:1185` withdraws `plan_update` there, and
|
||||
`tool.plan_update` was gated on `{{plan}}`, which is set whenever a plan exists
|
||||
in any mode. A new `plan_editable` variable — the plan text, blank in Plan mode
|
||||
— now gates it. Writing the test found a second instance the audit had missed:
|
||||
`context.plan` also said "Change it with plan_update", so that sentence moved to
|
||||
the fragment whose job it is.
|
||||
|
||||
**Later:** a read-only helper keeps seven fragments naming fifteen withdrawn
|
||||
write tools (`notes_create`, `report_write`, `file_write`, `skill_create`, the
|
||||
three schedule writers…). The principled fix is the split `tool.skills` /
|
||||
`tool.skills_write` already demonstrates, applied to `notes`, `report`,
|
||||
`schedule` and `agent_edits` — four more fragment pairs. That is a prompt
|
||||
restructure rather than a bug fix and it is proposed for after 1.0.0. The cost
|
||||
today is bounded: `{{tool_names}}` is authoritative and the model has it, so a
|
||||
helper wastes at most one round discovering a writer is absent.
|
||||
|
||||
### Three tool descriptions disagreed with their runners — *all fixed*
|
||||
|
||||
- **`subagent_run` opened "a second model with the same tools"** and contradicted
|
||||
itself two sentences later. The child is forced to `ask:False`,
|
||||
`subagent:False`, `write:False` with commands pinned to `SAFE_COMMANDS`. The
|
||||
leading clause is what a model reads when sizing a task.
|
||||
- **`knowledge_get` returned `extracted_text` whole** while every sibling reader
|
||||
capped and said so — `fetch` at 20k, `file_read`, the memories block, the
|
||||
skill index, the project listing. `MAX_EXTRACTED_CHARS` defaults to 120,000,
|
||||
so one call on a long PDF filled an ordinary window with nothing reporting it.
|
||||
Its description said "in full", which is why this read as correct: the tool did
|
||||
exactly what it claimed. Now `MAX_DOCUMENT_CHARS = 40_000`, cut with the model
|
||||
told, which is what `fetch` and `file_read` both do.
|
||||
- **`_run_ask_user` read `args["question"]`**, singular, against a schema
|
||||
declaring `questions` and a list — so its event always carried an empty
|
||||
`query`. Harmless only because the path is documented unreachable, which is
|
||||
exactly why nothing caught it.
|
||||
|
||||
### Fragment text that was false — *fixed*
|
||||
|
||||
`tool.subagent_agent` said a helper "reads and reports … and nothing else, in
|
||||
every mode" beside a `write` parameter that makes one write files, and named
|
||||
**seven** of the **twenty-three** commands in `SAFE_COMMANDS` — so a model
|
||||
avoided commands it was allowed, which costs nothing visible and is therefore
|
||||
never reported. Both halves are now pinned by tests against the real list and
|
||||
the real schema, because prose and a constant drift the moment one is edited
|
||||
alone.
|
||||
|
||||
The distinction the wording now has to carry, and did not before: **commands**
|
||||
are fixed to the read-only list in every mode, since an unattended chat cannot
|
||||
approve anything; **files** may be written, but only by a helper asked for with
|
||||
`write` and only from Edit or Auto.
|
||||
|
||||
### `tool.background` promises a notification that can be off — *later*
|
||||
|
||||
"When a background job finishes you are told in a new turn" has no `requires`
|
||||
for `agents.background_notify`, and the runner itself branches on that flag and
|
||||
says "Check on it with `job_output`" instead. One fragment, two behaviours. The
|
||||
fix is a variable and a gate, the same shape as `plan_editable`; deferred only
|
||||
because it wants the split above rather than a third one-off.
|
||||
|
||||
### `FAMILY_ASK` has no fragment at all — *later*
|
||||
|
||||
Every other family in `FAMILIES` contributes one. `ask_user` is offered in
|
||||
nearly every chat and **all** of its guidance lives in its schema description,
|
||||
which is the one thing an administrator cannot edit. Proposed rather than done:
|
||||
it is new prompt text, and new prompt text in an audit stage is how an audit
|
||||
turns into a feature.
|
||||
|
||||
### Smaller, all *stated*
|
||||
|
||||
- **`model_name` is resolved on every request and rendered by no shipped
|
||||
fragment.** It is documented in `VARIABLES` and available to an
|
||||
administrator's override, so it is an escape hatch rather than dead work. The
|
||||
cost is one small `select` on a table with tens of rows.
|
||||
- **Four pairs of fragments share an `order`**, which undercuts "sparse so a
|
||||
custom tool can be slotted between two of these". Ties break alphabetically
|
||||
and deterministically. One has a readable consequence: `context.schedule`
|
||||
sorts below `context.memories`, so a scheduled task's own instruction appears
|
||||
under the reader's remembered facts.
|
||||
- **`core.rounds` is dark on a stock install**, since `DEFAULT_CHAT_ROUNDS = 0`.
|
||||
Deliberate — `core.keep_working` is its complement and exactly one ever fires.
|
||||
- **A row-backed fragment with empty `guidance` can never be selected**, so for
|
||||
those "never configured" and "deliberately switched off" are the same state —
|
||||
the one distinction the absent-vs-empty convention exists to preserve.
|
||||
|
||||
---
|
||||
|
||||
## Controls
|
||||
|
||||
The four sweeps `CLAUDE.md` is shaped around were run by hand over every
|
||||
template and **all four came back clean**: 68 htmx verbs against 179 registered
|
||||
routes with zero method mismatches, 88 plain form actions and 30
|
||||
`fetch`/`htmx.ajax` call sites with zero, every verb-carrying element declaring
|
||||
its own `hx-target` or `hx-swap="none"` checked against every ancestor, no empty
|
||||
verb attributes, both `form="…"` sites correct, and every `hx-target`,
|
||||
`hx-include` and `from:` selector resolving to an id that exists.
|
||||
|
||||
Making each one a **test that sweeps** is Stage 2's work. A one-off audit that
|
||||
finds nothing is worth much less than the same audit made permanent.
|
||||
|
||||
## Capabilities that are built, documented and unreachable — *Stage 2*
|
||||
|
||||
The `Model.params_json` and `library.share` family: the code is right, the route
|
||||
validates, and nothing can reach it.
|
||||
|
||||
- **Folder nesting.** `PATCH /api/folders/{id}` handles `parent_id` with a cycle
|
||||
guard and `MAX_DEPTH = 8`, `partials/_folder.html` recurses, and `README.md`
|
||||
advertises "arbitrarily nested". No template or script submits `parent_id`.
|
||||
- **Moving a chat into a folder.** `api/chats.py` handles `folder_id` on PATCH;
|
||||
only the composer submits it, at creation. `chat/_composer.html` even carries
|
||||
a comment reasoning about "the only way into a folder was to make the chat
|
||||
elsewhere and move it".
|
||||
- **`Connection.extra_headers_json`** is read into every LLM request and written
|
||||
by no form, so its documented use — OpenRouter's `HTTP-Referer` — is
|
||||
unreachable. *later*: nothing advertises it, so nothing is currently untrue.
|
||||
|
||||
Written and never read: `Chat.compacted_at`, `User.last_login_at`,
|
||||
`Schedule.last_fire_at`, `Schedule.compiled_at`. Neither read nor written:
|
||||
`general.require_approval` (its comment says "reserved"), and the `back` form
|
||||
parameter on two `admin_models` routes.
|
||||
|
||||
*Stated:* `Message.parent_id` and `content_parts_json` are deliberate forward
|
||||
-looking columns; `Chat.archived` is already recorded in `PLAN.md` as a column
|
||||
nothing surfaces.
|
||||
|
||||
## Documentation — *Stage 2*
|
||||
|
||||
- `README.md` is stale by six phases. "Planned: Image generation · OCR ·
|
||||
semantic search" — two of the three shipped. Nothing under Features mentions
|
||||
schedules, reports, subagents, branding, quotas, web push, updates, Docker or
|
||||
LXC.
|
||||
- `CLAUDE.md` says "Three topics live in `docs/notes/`" and lists seven.
|
||||
- `CLAUDE.md` says 2088 tests; there are 2092 before this stage.
|
||||
- `tests/__pycache__/test_zz_{dump,live}*.pyc` are stale bytecode for two files
|
||||
that no longer exist.
|
||||
|
||||
## For the security stage
|
||||
|
||||
Carried forward rather than answered here:
|
||||
|
||||
- the three hand-rolled redirect loops each re-run `check_url` per hop
|
||||
(confirmed); does each also drop the secret when a hop leaves its origin?
|
||||
- `admin_tools.py` calls `check_url(server.url, allow_private=True)`
|
||||
unconditionally at save — a syntax check, with the per-row flag governing at
|
||||
call time. Confirm that reading is right.
|
||||
- the subagent restriction is the one gate that fails open unattended.
|
||||
`_child_scope` forcing `ask:False`, `subagent:False`, `write:False` and
|
||||
pinning `allow` to `SAFE_COMMANDS` is confirmed present; the question is
|
||||
whether every path reaches it.
|
||||
- `pyproject.toml` pins no upper bounds and `deploy/update.sh` runs
|
||||
`pip install -e` on every update, so a breaking upstream release arrives on a
|
||||
button press. pip's `only-if-needed` default limits the blast radius, which is
|
||||
why this is a note rather than an emergency.
|
||||
|
||||
## For the testing stage
|
||||
|
||||
- `api/library.py`: 28 routes, effectively zero HTTP coverage. Highest exposure
|
||||
— auth, ownership and permission gates all live at that boundary.
|
||||
- No real `sync_schema` upgrade test exists. `conftest.py` runs `create_all`
|
||||
then `sync_schema`, so the differ is only ever exercised as a no-op. Hard rule
|
||||
4 has no other safety net.
|
||||
- `cli.py`: zero tests, and `create-admin` is the documented bootstrap.
|
||||
- SSE has three end-to-end tests; `services/sse.py` has none.
|
||||
- `composer.js`, `terminal.js` and `audio.js` — 1,188 lines — have no assertion
|
||||
of any kind.
|
||||
- The suite has only ever run on Python 3.14. The Dockerfile ships 3.12 and
|
||||
`requires-python` claims 3.11.
|
||||
Reference in New Issue
Block a user