Repository navigation
chore: unship ACL tools - #1111
Conversation
|
Claude (@claude) review |
|
Greptile (@greptileai) review |
|
Claude finished Nikhil (@shadowfax92)'s task in 1m 57s —— View job PR Review: chore: unship ACL toolsTodo List
SummaryThis is a clean, well-scoped removal PR. The ACL scoring module ( VerificationNo orphaned references found. A broad ACL was never wired into the runtime. Confirmed by checking Package Changes
Dependency footprint reduction is significant. The removed tree includes:
The
|
✅ Tests passed — 1131/1136
|
Greptile SummaryThis PR removes the isolated ACL scorer reference module from the server package, along with its tests, fixtures, fixture runner, and the
Confidence Score: 5/5Safe to merge — this is a pure deletion of an isolated, unused reference module with no production runtime callers. All removed files were confirmed to have zero imports in the remaining codebase. The dependency and scripts are fully cleaned up. No runtime behaviour, CORS, or tool-registration logic is touched. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["apps/server (production)"] -->|"No import"| B["src/tools/acl/ ❌ DELETED"]
B --> C["acl-scorer.ts ❌"]
B --> D["acl-embeddings.ts ❌"]
B --> E["acl-edit-distance.ts ❌"]
B --> F["acl-stopwords.ts ❌"]
G["tests/ ❌ DELETED"] --> H["tools/acl-scorer.test.ts ❌"]
G --> I["__helpers__/acl-fixture-runner.ts ❌"]
G --> J["__fixtures__/acl/*.json ❌"]
K["package.json"] -->|"removed"| L["@huggingface/transformers ❌"]
K -->|"removed"| M["test:tools:acl script ❌"]
Reviews (1): Last reviewed commit: "chore: unship acl tools" | Re-trigger Greptile |
Greptile SummaryThis PR removes the entire ACL scorer subsystem — an isolated, unused feature for fuzzy/semantic element-blocking — from
Confidence Score: 5/5Safe to merge — this is a pure deletion of confirmed-unused code with no callers remaining in the codebase. All removed files were dead code: registry.ts never imported the ACL module, no other source file referenced it, and the @huggingface/transformers dependency was exclusively consumed by acl-embeddings.ts. The bun.lock update is consistent with the dependency removal. Nothing in the live request path is touched. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[apps/server] --> B[src/tools/registry.ts]
A --> C[src/tools/acl/ REMOVED]
C --> D[acl-scorer.ts]
C --> E[acl-embeddings.ts]
C --> F[acl-edit-distance.ts]
C --> G[acl-stopwords.ts]
E -->|depended on| H["@huggingface/transformers REMOVED"]
B -.->|no import confirmed unused| C
A --> I[tests/tools/acl-scorer.test.ts REMOVED]
A --> J[tests/__helpers__/acl-fixture-runner.ts REMOVED]
A --> K[tests/__fixtures__/acl/*.json REMOVED]
Reviews (2): Last reviewed commit: "chore: unship acl tools" | Re-trigger Greptile |
13:40:24 queen.choose - "Chose gHashTag/trios#1111 out of 22 open sub-issues" {"chosen": "browseros-ai#1111", "considered": "22", "inFlight": "1"} Twenty-two open sub-issues read, the one already in flight excluded, one named with a reason. The ordering is proven by the count rather than by reading: with a delegation running the command reports inFlight 1, without one it reports 0, so it is seeing a registry it could not have seen running before it. Three refusals on the way, each naming a mistake of mine. The bee answered "Outside my boundary. Not met" because it reads the issue body as the contract, and I had been refining the spec in comments while the body still held the criteria from when I first filed it. Then it explained the change correctly and edited nothing, because the range I named held the code to move and not the place to move it to. Then it spun on 25 tool calls across two files at once, and the Queen's own observer said so before I did. A spec is the issue body, it names both ends of a move, and it carries one boundary at a time. Closes gHashTag/trios#1140
15:14:25 queen.choose Chose gHashTag/trios#1111 out of 24 open sub-issues 15:14:26 queen.delegate Delegated gHashTag/trios#1111 to queen-swift 15:14:27 queen.transition browseros-ai#1111: queued -> running She read the epic, excluded what was already in flight, named one with a reason, and opened the work. Nobody typed the number. Proven the other way a minute later, without the approval: 15:16:18 queen.delegate Refused - browseros-ai#1111 not approved The consent rule holds and the refusal is in the record rather than only in the chat - that guard was silent yesterday. The proof needed two commands in one launch, because the probe wipes approvals at startup. Third time this epic that a capability existed and no run could reach it, and the third time the repair was a way to ask. Closes gHashTag/trios#1161 Closes gHashTag/trios#1160
Driven on the issue she actually picks:
12:21:14 queen.delegate Delegated browseros-ai#1111 to queen-swift
12:21:17 worker.start
12:21:18 queen.selftest.failed - "Delegation did not register a task"
The self-test resolves a task by the issue a human named, and /choose --start
leaves that empty. Choosing her own work and seeing work through had each been
proven; they had never been asked to hold together, and they did not.
The immediate failure is gone - the worker starts and the run continues past the
check that used to end it. The circle still does not close: the rule picks the
lowest-numbered open sub-issue, browseros-ai#1111 is architectural, and the bee produced
nothing before the app went away.
Also worth recording: lowest-numbered-not-in-flight reliably selects the oldest
item, which is the one everyone else skipped. Defensible rule, worst possible
first task.
Refs gHashTag/trios#1162
The rule was lowest-numbered-not-in-flight, which reliably picks the item everybody else skipped: browseros-ai#1111, architectural, and no bee ever came back from it. Task length against success rate is R² = 0.83 - near-certain under four human-minutes, under one in ten past four hours - so narrow and verifiable is where the success is, not a preference. The sort now counts the files named under the boundary heading, treats a directory as large, and uses the number only to break a tie. Driven both ways: by size: Chose browseros-ai#1117, fileCount 1 by number: Chose browseros-ai#1111, fileCount none One line of sort between them, same epic, same command. On a task that fits, the circle went further than it ever has with nobody naming anything: 14:27:50 queen.choose Chose browseros-ai#1117 out of 24 14:27:51 queen.delegate 14:30:28 Committed 1 file(s) 14:31:13 Reviewer returned 2 verdict(s) for 3 criterion(s) 14:31:14 running -> awaitingReview It stops there and should: two of three answered is not a pass. Closes gHashTag/trios#1163
Fourth instance of one shape in two days, and by now it is a class rather
than a bug: a condition the SELECTION could test is left for the REFUSAL to
find, and the tick ends there with a free worker slot.
already delegated -> filtered before the choice
boundary conflict -> filtered before the choice
accepted-but-unmerged -> filtered before the choice
no Границы section -> this one
She chose browseros-ai#1111 and refused it for having no boundary, with zero of four
workers running. The paths were already parsed during scoring, so the
condition was knowable one line earlier.
Worth stating as a rule for whoever meets the fifth: if the refusal can
name a reason, the selection can test it. A selection loop that learns its
constraints by being told no cannot advance past the first candidate that
violates one.
… one The first autonomous delegation into the cloud reached further than any before it: the Queen chose browseros-ai#1111 herself, cut the bee's worktree at /workspace/BrowserOS/trios/.worktrees/prod/queen-1111 in the container, and opened a turn. The turn died on Server error 403 at https://<host>/chat. Response: {"error":"Forbidden"} then, once the transport carried a bearer, on {"error":"Local authorization required"} Two failures, both classified `unmeasured`, and the issue was retired as exhausted - the retry policy blaming the work for a perimeter it never reached. Two holes, both mine. SSETransport never sent the token: I wired it into the git executor and not into the path that actually runs a bee. And requireLocalAuth proves "you are on this machine" by handing a token to trusted origins over loopback and checking it back, which a caller in a container can never do. TRIOS_API_TOKEN now satisfies both gates, from one exported check rather than two implementations - a server reachable from more than one machine has one credential, and asking the same question twice is how the answers come to differ. The audit records the outcome as `api-token` rather than silently logging it as an ordinary pass.
…task gets restarted Two deadlocks found by running the swarm rather than by reading it. FIRST, the retry policy charged a perimeter failure to the work. classify() returned `unmeasured` for a turn the server refused before a byte arrived - zero output tokens, zero tool calls, no commit - and `unmeasured` counts against the issue. Two such refusals retired browseros-ai#1111 as "attempts have already failed on their own merits". Nothing had failed on any merits; /chat had answered 403. A turn that produced nothing at all is now `interrupted`, which is what that case already meant: "nobody failed; the attempt simply stopped existing". Deliberately requires all three to be knowably zero - nil is "nobody counted", not "nothing happened", the distinction this file was rewritten for once already. SECOND, nothing resumed a task found already in `rejected`. sendTaskBackToWorker rejects and restarts as one act, so a task that lands in `rejected` between the two - an app stopped mid-way, a registry edited by hand - is never given its next turn. Its issue is skipped as spoken for while its boundary is held against everyone else: the queued deadlock staleQueuedReason describes, one state along. The tick now resumes them before choosing new work, bounded by the same capacity rule, because a resume is a worker like any other. The first attempt at this called sendTaskBackToWorker itself and was refused by its own first guard - a task already rejected cannot transition into rejected - silently, because that guard reports to the Queen's chat and not to the log. The resume does the restart half only, and says so when it cannot. 120 delegation checks, 0 failures.
`queend` answers "who is next" from the Queen's live registry, decoded as the
app writes it rather than summarised for the trip:
candidates [1111, 1128, 1129], the running registry
-> chosen: none, "nothing to choose"
browseros-ai#1111: its files are held by gHashTag/trios#1286, browseros-ai#1127, browseros-ai#1174
browseros-ai#1128: same
browseros-ai#1129: same
That is capacity, then boundaries, then order - the three rules that decide
which bee starts next, and the last part of the supervision loop still
reasoning on a laptop about a filesystem it cannot see. The rule naming the
holders is the one this session repaired from the Mac side; it now gives the
same answer from the other side, out of one implementation.
Made possible by the visibility pass on QueenDelegation, QueenSalience,
QueenCriterionVerdict and ModelPricing: 158 annotations, converged in three
compiler-driven rounds. The app builds unchanged - `public` on an internal
declaration is additive.
Two defects found by doing it:
Dates. The registry writes ISO 8601 and JSONDecoder defaults to a numeric
interval, so the whole question failed with "could not decode" - a message
naming neither the field nor the reason. The strategy now matches the
registry's own reader, and a decode failure carries the error.
A 17 KB command to `filesystem_bash` returns 500 INTERNAL_SERVER_ERROR. Worked
around by writing the payload with filesystem_write first, which is what that
tool is for, but the 500 is a real limit and is not documented anywhere.
What is still on the Mac is now plumbing rather than policy: a timer, a GitHub
client, and the call that dispatches a turn.
The chooser skipped any issue with a task recorded against it in ANY state - "a task already exists for it". Its own doc comment listed the states it meant: queued, running, awaitingReview, rejected, accepted, merged. `cancelled` and `failed` were never in that list and were excluded anyway. So an abandoned attempt, or one that failed and therefore must be retried, removed its issue from the board permanently. Measured on the live board this afternoon: of 40 candidates, SIX were unreachable for this reason - browseros-ai#1127, browseros-ai#1147, browseros-ai#1173 and browseros-ai#1286 cancelled, browseros-ai#1111 and browseros-ai#1133 failed - every one still open on GitHub, none of them ever choosable again. A failure is the state that most plainly means "do this again", and it was being read as "never do this". `QueenDelegationPolicy.claimOnIssue` now answers in three ways instead of one: live (queued, running, awaitingReview, rejected - someone has it or is expected back), done (accepted, merged - choosing it again would redo landed work, which is how browseros-ai#1244 collected six duplicate verification commits in an afternoon), or free. Only free is choosable, and cancelled and failed are free. It reads EVERY task for the issue rather than the first one the registry happens to list, so a live retry over an old failure is claimed by the retry. The refusals say which of the three now, rather than one sentence for all six states. On the live board that turned "a task already exists for it" into "the work already landed (accepted) - the issue is open and nobody closed it", which is a different problem and now says so. Gated by `queend-choose.test.ts`, which drives the real binary because queen-core has no test target and XCTest does not link under the CommandLineTools toolchain. Proven both ways: 6 of its 7 checks fail against the old chooser, all 7 pass against this one. Its skip when the binary is absent is stated in the file rather than left to be discovered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
browseros-ai#1111 was chosen, dispatched, worked, judged against two acceptance criteria and ACCEPTED at 15:26 - the first full autonomous circuit this loop has ever completed. And then it kept holding rings/SR-00/QueenInterfaceDivergence.swift for the whole 48-hour review window, because every finished dispatch was put on the board as `awaitingReview` whatever the Queen had decided about it. Starvation arriving from the one place nobody looks: a task that succeeded. The state now comes off the verdict, in the round and on the board by the same rule: accept -> accepted, terminal, holds nothing sendBack -> rejected, because the same bee is expected back on those files escalate -> awaitingReview, a person is needed and the 48-hour clock runs wait/none -> awaitingReview, not judged yet, the hold stands Proven by reverting the accept branch and watching 'releases the files of work the Queen accepted' go red, with a sibling check that an escalation still blocks. 407 tests green, typecheck 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…browseros-ai#1111) (#308) Two bees ran in parallel with disjoint boundaries (browseros-ai#1110 owned rings/SR-02/QueenBranchCommitter.swift, browseros-ai#1109 owned rings/SR-02/ChatViewModel.swift and rings/SR-00/QueenReviewVerdictRequest.swift). browseros-ai#1110 made baseBranch() return String?, exactly as its criterion asked; the call site lived in the other bee's file. Both honoured the one-owner-per-path rule and the build broke: rings/SR-02/ChatViewModel.swift:4142: error: value of optional type 'String?' must be unwrapped to a value of type 'String' The boundary rule protects against a conflict of writes. It says nothing about a conflict of interfaces: a signature changed in one file breaks a different file, and only a build of the two states together can see it. The acceptance-side half landed with browseros-ai#1128 (runInterfaceDivergenceWatchdog compiles the combined state before any acceptance; transitionToAccepted takes the proof as a parameter, so no path to .accepted can skip it). What remained open was the issue's second criterion: drift-guard judges two worktrees it builds itself, not two states of this tree, so returning the former behaviour moved nothing. This file closes that gap with the decision itself, as a pure rule: - QueenInterfaceDivergence.verdict(between:) takes the interface facts of each lane (declarations whose shape changed, calls and the shapes they expect), overlays the lanes the way verifyCombinedBuild overlays branches, and reports every call that no longer matches, plus the two-changers-one- declaration case. Findings name both sides - who changed what, and who still calls the old shape - because a refusal that does not name its subject sends the reader hunting for the wrong problem. - The single-file suite rides in the same file behind QUEEN_INTERFACE_DIVERGENCE_SELFTEST and runs on the incident's own facts: the browseros-ai#1110/browseros-ai#1109 pair is refused, while a change whose call moved with it, a body-only change, and a new call written against the new shape all pass. The block holds declarations only: -emit-module typechecks even inactive #if branches (measured on the container), so a top-level entry would break the app build; the documented one-line driver keeps the file buildable and the suite runnable where no linker exists. - The revert proof is executable: formerIsolatedVerdict restates the pre-browseros-ai#1111 loop (each lane judged alone, nothing crosses a boundary) and the suite shows it accepts the incident pair that the rule refuses. Revert the rule to that judgement and the incident scenario fails its first check - the check breaks. Verified live against two mutants (the isolated revert and a blanket-refuse revert): both fail the suite loudly and non-zero. Nothing is imported, not even Foundation, so the rule compiles anywhere the Swift standard library does and the suite runs interpreted on the Linux container: 17 checks, 0 failures, parse/typecheck/emit-module green with the flag both off and on. Co-authored-by: queen-1111 bee <bee@trios.local>
Summary
test:tools:aclscript and@huggingface/transformersdependency/lock graph.Design
This is a full removal of an isolated, unused leaf surface under
apps/server/src/tools/acl. No runtime trust, CORS, or tool-registration behavior changes. If the ACL reference is needed later, this PR can be reverted to recover it.Test plan
rg -n "@huggingface/transformers|src/tools/acl|tools/acl|acl-scorer|acl-embeddings|acl-edit-distance|acl-stopwords|test:tools:acl" .bun run --filter @browseros/server typecheckbun run lint(exit 0; existing warnings outside this change)server-toolsjobbun run --filter @browseros/server test:toolsfailed in existing browser-backed CDP tests after ACL removal: navigation visible/hidden tab assertions and CDP disconnect during input testsbun run test:allfailed on the same local CDP/browser instability; server agent/API/lib/browser groups, agent tests, eval tests, and build script tests passed before those failures