fix: name list filter flags for what they actually do - #58
Merged
Conversation
|
| Filename | Overview |
|---|---|
| internal/cmd/pagination.go | Adds shared registration and resolution helpers for renamed list filters and their deprecated alias. |
| internal/cmd/pagination_test.go | Covers artifact, instance, and function-run filter mappings along with alias visibility and pagination behavior. |
| internal/cmd/agents.go | Maps --all to the agents endpoint’s inverse only_active parameter and explicitly sends only_saved. |
| internal/cmd/functions.go | Maps artifact and run filters separately, making full run history the default. |
| internal/cmd/sessions.go | Replaces --only-active with --all while preserving running-only default behavior. |
| internal/cmd/personas.go | Replaces the artifact filter with --include-deleted while continuing to hide deleted personas by default. |
| internal/cmd/vaults.go | Replaces the artifact filter with --include-deleted while continuing to hide deleted vaults by default. |
| README.md | Documents the renamed resource-specific list flags and function-run history behavior. |
Reviews (2): Last reviewed commit: "fix: name list filter flags for what the..." | Re-trigger Greptile
The API exposes one `only_active` parameter, but it means a different thing per resource. Verified against prod by creating, deleting and stopping records: functions / vaults / personas active == not deleted sessions / agents active == still running function runs active == still executing A single --only-active flag covering all three is unlearnable - whatever a user infers from `sessions list` is wrong on `functions list`. Each resource now gets a flag named after what it does: functions|vaults|personas list --include-deleted (default: hide deleted) sessions|agents list -a, --all (default: running only) functions runs --running (default: full history) --only-active is kept as a deprecated, hidden alias that maps straight onto the API parameter, so existing scripts keep working. Only one behaviour changes: `notte functions runs` now returns the full history. Defaulting it to active-only made run history permanently empty - it returned [] for a function whose two runs had both completed, which reads as "this function never ran" and misleads anything diagnosing a failure. That is what surfaced this. Everything else keeps today's defaults. In particular `functions list`, `vaults list` and `personas list` continue to hide deleted records - an earlier draft of this change would have started listing soft-deleted rows by default, which is why the semantics were checked per resource rather than assumed. Separately, the filter value is now sent on every request. Previously the CLI only sent it when cobra reported the flag as Changed, which left the server's default in charge of the CLI's documented behaviour - the reason an omitted --only-active filtered at all despite defaulting to false. Sending it always keeps these defaults authoritative and immune to a server-side change. Pagination keeps the opposite convention and stays absent when omitted so the API applies its own page defaults; pagination_test.go covers both rules so they don't get unified by mistake. Verified against prod with a locally built binary: functions list 64 --include-deleted 100 (deleted hidden) sessions list 0 -a 100 agents list 0 --all 100 functions runs 2 --running 0 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
giordano-lucas
force-pushed
the
fix/only-active-default
branch
from
August 5, 2026 16:42
28752dd to
868b29b
Compare
greptile-apps
Bot
dismissed
their stale review
August 5, 2026 16:42
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Member
Author
|
@greptile new review the intend of the PR has change considereably |
giordano-lucas
added a commit
to nottelabs/notte-skills
that referenced
this pull request
Aug 5, 2026
Replaces the previous commit's guidance, which was wrong. It said --only-active was uniformly inverted and told agents to pass --only-active=false everywhere to get complete results. Checking each resource against the API - by creating, deleting and stopping real records - shows the parameter means three different things: functions / vaults / personas active == not deleted sessions / agents active == still running function runs active == still executing So the blanket advice was actively harmful for artifacts: widening those listings surfaces soft-deleted records, and acting on a deleted Function or vault id fails confusingly. Their defaults were correct all along. A new "Filters on list commands" section in notte-browser states the three meanings and the two rules that follow: don't widen artifact listings by reflex, do widen run listings. Per-command flag lines now say what their default hides, so the semantics are visible at the point of use rather than only in one table. The genuine problem is unchanged and still documented: `notte functions runs` lists [] once every run has finished, so an empty run list must never be read as "this Function never ran". notte-functions-doctor depends on this in Phase 2, where it recovers the last good run to learn what correct output looks like. Doctor now also checks --include-deleted before treating a missing Function as broken, since a deleted Function is a report, not a repair job. Guidance is written to hold on every CLI version: --only-active=false works everywhere, while newer CLIs use --include-deleted, -a/--all and --running (nottelabs/notte-cli#58, which renames the flags per semantic and keeps --only-active as a deprecated alias). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
profiles list never sent only_active at all - the flag was not registered - so the API's active-only default silently hid deleted profiles with no way to see them. Same class as the other artifact listings, just with the flag missing rather than mis-defaulted. ProfileListParams already carries OnlyActive, so this is CLI-side only. Verified against prod: a profile created then deleted disappears from the default listing and reappears with --include-deleted; across the account the default shows 31 profiles where --include-deleted shows 40. Not adding the flag to `functions secrets list`: ListSecretsParams exposes only `namespace`, with no only_active parameter, so there is nothing for the CLI to send. If secrets are soft-deleted server-side, exposing them needs an API change first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
greptile-apps
Bot
dismissed
their stale review
August 5, 2026 16:59
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
giordano-lucas
added a commit
to nottelabs/notte-skills
that referenced
this pull request
Aug 5, 2026
* fix(notte): re-verify every documented command against CLI v0.0.29
Audited the notte plugin's three skills, seven references and three
templates against the `notte` CLI as it actually behaves, reading the CLI
source where `--help` did not settle the question.
Correctness:
- `notte functions run` blocks. function-management.md described it as
fire-and-forget with a `sleep 10` + `run-metadata` poll, while self-test.md
and both Function skills built their pass/fail protocol on the inline
`result`. The CLI issues one synchronous POST with no client-side polling.
- Because it is synchronous it is bounded by the global `--timeout` (60s), so
a slow Function fails the command while the run continues server-side. This
reads as a broken Function and was documented nowhere.
- One name for file storage (`use_file_storage=True`); it was written three
ways across four files.
- A Function file needs a module-level `run()` call. Examples disagreed;
without it the run returns `null` with no error.
- Four examples called `len()` on a scrape result to build a `count`, counting
dict keys rather than rows.
- Dropped `page observe <url>` (takes no arguments), `--browser-type firefox`
(unsupported), the invented `--max-steps` default, and the 30s timeout
default (it is 60).
Security:
- The skill warned that `--password` leaks via argv, then prescribed
`--password "$VAR"` as the fix; the shell expands that before exec, so `ps`
sees the plaintext either way. Now states what the env-var form does and
does not buy you, and directs callers to load a credential once and rely on
the vault after that.
- templates/authenticated-session.sh pulled the plaintext password out of the
vault and passed it as an argv to `notte page fill` - the exact leak the
skill warns about, and a defeat of the sentinel mechanism. Rewritten to
attach the vault and fill sentinels.
- Reconciled the two contradictory accounts of how vault credentials reach the
page; the sentinel mechanism is the correct one.
Coverage:
- Documented `notte search`, `notte profiles`, `notte functions secrets`, the
bundled MCP servers, and the new `sessions start` flags.
- `anything-api` is a marketplace of ready-made Functions, so forge now checks
it before paying to explore a site.
- `--create-phone-number` is gated per-account: documented that it will fail,
that it is an entitlement rather than a transient error, and how to request
access.
- Widened `allowed-tools`, which restricted all three skills to `Bash(notte:*)`
while instructing them to run curl, jq and diff.
Both validators pass; shell templates pass `bash -n` and every Python fence
parses.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(notte): guard run() with __name__ == "__main__" to stop double execution
Corrects a wrong rule I introduced in the previous commit. I had documented
that a Function file "must end with a module-level run() call", inferring it
from the fact that the workflow-code export and the skeleton both ended that
way. That inference was wrong, and the justification attached to it - that a
run which is only defined returns a null result - was never verified.
The Notte runtime imports the workflow file and calls run() itself. An
unguarded module-level run() therefore fires twice: once during the import,
once from the runtime. For a browser Function that means two sessions, double
the cost, and every side effect - a form submission, a purchase, a write -
performed twice.
Every example now uses:
if __name__ == "__main__":
run()
which stays single-shot in the cloud while keeping the file directly runnable
for local testing. `notte sessions workflow-code` emits an unguarded call, so
adding the guard is now documented as part of cleaning the export, alongside
removing `from __future__ import annotations`.
Updated in the skill, the function-management and interop references, the
skeleton template, the Cursor rules, and the changelog.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Revert "guard run() with __name__ == '__main__'" and drop the run() rule entirely
Reverts 72e6dc8. The double-execution behaviour it guarded against belonged to
an earlier version of the Notte function runtime and no longer applies, so the
`if __name__ == "__main__":` wrapper is unnecessary.
Does not restore the claim from 4485a9c that a Function file "must end with a
module-level run() call" - that was equally wrong. A plain revert would have
put it back.
The truth is that it makes no difference: the runtime invokes run() itself, so
a trailing module-level call can be present or absent. The examples keep the
call to match what `notte sessions workflow-code` emits, and the reference,
interop notes, skeleton, forge cleanup step and rules now say explicitly that
it is optional - so the next reader does not infer a rule from the fact that
some files have it and others do not, which is how both wrong versions of this
got written.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(notte): correct run-result docs from live Function runs
Deployed and ran three throwaway Functions against prod to replace inference
with measurement, then deleted them. Findings:
1. The trailing run() call really is irrelevant, as reported. A Function with
no module-level call returned "Done" normally, so the runtime invokes run()
itself. A Function *with* a trailing call logged "INVOCATION #1" exactly
once, so there is no double execution either. The wording added in the
previous commit is now verified rather than asserted.
2. `functions run` returns the value of run() serialized to JSON - a dict comes
back as a real nested object. The "may arrive as a JSON-encoded string, so
parse defensively" hedge I added was wrong and is gone. `run-metadata`'s
result *is* a Python repr, confirming what the docs already said, so contract
validation should read `functions run` and use run-metadata only for logs.
3. `notte functions runs` returned an empty array for a Function with three
completed runs. Anything that discovered a run id through that list would
silently fail, so the skills now take `function_run_id` straight from the
`functions run` response. notte-functions-doctor treats empty run history as
uninformative rather than as evidence a Function never worked.
4. `functions run` returns no logs; they come from run-metadata.
Point 3 looks like a platform bug rather than a docs problem - flagged on the PR.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(notte): --only-active is inverted on every list command
Ran down the empty-runs-list finding from the previous commit. It is not that
run history is missing - it is that omitting --only-active filters it out.
The CLI registers --only-active with a Go default of false ("Only return active
X", i.e. an opt-in filter) but only sends the parameter when cobra reports the
flag as Changed. With the parameter absent the API applies its own default,
which is active-only. So the flag's documented semantics are the opposite of
what omitting it does.
Measured on prod:
notte functions runs --function-id <id> -> []
notte functions runs --function-id <id> --only-active=false -> both runs
notte sessions list -> 0
notte sessions list --only-active=false -> 10
notte vaults list -> 6
notte vaults list --only-active=false -> 10
Affects sessions list, agents list, functions list, functions runs,
personas list, and vaults list - every command using this flag pattern.
Skills now pass --only-active=false where completeness matters and warn against
reading an empty list as "nothing exists". The sharpest case was
notte-functions-doctor Phase 2, which recovers the last good run to learn what
correct output looks like: empty history would have read as "this Function never
worked" and pushed the diagnosis to the wrong failure class.
Replaces the previous commit's weaker guidance ("do not rely on functions runs"),
which described the symptom without the cause or the workaround.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(notte): "active" means something different on each list command
Replaces the previous commit's guidance, which was wrong. It said --only-active
was uniformly inverted and told agents to pass --only-active=false everywhere to
get complete results. Checking each resource against the API - by creating,
deleting and stopping real records - shows the parameter means three different
things:
functions / vaults / personas active == not deleted
sessions / agents active == still running
function runs active == still executing
So the blanket advice was actively harmful for artifacts: widening those
listings surfaces soft-deleted records, and acting on a deleted Function or
vault id fails confusingly. Their defaults were correct all along.
A new "Filters on list commands" section in notte-browser states the three
meanings and the two rules that follow: don't widen artifact listings by reflex,
do widen run listings. Per-command flag lines now say what their default hides,
so the semantics are visible at the point of use rather than only in one table.
The genuine problem is unchanged and still documented: `notte functions runs`
lists [] once every run has finished, so an empty run list must never be read as
"this Function never ran". notte-functions-doctor depends on this in Phase 2,
where it recovers the last good run to learn what correct output looks like.
Doctor now also checks --include-deleted before treating a missing Function as
broken, since a deleted Function is a report, not a repair job.
Guidance is written to hold on every CLI version: --only-active=false works
everywhere, while newer CLIs use --include-deleted, -a/--all and --running
(nottelabs/notte-cli#58, which renames the flags per semantic and keeps
--only-active as a deprecated alias).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(notte): correct the scrape -o json shape from a live scrape
exploration.md documented the parsed extraction as living under
`.structured.data`, with the top level being `{markdown, structured}`. Ran it
against a real page:
scrape --instructions "Extract heading and subheading as JSON" -o json
-> {"heading": "...", "subheading": "..."} # top level, no wrapper
scrape (no --instructions) -o json
-> {"markdown": "..."} # nothing else
So the extracted object is returned directly and there is no wrapper to unpack.
The claim predates this branch, but templates/data-extraction.sh was rewritten
earlier in this PR to unwrap `.structured.data`, which never matched; it worked
only because the `// .[1]` fallback caught every case. Both merge steps now use
the scrape result directly and say why.
Also verified while checking this, no changes needed:
- vault sentinel substitution works as documented. Filling `cooljohnny1567` /
`mycoolpassword` on a session started with --vault-id logged in successfully;
the same fills without --vault-id failed with "Your username is invalid!",
confirming --vault-id is what enables substitution. The CLI echoes the
sentinel rather than the resolved secret, so it does not leak to the terminal
- observe returns I1/I2 for inputs and B1 for the submit button, matching the
documented ID convention
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(notte): a command timeout is not a failed run - never retry it
Review catch: the self-test failure table told the agent to answer a command
timeout by re-running with --timeout 600. Since the original invocation is
still executing, that starts a second concurrent run and repeats any
non-idempotent side effect - a form submission, a purchase, a write. The skill
that carries this guidance is the same one whose confirmation gates single out
"purchases, anything that writes", so the advice actively contradicted its own
safety model.
Confirmed the premise rather than assuming it. A Function needing ~45s,
invoked with --timeout 10:
command -> {"error":"API request failed: ... context deadline exceeded"}
run -> status "active" immediately after the client gave up
~35s on -> status "closed", result {'finished': True}
So the client disconnecting does not cancel the run; it completes normally.
Retrying genuinely double-executes.
Guidance is now split. Setting a generous timeout up front is the fix
(--timeout 600 on the first invocation) and stays recommended. Reacting to a
timeout is different: do not re-run, find the in-flight run instead. The
active-only default on `notte functions runs` - called out elsewhere in this PR
as a trap for reading history - is exactly the right filter for spotting a run
still in flight, so the recovery path is a single command.
Applied to self-test.md (failure table plus a new section), the forge Phase 4
note, notte-browser's long-running Functions note, and function-management.md.
notte-functions-doctor gets a related caution: it re-runs a Function to
reproduce a failure, which has the same hazard when the workflow writes
something. It now says to check the downloaded workflow file for side effects
first, prefer reading the last failed run from history, and confirm with the
user before invoking - it operates on live, possibly scheduled Functions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
giordano-lucas
added a commit
to nottelabs/notte-skills
that referenced
this pull request
Aug 5, 2026
…ll links
Review follow-ups on notte-functions-doctor.
1. Function lookup stopped at one page. Phase 1 used a single --page-size 100
request, so a Function past the first 100 read as missing. Measured the
actual constraints: the API caps page_size at 100 ("Input should be less
than or equal to 100" for 500), and the CLI prints a bare array with the
paginated response's has_next dropped, so a short page is the only
end-of-list signal. Phase 1 now ships an all_functions() helper that pages
until one arrives, and says never to conclude a Function is missing from a
single request. Phase 5 reuses the same helper.
Same pass caught a flag that does not exist yet: the deleted-Function check
used --include-deleted, which ships in nottelabs/notte-cli#58 and is still
open. `notte functions list --include-deleted` fails on the released CLI.
Switched to --only-active=false, valid on every version, with the newer flag
named as an alternative.
2. Throwaway verification copies were matched by display name, so two repairs
of Functions sharing a name could collide: `head -1` picked one without
proving ownership, reuse would overwrite it, and the prefix-only cleanup
guard would then delete it. The throwaway is now named after the live
Function's id - unique and immutable where a display name is neither. Reuse
matches that name exactly and aborts rather than guessing when several
match; the cleanup guard is an exact match, not a prefix.
3. Regression coverage. The behaviours changed here are prose instructions to a
model, not code paths, so there is no unit to test - and a blanket bash-lint
of skill fences is not viable either: 25 of 151 are flag-reference blocks
that are documentation notation rather than runnable shell. What is testable
is the failure mode a rename actually causes, so validate-plugins.py now
checks that every relative markdown link inside a skill resolves. Confirmed
it works by pointing a doctor link back at notte-functions-forge, which the
check rejects. The four runnable shell blocks added here pass bash -n.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
giordano-lucas
added a commit
to nottelabs/notte-skills
that referenced
this pull request
Aug 5, 2026
…n both Function skills, reset versioning to 0.0.2 (#29) * feat(notte)!: rename notte-functions-forge to notte-functions-build, and fix 8 issues in both Function skills BREAKING CHANGE: the skill is invoked as /notte-functions-build (Claude Code) and $notte:notte-functions-build (Codex). --- The rename --- "Forge" was a metaphor nobody types. The tell was in the skill's own description, which had to enumerate "forge, build, generate, or bake" to be discoverable - the name was not carrying itself, and someone reaching for a slash command guesses "build". The docs carried the metaphor throughout ("forged Function", "forging", forged_function.py), which a reader has to decode. "Build" also matches the vocabulary of the bundled anything-api MCP server, whose `build` tool does the same job from a natural-language description. Aligning them gives one concept one name across the plugin. notte-functions-doctor keeps its name: also a metaphor, but apt and unambiguous, and "repair"/"fix" would over-trigger on generic requests in a way "doctor" does not. Historical CHANGELOG entries keep the old name - they record what the skill was called at the time. --- Fixes found while re-reading both skills end to end --- build: - the marketplace check sat in Phase 0, ahead of the phase that works out the target and fields, so there was nothing to search for yet. Moved to Phase 1c, after intent is parsed and before the plan gate - Phase 3 said to *add* a run(...) entry point when the export already defines one; an agent could reasonably end up with two - Delivery promised "three ways" to invoke a Function and listed two - the --var example used a parameter absent from the worked example doctor: - Phase 1 called a bare `notte functions list`, which pages at 10 - on a busy account the Function being repaired simply would not appear - the "it may have been deleted rather than broken" check was in Phase 2, too late to be useful; moved to Phase 1 where the Function fails to turn up - [doctor-verify] throwaways leaked: cleanup only ran in Phase 6, after the contract passed AND the user approved, so an abandoned or rejected repair left copies behind. Phase 5 now reuses an existing throwaway instead of stacking new ones, and states the name-guarded delete runs however the repair ends - Phase 6 re-runs the live Function to confirm health, which writes again if the Function writes; it now defers to the user for those Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(notte): reset versioning to 0.0.2 The plugin was never published. The 1.x numbers in the changelog were reconstructed from git log rather than shipped, and no release was ever tagged, so nothing downstream depends on them. Carrying a 2.0.0 forward would imply a release history that does not exist and would make every future rename or restructure look like a major event. Under 0.x, breaking changes are expected and do not need a major bump (semver spec item 4), which fits a plugin still settling its skill names and command surface. Changelog restructured to match: - versioning restarts at 0.0.x, explained in the preamble - 0.0.2 is this branch: the CLI v0.0.29 re-verification pass and the notte-functions-build rename, which were drafted as 1.6.0 and 2.0.0 before the reset and are one release now. Their two "Bug Fixes" lists are merged and the headless intro paragraph is gone - 0.0.1 collapses everything that existed before, since none of it was released either. The original 1.0.0-1.5.0 headings are demoted and kept underneath for provenance, with their numbers explicitly retired - the rename is filed under "Renames" rather than "BREAKING CHANGES": with nothing published under the old name there is nothing downstream to break, though anyone running from the default branch needs the new invocation Historical entries keep the old skill name - they record what it was called at the time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(notte): paginate Function lookup, key throwaways by id, check skill links Review follow-ups on notte-functions-doctor. 1. Function lookup stopped at one page. Phase 1 used a single --page-size 100 request, so a Function past the first 100 read as missing. Measured the actual constraints: the API caps page_size at 100 ("Input should be less than or equal to 100" for 500), and the CLI prints a bare array with the paginated response's has_next dropped, so a short page is the only end-of-list signal. Phase 1 now ships an all_functions() helper that pages until one arrives, and says never to conclude a Function is missing from a single request. Phase 5 reuses the same helper. Same pass caught a flag that does not exist yet: the deleted-Function check used --include-deleted, which ships in nottelabs/notte-cli#58 and is still open. `notte functions list --include-deleted` fails on the released CLI. Switched to --only-active=false, valid on every version, with the newer flag named as an alternative. 2. Throwaway verification copies were matched by display name, so two repairs of Functions sharing a name could collide: `head -1` picked one without proving ownership, reuse would overwrite it, and the prefix-only cleanup guard would then delete it. The throwaway is now named after the live Function's id - unique and immutable where a display name is neither. Reuse matches that name exactly and aborts rather than guessing when several match; the cleanup guard is an exact match, not a prefix. 3. Regression coverage. The behaviours changed here are prose instructions to a model, not code paths, so there is no unit to test - and a blanket bash-lint of skill fences is not viable either: 25 of 151 are flag-reference blocks that are documentation notation rather than runnable shell. What is testable is the failure mode a rename actually causes, so validate-plugins.py now checks that every relative markdown link inside a skill resolves. Confirmed it works by pointing a doctor link back at notte-functions-forge, which the check rejects. The four runnable shell blocks added here pass bash -n. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(notte): target CLI v0.0.30, drop the compatibility hedging notte-cli #58 and #59 merged and shipped in v0.0.30 two hours ago, so --include-deleted, -a/--all and --running are all released. Upgraded locally and re-verified against v0.0.30 rather than assuming: functions runs -> 2 --running -> 0 functions list -> 64 --include-deleted -> 100 sessions list -> 0 -a -> 100 sessions start --browser-type firefox -> rejected Two things this fixes. A live error: the deleted-Function check in doctor Phase 1 used --include-deleted while it was still unreleased, so it failed outright on v0.0.29. I had hedged the rest of the docs onto --only-active=false for portability, which was the right call an hour ago and is now just noise. A claim v0.0.30 invalidated: the recovery path for a timed-out run said the active-only default on `functions runs` surfaces in-flight runs. That was true before #58 and is false after it - the default is now the full history, and in-flight runs take --running. Corrected in notte-browser, function-management and diagnosis. Everything else simplifies. Run history is the default, so the "pass --only-active=false or history looks empty" guidance is gone from four files; artifact listings say --include-deleted directly; session and agent listings say -a/--all. The skills now state they assume v0.0.30, with `notte version` as the check, rather than translating flags per version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(notte): never adopt a verification copy by name Review catch. Making the throwaway's name unique was not the same as proving ownership, and I conflated the two. `[doctor-verify] {live function id}` is predictable, so a concurrent repair of the same Function - or anyone who typed that label - produces the identical name. The reuse branch treated a single name match as proof, overwrote that Function with repaired_function.py, and the cleanup then deleted it. One unrelated Function wearing the expected name was enough to destroy it. The previous round hardened this in the wrong direction: id-in-the-name, exact match, abort when several match. That narrows the window without closing it, because the evidence is still just a name. Doctor now creates its own copy every time and treats the id returned by `notte functions create` as the only proof of ownership - it is yours because you made it. Iteration uses that id, never a name lookup. Strays from an abandoned earlier attempt are reported to the user rather than adopted or deleted, since ownership cannot be established either way. That loses the cross-attempt reuse the earlier round added, which is the right trade: the failure it avoided was orphaned copies, and the failure it risked was destroying someone else's. The name check before the delete stays, demoted to a second belt against a stale variable and labelled as not being authorization. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(notte): correct changelog entry that still claimed throwaway reuse The leak entry said Phase 5 'now reuses an existing throwaway rather than stacking new ones', which the ownership fix in the previous commit removed. Reuse is gone: doctor creates its own copy and reports strays instead of adopting them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The API exposes a single
only_activeparameter, but it means a different thing per resource. Verified against prod by creating, deleting and stopping real records:only_activeactually meansfunctions,vaults,personas,profilesonly_active=falsesessions,agentsclosed, not deletedfunctions runs[]One flag named
--only-activecovering all three is unlearnable: whatever you infer fromsessions listis wrong onfunctions list.What changes
Each resource gets a flag named after what it does:
functions list,vaults list,personas list,profiles list--include-deletedsessions list,agents list-a,--allfunctions runs--running--only-activeis kept as a deprecated, hidden alias mapping straight onto the API parameter, so existing scripts are unaffected:The one behaviour change
notte functions runsnow returns the full history. Defaulting it to active-only made run history permanently empty — it returned[]for a function whose two runs had both completed, which reads as "this function never ran" and misleads anything diagnosing a failure. That's what surfaced this.Everything else keeps today's defaults. In particular
functions list/vaults list/personas listcontinue to hide deleted records.profiles listhad no filter flag at allprofiles listnever registered or sentonly_active, so the API's active-only default hid deleted profiles with no way to see them. Same class of bug as the rest, just with the flag missing rather than mis-defaulted.ProfileListParamsalready carriesOnlyActive, so this is CLI-side only.Verified: a profile created then deleted vanishes from the default listing and returns with
--include-deleted; account-wide the default shows 31 where--include-deletedshows 40.Not adding it to
functions secrets listListSecretsParamsexposes onlynamespace— there is noonly_activeparameter, so there is nothing for the CLI to send. If secrets are soft-deleted server-side, surfacing them needs an API change first.Also: the value is now always sent
Independent of the defaults. Previously the CLI only sent the parameter when cobra reported the flag as
Changed:With it omitted the server's default decided the CLI's documented behaviour — which is why an omitted
--only-activefiltered at all despite defaulting tofalse. Sending it on every request keeps these defaults authoritative and immune to a server-side change.Pagination keeps the opposite convention:
--page/--page-sizeare genuinely optional and stay absent when omitted so the API applies its own paging defaults.pagination_test.goasserts both rules so they don't get "unified" by mistake.Verification
Locally built binary against prod:
go build,go vet,gofmt -lcleango test ./internal/...— all 10 packages passinternal/cmd/pagination_test.gocovers all three semantics, the deprecated alias still winning when passed, the alias being hidden-but-parseable, and the pagination conventionFollow-up
The three-way meaning of
only_activeis really an API-side modelling issue — distinct parameters (or per-resource names) would let clients stop guessing. This PR makes the CLI correct and legible regardless of which default the API applies.🤖 Generated with Claude Code