Repository navigation
ci: add release pipelines for iii-lsp and VSCode extension - #17
Merged
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 18 minutes and 48 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughWorkflow enhancements adding CI/CD support for VS Code extension and LSP projects. Changes include new release workflows for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/ci.yml (1)
94-113: Consider renaming job to reflect actual validation scope.The job name says "Lint" but the actual step runs a packaging dry-run (
vsce package --out /dev/null), not linting. Consider renaming to "Package Check" or "Build Check" for clarity. Alternatively, add actual linting (e.g., ESLint) if that's intended.💡 Suggested name change
iii-lsp-vscode: - name: "iii-lsp-vscode: Lint" + name: "iii-lsp-vscode: Package Check" needs: changes🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 94 - 113, The job labeled "iii-lsp-vscode: Lint" is misleading because the step runs a packaging dry-run (`npx `@vscode/vsce` package --out /dev/null`) rather than linting; update the job's name field (the "name" under the iii-lsp-vscode job) to something like "iii-lsp-vscode: Package Check" or "Build/Package Check" to reflect the actual validation, or alternatively add a lint step (e.g., run ESLint) if linting was intended so the name remains accurate..github/workflows/release-lsp.yml (1)
44-53: Prerelease regex may not match all intended patterns.The regex
-([a-z]+)\.[0-9]+$only matches lowercase prerelease labels (e.g.,alpha,beta,rc). If uppercase or mixed-case labels likeRCorAlphaare ever used, they won't be detected as prereleases. This matches the pattern inrelease-vscode.yml, so it's likely intentional, but worth noting for consistency withcreate-tag.ymlwhich only offers lowercase options.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-lsp.yml around lines 44 - 53, The prerelease regex only matches lowercase labels, so update the regex used on VERSION in the elif clause (currently `-([a-z]+)\.[0-9]+$`) to allow mixed/upper-case prerelease tokens (e.g., use a character class like [A-Za-z]+ or an equivalent case-insensitive approach) so values like "RC" or "Alpha" are detected as prereleases; adjust the regex in the release-lsp.yml condition that tests VERSION and keep the dry-run branch unchanged..github/workflows/create-tag.yml (1)
150-159: Consider adding validation for missing version field in package.json.If
.versionis missing from package.json,jq -r '.version'returns the literal string"null"rather than failing. While the providediii-lsp-vscode/package.jsonhas a valid version field, adding explicit validation would make the workflow more robust against future manifest issues.💡 Suggested defensive check
read_node_version() { - jq -r '.version' "$1" + local ver + ver=$(jq -r '.version // empty' "$1") + if [[ -z "$ver" ]]; then + echo "::error::No version field in $1" + exit 1 + fi + echo "$ver" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/create-tag.yml around lines 150 - 159, The workflow's read_node_version helper can return the literal "null" when package.json lacks .version, so add validation after invoking read_node_version (and similarly for read_cargo_version via MANIFEST_TYPE) to verify the returned current value is non-empty and not "null"; if invalid, emit a clear error via workflow logging and exit non‑zero. Locate the read_node_version function and the branch that sets current (using MANIFEST_TYPE and read_cargo_version/read_node_version) and insert a guard that checks current != "" && current != "null" (or use jq -e to fail fast) and fail the job with a descriptive message if the check fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 94-113: The job labeled "iii-lsp-vscode: Lint" is misleading
because the step runs a packaging dry-run (`npx `@vscode/vsce` package --out
/dev/null`) rather than linting; update the job's name field (the "name" under
the iii-lsp-vscode job) to something like "iii-lsp-vscode: Package Check" or
"Build/Package Check" to reflect the actual validation, or alternatively add a
lint step (e.g., run ESLint) if linting was intended so the name remains
accurate.
In @.github/workflows/create-tag.yml:
- Around line 150-159: The workflow's read_node_version helper can return the
literal "null" when package.json lacks .version, so add validation after
invoking read_node_version (and similarly for read_cargo_version via
MANIFEST_TYPE) to verify the returned current value is non-empty and not "null";
if invalid, emit a clear error via workflow logging and exit non‑zero. Locate
the read_node_version function and the branch that sets current (using
MANIFEST_TYPE and read_cargo_version/read_node_version) and insert a guard that
checks current != "" && current != "null" (or use jq -e to fail fast) and fail
the job with a descriptive message if the check fails.
In @.github/workflows/release-lsp.yml:
- Around line 44-53: The prerelease regex only matches lowercase labels, so
update the regex used on VERSION in the elif clause (currently
`-([a-z]+)\.[0-9]+$`) to allow mixed/upper-case prerelease tokens (e.g., use a
character class like [A-Za-z]+ or an equivalent case-insensitive approach) so
values like "RC" or "Alpha" are detected as prereleases; adjust the regex in the
release-lsp.yml condition that tests VERSION and keep the dry-run branch
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c1c2feb3-e447-4dd8-a266-010f24ce62c0
⛔ Files ignored due to path filters (1)
iii-lsp-vscode/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
.github/workflows/_rust-binary.yml.github/workflows/ci.yml.github/workflows/create-tag.yml.github/workflows/release-lsp.yml.github/workflows/release-vscode.ymliii-lsp-vscode/package.jsonregistry/index.json
This was referenced Apr 8, 2026
This was referenced May 4, 2026
3 of 6 tasks
This was referenced Jun 13, 2026
ytallo
added a commit
that referenced
this pull request
Sep 25, 2026
* feat(sentinel): scaffold the error-monitor worker sentinel watches an iii engine's own telemetry for failures: it will group them by deterministic fingerprint, freeze the evidence before the engine's 10k-span ring discards it, and open assisted harness investigations beside the console page. This is the skeleton — the configuration surface, `sentinel::status`, and the boot sequence the rest hangs off. Design of record: tech-specs/2026-09-sentinel/. Boot runs in two phases, and the order is load-bearing rather than stylistic. The interface (config, functions, the configuration doorbell) registers first; the durable dependencies are claimed in the background. Release interface capture boots this binary against an isolated engine that has no `database` and no `queue` and expects a non-empty surface within seconds, so claiming the store first would hang the capture and hide the worker from the registry. Until the dependencies answer the worker is up but not enabled: `sentinel::status.enabled` is false and says which dependency is missing, instead of scrolling errors. A stored configuration that does not validate is treated differently from a configuration worker that is down. The second is transient — retry, the value is still out there. The first is permanent, and retrying it forever reads exactly like the transient case while the operator waits for a worker that will never start. So a refused value boots on the shipped defaults, disabled, with the reason in `status.config_error`; the reload doorbell heals it the moment the entry is fixed, and a refused reload never replaces the live value. Two spec assumptions were measured and corrected here. The Rust SDK reports the binary's file name as `service.name` unless told otherwise, and exports no namespace resource attribute — this worker attributes errors by service name, including its own, so it names itself explicitly and ships a `service_aliases` map for SDKs that differ. And `metadata.internal` does not stamp `iii.function.kind=internal`: the engine only stamps that for built-ins, which closes spec risk #17 negatively and makes the ingest's phantom-tick ring mandatory rather than a belt. Verified against a live engine: status answers, the surface lists with the expected metadata, a config update hot-reloads, and an invalid one is refused with the live value intact. * feat(sentinel): group and redact — the pure core of ingest The four decisions that make a monitor useful rather than a log tail, written as pure functions with no I/O so they can be reviewed and tested as rules rather than as pipeline behaviour. Normalization is what turns a thousand occurrences into one group, and its rule order is load-bearing: identity numbers are protected first, because `HTTP 429` and `HTTP 500` are different failures and must never collapse, and that protection has to survive the hex rule, not just the number rule — `status 12345678` would otherwise read as a hash. Masking numbers before hashes would eat the first digits of every commit and trace id. Redaction runs on capture, before the first insert, because dropping attributes by key name does nothing for a connection string inside an `exception.message` — which then travels to a model provider intact. Computing the fingerprint from the redacted form also stops a per-request token from splitting one failure into a thousand groups. It is a denylist and therefore never complete, so it is one of three layers and the count is visible in `sentinel::status`. The lifecycle enforces the rule the adversarial review found missing: a recorded diagnosis moves a group forward from investigating or new and never undoes a person. Resolve while a diagnosis request is still in flight and the answer lands as a new version with the group still resolved. `resolve until version change` will not manufacture a regression from an unknown version either — in a dev stack that is every occurrence. Both grouping rules are fixture-driven: a table of real stack messages with the reasoning per row, versioned alongside the normalizer, because changing a rule changes the identity of every existing group. An independent pass asserts no fixture leaves its secret behind, whatever the hand-written expectations claim. * feat(sentinel): the durable store, tested on the driver it will run on Nine tables, a versioned migration path, and the write that every occurrence goes through. Two shapes in here are the data-model defects the adversarial review found, now enforced by the schema rather than by convention. An ERROR log arrives before the span that explains it, and a log-sourced group is only created if that wait times out — so the row has to exist before its group does. `group_id` is therefore nullable, but a CHECK binds the nullability to `pending_join = 1`, which keeps "a log is waiting" from decaying into "nulls happen sometimes". The partial unique index on running investigations is the same kind of guarantee: one first pass per group, enforced by the store instead of by whoever remembers to look first. State changes are compare-and-set. A transition is decided from a snapshot read a moment earlier, so the UPDATE carries the `updated_ms` it saw and returns the id it changed; no row back means somebody moved the group in between and the writer re-reads and decides again. That is what makes "a record never undoes a human decision" survive concurrency instead of being a comment above a function. Transactions are one-shot for a related reason: the interactive form pins a connection across round-trips and would serialize four ingest jobs behind the slowest one. The tests run the real statements against rusqlite — the same driver the `database` worker uses for SQLite — rather than a hand-written fake of the store. A fake agrees with whatever the code does; this disagrees when the SQL is wrong, which is how the CHECK, the partial index, and the RETURNING the compare-and-set depends on are all covered. It also settles two open questions from the plan: both work as written. * feat(sentinel): adapters — a trace tree and a log record become one shape Everything downstream works on `ErrorEvent`, never on a span or a log record, which is what makes a third source an adapter rather than a second pipeline. Two rules in the span adapter decide what becomes a group, and both are about not creating one. A failure propagates upward — the shell call fails, the turn step fails, the root fails — so only leaf error spans become events and the ancestors ride along as `propagated_through`; a parent that fails after its child succeeded is itself a leaf and still counts. And a trace whose root belongs to this worker is dropped whole rather than span by span, because the error the Sentinel causes surfaces in the worker it called: a failing `database::execute` is a `database` span, so excluding by service name would leave the loop open. The queue step is caught by name for the same reason — it runs as its own trace called `fn_queue sentinel-*`. Worker `execute <fn>` spans carry no `function_id` attribute; the engine derives it from the span name. That derivation now lives in one place, because attribution differing between two call sites is exactly how a group ends up filed under the wrong worker. The evidence bundle exists because the engine's span ring is ten thousand spans deep and the trace is usually gone by the time someone clicks. It is capped three ways: an attribute allowlist so a worker stamping something unexpected cannot widen what is stored, per-value caps with the truncation marker inside the budget rather than added to it, and a byte budget spent nearest the failure — the drop order is a walk out from the origin span, so a sibling subtree goes before the failure's own ancestors. Log records get the same treatment with one difference worth naming: their attributes are an object of arbitrary JSON where a span's are an array of string pairs, and they carry no session at all. The session has to be resolved from the trace, which is also what applies the investigation exclusion to logs. * feat(sentinel): the ingest pipeline — a tick becomes a group The three pieces that decide what a group *is*, and a pipeline that spends most of its effort deciding what not to record. Attribution is the registry's answer, not the span's. A span's `service_name` is whoever emitted it, which for a call whose callee span has not arrived is the engine's own hop — attributing by it files the failure under `iii` and points the investigation at the wrong repository. The cache is keyed by `(namespace, function_id)` because the engine allows the same id once per namespace; a single-key cache would let one registration overwrite the other. When nothing can break a tie, the ambiguity is kept under its own namespace value rather than resolved by guessing, so an ambiguous group never mixes with a resolved one. The phantom-tick ring is the mechanism rather than a belt. The engine drops ticks for spans of functions registered as trace-trigger targets, which covers the handler but not the `database::execute` the handler queued — that span belongs to `database` and ticks normally. Registration metadata cannot close it either: `iii.function.kind` is stamped only for engine built-ins. The version fallback makes regression detection work where it is needed most. A development build reports the same string for weeks, so `until version change` would never fire; when the self-reported version is indistinct the mapped checkout's commit takes its place. Ten pipeline tests run over a real SQLite store: the cascade becomes one group owned by `state` rather than `iii`, a span ticking twice is one occurrence, the worker's own traces and its investigations are dropped and counted, a trace that left the engine's ring is counted rather than guessed, a secret in a `status_description` never reaches the store, and a log waits for its span, folds into it when it arrives, or is promoted to its own group when it never does. That last path is also where the tests earned their keep: promotion was being refused as a duplicate of the very row it attaches. * feat(sentinel): the read and mutate surface the console calls Listing sorts regressions first: a failure that came back after somebody fixed it is the one piece of news in a list of known problems. Sessions affected is counted from its own table rather than as a DISTINCT over occurrences, so the number stays true once the thousand-row cap starts cutting, and the sparkline is read from the hourly buckets instead of scanning. Mutations go through the same compare-and-set as the ingest, so 'only a human resolves' holds when an occurrence lands in the same instant rather than only when the timing is kind. Ignoring captures its baseline at the moment somebody says so — 'fifty more occurrences' counts from now, not from the group's first — and reopening clears everything the closing decision set, including the version it was resolved on. Evidence that retention has pruned reads as pruned rather than as a missing occurrence: the row stays and says the bundle went. * feat(sentinel): wire the pipeline to the engine and prove it on a live stack The gateways, the trigger bindings, the event fan-out and the boot that assembles them. With this the worker does the thing it is for: pointed at a running engine it finds real failures, files each under the worker that owns the failing function, and freezes the trace. Readiness is now proven by doing the work rather than by looking the dependency up, and the live stack is what taught that. The old probe asked `engine::functions::info` whether `database::execute` existed and was told yes — because a bare id resolves across namespaces — while every actual call failed, since a call routes to the caller's own namespace. Migrating the schema and defining the queue are the probe now; they are the operations the worker needs anyway, and their failure message names the real problem (`UNKNOWN_DB`, the wrong namespace) instead of a cheerful "ready". Every outbound call is tagged `iii.tag.hidden` and remembers its trace id in the phantom ring, because those calls *are* the traces that would otherwise tick the trigger feeding this worker. Events are a doorbell, not a record: a subscriber re-reads the group rather than trusting the payload. Occurrence events coalesce per group over a second so an incident with a thousand occurrences is not a thousand deliveries, while a state change goes out immediately, since that is the one an alert cares about. Bindings live in slots the recovery loop reconciles — a monitor that quietly stopped watching is worse than one that never started. The live run also caught a real defect: the settle pass rebuilds the bundle from the span, and a span names the process that emitted it rather than the worker that owns the function, so re-reading a trace was quietly re-filing its evidence under the wrong worker. The attribution is carried across the rebuild, with a test. * feat(sentinel): the investigation, and the one write it can make An investigation is a harness session beside the group's page, not a job that returns a result. The structured half — the diagnosis — arrives as a function the agent calls, so nothing forces the model into JSON and the person watching can interrupt and get prose back. Three rules are enforced rather than hoped for: - The row exists before the harness is told anything. A doorbell, or the agent's own `record`, can beat the send's answer back; a test reads the store from inside the send call to prove the row is already there. - One running first pass per group, guarded by the partial unique index. A second Investigate gets the existing session, not a second one. - Identity never comes from the payload. `record` reads `iii.session.id` from the invocation's baggage in the handler body, before any spawn. The request has no investigation id and no fallback: without baggage it is `no_investigation`, and a session investigating another group is `not_this_group`. The lifecycle race resolves in the person's favour. A resolve that lands while the agent is still typing wins: the diagnosis is written anyway, as a new version, and the response says the group stayed resolved. `trace::get` and `logs::list` are the agent's only window onto the live engine, and they pass everything back through the ingest's redactor — `engine::traces::*` and `engine::logs::*` stay on the deny list, so a token in a neighbouring span of the same trace never reaches a model. A first pass that ended having said nothing is nudged once, ever; one that failed, was cancelled, or whose session the harness has forgotten puts the group back where it came from. The recovery loop reconciles the running ones every thirty seconds, because a doorbell that never rang would otherwise leave a pass running forever. Also here: retention tests that were left open in M1, and investigation counts on `sentinel::status`. * fix(sentinel): read the caller's identity inside the handler's future On a live stack the agent investigated a real failure, reached a conclusion, and was told six times in a row that its call carried no session. The rule was right and the read was in the wrong place. The SDK evaluates `handler(request)` and only then wraps the future it returned with the invocation's OTel context (iii-sdk-0.23.0 src/iii.rs:2327). Reading the baggage in the closure body therefore runs outside that context and finds nothing at all — the same reason a bare `tokio::spawn` loses it. The read moves inside the async block, where the context is live. A source test pins it: within the `diagnosis::record` registration, the baggage read must come after `async move {`. It is the kind of mistake that compiles, passes every unit test, and only shows up when a model is on the other end. Also applies the M1 and M2 deviations to the spec: `agent_trigger` does not validate a payload against the function's schema (the typed request's serde does), chat mode uses `session::ensure` with the marker in `origin`, the queue is a retry rather than a boot failure, readiness is proven by doing the work, transactions are one-shot with compare-and-set, the settle pass carries attribution across the rebuild, and `status`/`occurrences::list` are denied to an investigation rather than exempt. * feat(sentinel): the console page, checked against the live stack The page a person opens when something failed: the list of groups with a 24-hour sparkline, the detail with the frozen evidence, and the investigation session opened beside it. Built on the shared console surface throughout — `useWorkerLive` over this worker's two trigger types, `useContainerNarrow`, the page chrome, the table family, `Settings*` with `DirectoryPicker` for the configuration form. The sparkline is CSS bars rather than inline SVG, which is the one thing the design system asks a worker page not to reach for. Three rules the page keeps: - It calls this worker and nothing else. `engine::traces::*` is no more the console's to read than it is an investigation's, and an embed test fails the build if the built bundle mentions it. - The session column's state is not knowable from here, so the header always offers the button — quiet with a live dot when the session is already in front of the user, a pill otherwise. Both call `selectConversation`, which is idempotent. - Keys the configuration form does not render survive a save. A worker gains a setting, somebody sets it by hand, and the form must not drop it. Checked on the dev stack against real captured failures, in both themes and at 760px: the list, the detail, the evidence with its stack trace one click away, the menus, the tabs, and no console errors. Four layout defects that only a rendered page shows were fixed on the way — a facts card shrunk to two pixels by a long sibling, a table wider than its pane, missing gutters, and an exception payload that buried the tree it sat above. Also here: a `call` example, because there is no iii call CLI and proving a worker against a real engine needs something that speaks the protocol. * feat(sentinel): the dev stack, and the smoke that needs a real engine The compose entries that were deferred twice: a `database` container the stack did not have, the sentinel after it, and a disabled block for interface collection — an ingesting monitor would capture the collection run it is part of. (`harness/config/` is local and gitignored, so the seeded configuration stays a local file.) The smoke is the only test here that needs an engine, and the only one that can catch a bug where every piece is right and the wiring between them is not. A throwaway worker fails three times; the group appears attributed to the worker that owns the function rather than whoever called it, with the evidence already frozen and `execute sentinel-e2e::boom` as its origin span; and no group belongs to `sentinel`, `database` or `queue`, which is the cut that stops the monitor from monitoring itself. Connect-or-skip, so a run without a stack stays green; `III_E2E_REQUIRE=1` makes a missing engine a failure. Against the live stack the whole thing takes a second and a half. The README drops its "early" notice and describes what the page actually does now. * fix(sentinel): a second Open in chat reopens the seated session Two small ways an investigation could surprise the person who started it. Clicking Open in chat twice seated the evidence in a second session, so the sidebar grew a duplicate conversation about the same failure while the first one still held the transcript. The guard now covers both shapes: a running first pass, or a chat session already seated for that group. And a chat session whose evidence could not be appended stayed `open` — a conversation offering to discuss evidence it never received. It fails with the reason instead, the way a refused `harness::send` already did. * chore(sentinel): drop an empty file a stuck shell left behind A redirect created ui/src/ui.placeholder before its command blocked on stdin, and it rode into the page commit. Zero bytes, no references, no effect on the build: the linter and tsc both ignore the extension. * docs(sentinel): the four things the implementation proved and the spec did not carry The M1 and M2 divergences went in with the code. These are the ones the console delivery and the live run turned up after it. - **Where the baggage is read.** The spec said the handler reads `iii.session.id` with `get_baggage_entry`, which is true and not enough. The SDK wraps the future the handler returns, not the closure that builds it, so a read in the closure body returns nothing. It compiles, it passes every unit test, and it fails only with a model on the other end. - **The chat-evidence renderer does not exist.** The spec had the Sentinel labelling its own transcript entry. `registerTranscriptRenderer` is a type with no consumer in the console, so the entry reads as the user's and the heading in the text is what identifies it. - **The Traces deep link.** The spec left two options to the UI delivery. The delivery took the second: render the tree from the frozen bundle and say so when the trace has left the ring. Nothing in the console changes. - **What the UI section actually shipped.** It read as if all of it landed. Six pieces did not, each an isolated addition, and they are named where a reader will look for them. * feat(sentinel): pick the model from the catalog, and keep every reading Two of the six pieces the console delivery left out. **The model comes from the router now.** A text field for a model id is an invitation to a typo that only surfaces when somebody clicks Investigate, so the form asks `router::models::list` and re-reads when the router says the catalog changed. The choice is stored as the pair the schema already has, `model` plus `provider`, rather than a joined string the worker would have to split. Two rules the pure half carries: a stored model the router no longer offers is kept and labelled rather than silently dropped, because a cleared credential comes back; and a raw id can still be typed, because the catalog is sometimes behind what the router serves. **Every reading is kept, so the page shows them.** `sentinel::diagnoses::list` is the history behind the diagnosis in force, collapsed under it. It is on the investigation's deny list for the same reason `occurrences::list` is: an agent that reads what an earlier one concluded is anchored to that answer, which is the opposite of a second opinion. Ordering gained the id as a tiebreak, since a millisecond holds more than one recording when somebody asks twice. **And a defect the real pass turned up.** `useWorkerLive` re-reads on its triggers and its poll, not when the query itself changes, so changing a filter left the list answering the previous question until the next error arrived. The search box looked broken. It refetches on a settled query now. Checked in the console against the live stack: the picker offers 46 models grouped by provider with the configured one checked, and the Diagnosis tab shows the current reading over one earlier one with its provenance. * fix(sentinel): the configuration form was never in the repository `.gitignore` carries an unanchored `config/` for the local runtime composition directories, and it swallowed `sentinel/ui/src/config/` whole: the form, its model, the catalog and their tests, six files. Everything built and every test passed, because the files were on my disk. A fresh checkout would have failed at `page.tsx`, which imports a form that is not there. The directory is `src/settings/` now. Renaming one worker's folder is cheaper than punching a hole in a repo-wide ignore rule that exists for a good reason, and it matches what the console calls the surface anyway. No other worker UI uses that name, so nothing else was hiding. * feat(sentinel): the last four pieces of the page **History reads a real table.** `sentinel_transitions` is schema v2: one row per move, written inside the transaction that makes it, so the history cannot disagree with the status above it. A refused move leaves no trace claiming it happened, and a test pins that. The actor is a role — ingest, agent, investigation, console — not a person: the console hands this worker no user identity, and the engine's caller uuid would read as one without being one. The daily pass keeps 200 moves per group plus the beginning, because tiny times forever is still forever. **Batch actions offer only what applies to all of them.** A selection mixing an open group with an ignored one gets nothing, and says so, rather than promising a Resolve that refuses halfway and leaves the person working out which half. Each group is still its own compare-and-set, so the result is a count and the first refusal, never a silent partial success. Investigating is never a batch: each one opens a conversation somebody is meant to watch. **Investigate with…** opens the one dialog the page has, on the same catalog the settings form uses, and changes nothing about the configured default. **Occurrences link to the conversation.** An error with a session is an error somebody was in the middle of, and the transcript says what they were doing. Checked in the console against real groups: selection and select-all, the four history rows a group picked up from being resolved, reopened, ignored and unignored, the session column, and the dialog. No horizontal overflow at 1280, 900, 760 or 420, which took fixing the shrink-to-fit idiom the earlier pass had reached for with `table-layout: fixed` and broken. * fix(sentinel): follow the console's own contract, including on a phone Reviewed the page against `harness/ade-worker-design` as it stands on GitHub. The local copy of that skill is stale and missing `planning.md`, so the review used the fetched one. Two things broke the skill's definition of done. `PageHeader` never received `onRequestClose`, so a pane holding this page had no close action. And the page had never been seen at phone width in a running console, which the skill counts as not done regardless of what the build says. Four places reimplemented what the console already gives: - Rows hand-rolled `tabIndex` and an Enter/Space handler. `TableRow` has `interactive`, which also walks with the arrow keys. - The counts strip was a div. It is a `StatusBar` now, which is where the design system puts counts. - Back was a text button. It is an `IconButton` and a `Breadcrumb` inside a `Toolbar`, the documented path recipe. - The loading skeleton was one bar across the row rather than the shape of the content it replaces. Two rules were simply broken. `R` and `Escape` were bare-key shortcuts; the palette rows stay, the keys go. And the absolute timestamp sat in a tooltip on a non-focusable span, so it was mouse-only: it moved into the detail's fact row, beside the relative one. The live binding could collide. Two panes of this page in one tab both called `iii.on` with the same handler id, and closing one would have torn down the other's subscription. The id is keyed per pane now, as is everything that survives a reload: filters and the open group were component state, so they were lost on reload and shared between panes. **And the list is no longer a table at every width.** The skill is explicit that narrow panes change the interaction model rather than shrink the desktop one: each level is `List` with `ListItem`, which owns the touch target, keyboard traversal and neutral selection. Below the threshold the rows are list items at 60px, batch selection drops (it is a wide-pane affordance, not a phone one), and the filters become menus that open as bottom sheets — a segmented rail of five options in eighty pixels scrolls, so nothing was lost, but nothing was findable either. Search moved to its own strip there, because one strip squeezed the filter labels to a letter and an ellipsis. Seen in the running console at 390, 700 and 1280, in both themes, plus as a real workspace pane beside another worker's page: list shape and row height correct at each width, no horizontal overflow anywhere, breadcrumb and back working, and the close action present on the pane. * fix(sentinel): the rules the first review pass did not reach Re-read `patterns.md` end to end and the half of `console-design` I had skimmed. Five more findings, four of them rules. **Surfaces, not borders.** Three regions were framed with a hairline while already carrying a background: the batch bar, the captured message and the investigation line. The design system's first principle is that a region is a one-step background change, and it lists the three strokes that exist — none of them frames a region. The frames are gone. Two hairlines that guided flat lists went with them; the span tree keeps its one, because depth is what that list is about. **Async outcomes were silent to a screen reader.** A batch action reported itself in a panel and nowhere else. It goes through `LiveRegion` now, with a monotonic sequence so the same sentence twice is announced twice. **One throwing tab took the page down.** Each tab fetches on its own, so each is wrapped in `ErrorBoundary`, which the design system lists for exactly this. **The configuration form ignored `focusField`.** A host deep link into a setting did nothing. It now opens the deck level that holds the field, focuses the control and scrolls it into view — and the deck's field carries its real dotted path rather than a label, so the path the host sends is the path the form answers to. Also: the confidence badge is one word, as badges are; the dialog takes the creation-modal width and submits on Enter unless the combobox is claiming the key; versions set tabular figures like every other number. One thing I am leaving as it is, with the citation. The detail is a screen in the same pane rather than a second page opened through `host.panels.open`. That is the Console catalog archetype, whose wide shape is a grouped list leading to a breadcrumb, an identity masthead and tabs — which is what this is. The record-screen pattern is a different archetype, for one record with a body, properties and history. * fix(sentinel): a third pass, and a defect the second one introduced Read the half of `console-design` I had still not opened: the surface ramp, the ink and status tokens, the font roles, the radii and motion table. **The second pass broke the diagnosis header.** Splitting "low confidence" into a one-word badge left two eyebrows in the same row, separated by a chip, and they read as one label: "CONFIDENCE CONVERSATION · 1D". The row keeps one eyebrow now, labelling the badge, and the provenance moved to the foot beside the model, the investigation and the checkout — which is where a reader looks for where something came from. **The investigation strip used the wrong step of the ramp.** `panel-raised` is for headers, cards, popovers and tool cards; an inline strip in the body is a secondary card, which is what `surface` is for. **And one line was decoration.** `data-settings-deck-fallback` never reached the DOM, because `DirectoryPicker` does not forward unknown props. Measured in the running console, then removed rather than left looking like it worked. The comment says why, so nobody adds it back. Audited and clean: font roles (one `--font-code`, on the captured message; mono everywhere else in the chrome), no colour literals and no token outside the public set, no text below the eleven-pixel floor, accent spent only on the sparkline and alert only on the regression mark, state carried on `data-*` rather than extra classes, and the worker-side asset delivery going through the SDK message path with unknown paths refused. * perf(sentinel): read the list page in three queries, not two per row `groups::list` asked the database for the sessions and the sparkline of every row it was about to return, so a fifty-row page cost a hundred and one round trips. The two lookups now take the whole page's ids at once and the row builder reads from the maps, which makes the page's cost flat in the number of groups rather than linear in round trips. * fix(sentinel): the worker was starving the thread that carries its replies Every `sentinel::*` call on the live stack timed out — the console's `groups::list` included, which is what the page showed — while the process sat at a hundred percent CPU on one thread and no other worker was affected. The SDK spawns each invocation onto the runtime it built for its own `iii-connection` thread, and that runtime is `current_thread`: every handler in flight shares one thread with the socket that carries the replies those handlers are waiting for. The ingest pipeline is heavy enough to hold it — it measures and trims an evidence bundle for every error span in a trace, four traces at a time — so its own calls timed out, the queue redelivered the jobs that caused it, and the redeliveries landed on the same saturated thread. Nothing recovered on its own. The pipeline now runs on the worker's real runtime and the connection thread does nothing but carry messages. The queue could not drain either. The sweeper reads the parked logs whose wait has expired and hands them to the queue twice a second, and a row it cannot promote stays expired, so the same handful of rows was queued thousands of times a minute. Reading them now claims them: the wait is pushed forward in the same breath, so a stuck row retries on a backoff and the deadline still lives in the store, where a restart cannot start the flood again. Measured on the dev stack before and after: the connection thread went from 99.6% to 1.5%, `queued` from climbing by six a second with nothing processed to flat with the parked rows cleared, and `groups::list` from a ten-second timeout to well inside it. * docs(sentinel): the two facts the live stack taught the spec The SDK gives every handler to the runtime on its own connection thread, and that runtime is single-threaded — a constraint that shapes the architecture as much as the engine's bounded span store does, so it sits beside it. And reading an expired parked log has to claim it, or the sweeper hands the same row over twice a second forever. * feat(sentinel): the resolve facts and the last ingest, for the page The design's regressed note says what was resolved, on which version, and whether only a version change reopens it, and its header says when ingest last recorded something. The rows already held all of it; `GroupSummaryV1` now carries `resolved_at_ms`, `resolved_version` and `resolve_until_version_change` — the spec's reference type had `resolved_version` all along and the contract had dropped it — and the status counts carry the newest `last_seen_ms` from the same query. * feat(sentinel): rebuild the console page to the design, and fix what the rebuild found The page had drifted a long way from the prototype in tech-specs/2026-09-sentinel/design/mock. It is rebuilt against it, state by state, on the console's own components: - List: the Open / Regressed / Ignored / Resolved scopes and the time window as segmented controls, the worker menu and search, the summary chips, and the table the design draws — issue with the exception type in bold and where it failed underneath, count, sessions, first and last seen, a 24-hour sparkline whose hours since a regression are hot, and the status with its glyph. A footer says how many groups the filter shows and what it hides. - Detail: a reading column with the large title, the kicker, the badges and the repository, the actions the state allows as primary and pill buttons with their menus and hints, the state note, the facts grid, and tabs that carry their counts. - Latest occurrence: the origin span as key/value pairs with the exception block, the trace path with durations, the payloads one click away, and the log record for a log-source group. - Occurrences open the evidence of the row clicked; History is a timeline that always has the group's birth; Diagnosis shows the provenance, the finding as prose, the cited evidence, the fix and every version, each one viewable. - Configuration follows the design's order: investigation, repositories with their workers and the ones seen this week without one, sources, retention. Interface bugs fixed on the way: the page and its tab were called "errors"; Investigate was offered on a resolved group, which the lifecycle refuses; with no default model, Investigate could only fail, and now opens the model picker; narrowing to one worker emptied the worker menu of every other; "1 sessions"; a repository path and hex ids rendered in capitals; the exception message printed three times; the stack trace disclosure rendered as an unstyled bar; the empty list's text squeezed into a column; the open count disagreed with the rows without saying why; every action's confirmation stacked a second panel on top of the state note that already said it; page banners ran edge to edge outside the content column. * fix(sentinel): the page could not scroll `PageMain` is a clipping flex column by contract — a page brings its own scroll region — and this one had none, so everything below the fold was cut off: the detail of any group, the diagnosis, the occurrences table, and the list once it outgrew the window. One scroll container now holds the banners and the view, and opening a group or going back starts at the top rather than wherever the other view was. * fix(sentinel): a review pass over the rebuilt page Against the design, the design-system skill and the page's own behaviour, at phone, narrow and wide widths: - The detail never updated on its own: only the list re-read on group events. It now re-reads with the list, so a recorded diagnosis, a regression or a resolve made elsewhere shows up without a reload, and the occurrences and history tabs re-read in place instead of blanking. - A slow answer for the group somebody just left could paint over the one they opened; only the newest read lands now. - The Diagnosis tab said "No diagnosis yet" while its versions loaded, or for good if that read failed, although the group carried one. - On a phone the Back button slid under the header's own actions, the list gave half its width to a badge, a count and a time and cut every title to a word, the provenance of a diagnosis squeezed its text into a narrow column, and the actions were 32px — under the touch target. Back is an arrow there and sits before the kicker at every width, as the design has it; the row's facts go under its title; the provenance stacks; the actions are 44px. - Failed reads of the evidence, the occurrences and the history were drawn as empty states with nothing to do; they are alert panels with Retry. An action's error can be dismissed. A failed first read of the list no longer leaves skeleton rows under the error forever. - A selected regressed row kept its wash and showed no selection. - Relative times, the copy flash and the status dots are the console's own (`formatRelative`, `useCopyFlash`, `StatusDot` with its pulse) rather than local copies; the detail loads in its own silhouette. * fix(ade): a Selector inside a dialog could not scroll, take focus or close alone Every searchable picker in a modal dialog was broken — the Sentinel's "Investigate with…" and the model field of any worker's settings among them: the wheel did nothing over a long list, typing never reached the search, arrows and Enter could not pick, and Escape closed the whole dialog instead of the list. Two causes, both in the shared component. The list is portalled outside the dialog, and a modal dialog cancels every wheel and touch scroll outside its own content. The popover is now modal, as Radix's own Select and DropdownMenu are by default, so it holds a scroll lock of its own above the dialog's. And `@radix-ui/react-popover` was pinned to 1.1.6 while the dialog, dropdown and select had moved on, so the popover ran on its own copies of `react-focus-scope` and `react-dismissable-layer`. Two focus-scope stacks cannot pause each other: the dialog's trap handed focus straight back to the trigger whenever the search took it, and its dismissable layer read the list as outside. Popover 1.1.20 is the release that ships with dialog 1.1.20 and shares both. Measured against the running stack through `vite` from this checkout: the list scrolls, focus lands in the search, typing filters, arrows and Enter pick, a click on the dialog closes only the list, Escape closes only the list — and on the console built without these changes, none of that held. The new test fails on the scroll without the modal popover. * fix(sentinel): an engine log about a function is grouped by that function A log's call site was `code.function`, then `target`, then its instrumentation scope. The engine's own ERROR logs — "Function not found" and the like — name the function they are about in `function_id` and share one scope, so every one of them folded into a single group that could not say which function it meant: "Function not found", 34 times, across `provider::claude-code::count_tokens`, `sentinel::status` and whatever else was missing. `function_id` now comes before the scope, in both the log adapter and the promotion path that rebuilds an event from its parked evidence. New logs of that kind open one group per function, and the row shows it; the existing mixed group stops growing. * feat(sentinel): a UI/UX pass — reachability, continuity, and the repository form Walking the page's tasks — find what needs attention, understand it, investigate, decide, configure — turned up these: - Every group past the first fifty was unreachable: the footer said "showing 50 of N" and offered nothing. Show more reads the next page. - Opening a group from far down the list and going back landed at the top; the list now returns to where it was left. - An empty Diagnosis tab explained what an investigation is and offered nothing unless a session already existed; it offers Investigate. - A failure outside any conversation filled the occurrences table with a column of dashes for session and turn. Those rows show their own message instead — the concrete values the title normalized away. - "0 sessions" sat among the badges of every group without one. - Repositories: a checkout's folder could not be changed short of removing it and losing its workers; picking a mapped folder again added a copy; a new repository opened with no workers and no sign that it mapped nothing until Edit. The folder is a DirectoryPicker on the edit screen, warned when it is gone from disk; a mapped folder opens its repository; a new one opens straight into editing. - Workers, of a repository and ignored, were a comma-separated field that asked for every name from memory and cut the list at the input's width. They are tokens that remove themselves, added from the workers that failed this week or typed; one taken from another repository moves instead of being mapped twice. * fix(sentinel): the monitor never monitors its own functions A group grew to 5 710 occurrences in 25 minutes, titled after `iii::sentinel-ui::events::tab-…` — the live-update handler the Sentinel page registers from each browser tab. When the console restarted, its tabs left those handlers dead; every group change was delivered to them, the engine logged "Function not found", the log changed a group, and the change was delivered again. One rule now says what is the worker's own — `sentinel::*` and its page's `iii::sentinel-ui::*` handlers — and it applies everywhere an error can come in: a trace's root (where only `sentinel::*` was checked before), an error leaf inside another worker's trace (the console timing out on `sentinel::groups::list`), and logs, judged by service, `function_id` and call site before the trace is even looked up, since the engine's logs about a dead handler carry none. Measured on the dev stack: three calls to missing `iii::sentinel-ui::events::…` functions produced three engine ERROR logs; all three were dropped (`dropped_own_trace` 0 → 3) and no group was created. * feat(sentinel): call them projects, and say where to map one Every place a person reads the word — the settings section, Add project, the chip and the note on a group with none, the Diagnosis tab, the settings list's description — says project instead of repository, the word the console's own folder picker already uses. The note on an unmapped group no longer says only "map it in the configuration": it names the settings icon on the page and the Projects section, and which worker to add. There is no host call to open the form at that field, so the sentence is the way there. The configuration key stays `repositories`: renaming it would strand the value already stored. * feat(sentinel): the configuration key is `projects` too The settings call them projects; the stored key now does as well. `repositories` is still read — a value stored under it keeps working and is folded into `projects` on load — and never written back; both set at once is refused, since which one is meant cannot be guessed. The former key stays in the schema as deprecated and optional, so a stored value that still uses it validates, and the configuration worker accepts values saved under the new one. (An `Option`: the schema generator lists a `skip_serializing` field as required whatever its default, and a required former key refused every save.) The form reads either and saves `projects` only. Defaults, the production override, the README and the spec follow. `sentinel::status` still reports the mapped checkouts as `repositories`; that is a response field, not configuration. On the dev stack the stored value was rewritten under the new key with `configuration::set`; the worker reloaded with both projects intact. * fix(sentinel): read a huge trace by its error spans, not as one frame The worker was dropping its engine connection every two to four seconds. A long harness turn — 2 428 spans, each prompt and reply a payload event — had errors, each error ticked an ingest, and the ingest asked `engine::traces::tree` for the whole trace: an 80 MiB answer the SDK refuses above 16 MiB by closing the socket. Every call in flight failed, the console dropped and re-added the page and its settings form (which is how it showed: the form flickering, the project editor closing by itself), and the next error asked again. A trace past 300 spans is now read by its error spans only, 25 at a time and at most 200, nested by their own parent links — the failure and the chain it propagated up, which is what grouping and evidence use. The evidence counts the spans it left out. The investigation proxy `sentinel::trace::get` goes through the same read, and its optional flat span list is capped. Measured on the dev stack: two minutes with one connection, no oversized frame, 80 jobs processed and none failed, where before the socket had been refused 164 times. * fix(sentinel): the template stack carries the database and the sentinel too * chore(sentinel): drop the tech spec * test(ci): count the sentinel's frontend in the catalog * fix(sentinel): initialize configuration through the atomic ensure
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
1 participant
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.
Summary
iii-lspbinary (cross-platform builds via_rust-binary.yml)iii-lsp-vscodeextension (VS Code Marketplace + OpenVSX)iii-lspandiii-lsp-vscodetocreate-tag.yml(supports both Cargo.toml and package.json manifests)iii-lspto worker registryiii-lsp-vscodeiii-sdkdependency)Required secrets before first release
VSCE_PATOVSX_PATTest plan
iii-lsp, patch bump, dry-run → verify binary buildsiii-lsp, patch bump, real → verify GitHub Releaseiii-lsp-vscode, patch bump, dry-run → verify VSIX packagesiii-lsp-vscode, patch bump, real → verify Marketplace + OpenVSX listingsSummary by CodeRabbit