Add startup immediate poll unit test - #4
Merged
Conversation
frantic-openai
deleted the
frantic/mt-699-schedule-refresh-on-startup
branch
February 27, 2026 02:00
filipeforattini
referenced
this pull request
in forattini-dev/fifony
Mar 17, 2026
- Unread items get subtle state-colored background tint instead of just a border-left — much stronger unread signal - Unread dot (colored circle) replaces border-left as read indicator - Identifier extracted and shown as monospace badge (#4, ISSUE-7) separate from the title text - Title uses line-clamp-2 instead of truncate — shows more context - Dismiss button hidden by default, appears on hover/focus — removes visual clutter in a 320px dropdown - Empty state shows icon + helper text instead of plain "No notifications" - Removed divide-y between items — the tinted backgrounds already create visual separation
ppatel-oai
pushed a commit
that referenced
this pull request
Mar 17, 2026
Co-authored-by: Alicia Sun <@openai.com>
filipeforattini
referenced
this pull request
in forattini-dev/fifony
Mar 20, 2026
filipeforattini
referenced
this pull request
in forattini-dev/fifony
Mar 20, 2026
guangyucoder
referenced
this pull request
in guangyucoder/symphony
Apr 10, 2026
…ker gaps
Three bugs caused ENT-146 to get stuck in Merging with backoff:
1. Verify force-accept let unverified code reach handoff/merge. Now verify
returns {:fail, ...} after 3 attempts, triggering the circuit breaker
→ Human Input Needed (matching AGENTS.md invariant #4).
2. Merge closeout blindly marked phase "done" without checking if the PR
was actually merged. Now calls Verifier.check_pr_merged/1 (gh pr view)
and retries if PR is still open.
3. :merging and :rework dispatch rules lacked replay_current_unit_rule,
so merge/rework retries had no circuit breaker protection. Added replay
as first rule in both flows, plus Merging-entry cleanup to clear stale
non-merge current_unit.
4 tasks
nigel-upstart
referenced
this pull request
in nigel-upstart/sardine-run
May 9, 2026
Five fixes from the five-axis review of feat/sardine-run-migration: #1 Workspace path fallback (correctness): SessionDetailPresenter now falls back to <workspace_root>/<identifier> when the orchestrator entry has no workspace_path. Sessions in their first seconds (before AgentRunner writes back) no longer render :unconfigured for the git log section. #2 Refresh split (performance): observability_updated broadcasts no longer re-run shell-outs. New SessionDetailPresenter.live_payload/2 returns just the snapshot-derived slice; SessionDetailLive merges it with the previous filesystem-derived sections. A separate 10-second :filesystem_tick refreshes git log / log tail / notes / paths. Saves one git-log + one tail invocation per second per open session. #3 Notes size cap (security/perf): notes.md is now read via File.open + IO.binread with a 64 KiB cap. Previously File.read could load arbitrarily large notes into the LiveView socket diff. #4 Real dispatch coverage (correctness): write_session_dashboard_url/2 is now a public Orchestrator function with @SPEC, covered by tests that exercise the actual URL construction (server.port = 4567, server.port = nil, host = 0.0.0.0 rebound to 127.0.0.1, malformed issue_id rejected). The existing tests only called SessionWriter directly and would have missed URL bugs. #5 Dead clause removed: dashboard_url_host/1's first head matched a strict subset already handled by the binary clause's cond block.
nigel-upstart
referenced
this pull request
in nigel-upstart/sardine-run
May 9, 2026
#1 (important) SSR no longer runs filesystem reads. Disconnected mount now assigns :result -> :loading and renders a placeholder. The connected mount runs assign_payload as before. Avoids blocking the conn process for up to 5s on a static render. #2 (important) :error/:loading -> :ok transition does an immediate full assign_payload instead of seeding default filesystem keys and waiting up to 10s for the next :filesystem_tick. No more transient header.workspace_path / paths:hidden inconsistency. #3 (suggestion) read_capped/2 trims trailing bytes that form an incomplete UTF-8 codepoint, so a multi-byte char straddling the 64 KiB boundary doesn't produce invalid UTF-8 in the rendered <pre>. #4 (suggestion) Comment on the :sys.get_state/1 sync barrier in the LiveView tests, explaining it is safe only because handle_info is fully synchronous. #5 (suggestion) Filter run_git_log output to lines matching <sha> <subject> so any stderr (hint:/warning:) that gets interleaved by stderr_to_stdout: true is dropped before render. openai#6 (suggestion) clear_session_dashboard_url/2 now has a @SPEC documenting that it always returns :ok (best-effort), distinct from write_session_dashboard_url/2 which propagates errors.
nigel-upstart
referenced
this pull request
in nigel-upstart/sardine-run
May 9, 2026
* refactor(elixir): replace Linear tracker with traffic_control adapter
Drops the Linear GraphQL adapter, client, and dynamic_tool surface in
favor of a traffic-control-state filesystem adapter. The Tracker
behaviour now dispatches to TrafficControl.Adapter for `kind:
traffic_control` and stays on the Memory adapter for tests.
- Rename `Linear.Issue` to `Tracker.Issue`, drop the rest of the Linear
namespace.
- Stub `Codex.DynamicTool` until sardine-run-specific tools land.
- Make `tracker.kind` accept only `traffic_control` and `memory`,
surface clearer state-repo errors in the orchestrator log.
- Switch the test workflow fixture to traffic_control YAML and add
`make_state_repo!/1` + `write_session_yaml!/3` test helpers.
* test(elixir): convert workspace config tests to traffic_control schema
* test(elixir): convert core tests to traffic_control schema
Drop Linear-specific config tests (LINEAR_API_KEY/LINEAR_ASSIGNEE env
resolution, project_slug validation), update prompt template
assertions to match the new "Traffic Control session" wording, and
adjust tests that relied on legacy active_states defaults to set
explicit active/terminal state lists.
Also rewrite WORKFLOW.md to use the traffic_control tracker schema and
the new prompt header so the in-repo round-trip test stays green.
* test(elixir): rename Linear references in dashboard tests
* test(elixir): drop linear_graphql tool spec assertion and rename tool fixtures
* test(elixir): regenerate status dashboard snapshots for state_repo project link
* build(elixir): drop SymphonyElixir.Linear.Client from coverage ignore list
The Linear client module is gone after the traffic_control migration, so
remove the stale entry from the test coverage ignore list.
* test(elixir): replace Linear-specific dynamic tool and adapter tests
Rewrite the dynamic tool tests to cover the post-migration shape (no tools
advertised yet, structured failure for any tool name). Drop the Linear
adapter delegation tests in favour of asserting the traffic_control
adapter resolves when the workflow selects it.
* test(elixir): drop live Linear/Codex E2E test
The live end-to-end test depends on real Linear API calls and a SYME2E
team to drive issue creation; with Linear removed the file no longer
applies. Delete it rather than leaving Linear scaffolding in the suite.
* test(elixir): add traffic_control adapter unit tests
Cover resolve_state_repo, fetch_candidate_issues, fetch_issues_by_states
with case-insensitive filtering, fetch_issue_states_by_ids, create_comment
note appending, and update_issue_state YAML rewrite + append fallback.
* feat(elixir): advertise sardine_run_session tool spec
Replace the no-op dynamic tool stub with a single-tool advertisement
for sardine_run_session. The tool exposes a multi-op contract
(operation + session_id required, plus per-op fields) so a Codex agent
can update its assigned Traffic Control session via tool/call.
Operations are stubbed for now and return a structured validation
failure; subsequent commits wire each operation through to the
session.yaml/notes.md/links.yaml writers.
* feat(elixir): implement status/note/link/focus/next_step ops
Add SymphonyElixir.TrafficControl.SessionWriter and wire the
sardine_run_session tool's status, note, link, focus, and next_step
operations through to it.
- status: regex-patches the top-level `status:` line and rewrites the
nested `waiting:` block (cleared to `null` when status != waiting).
- note: appends to `sessions/{id}/notes.md` via the existing
Adapter.create_comment/2 helper.
- link: appends an entry to `sessions/{id}/links.yaml`, creating the
file if needed; rendered output round-trips through YamlElixir.
- focus / next_step: regex-patch their top-level scalar lines.
Heartbeat is still stubbed and lands in the next commit.
* feat(elixir): implement heartbeat with sardine_run runtime fields
Wire the sardine_run_session tool's heartbeat operation through to
SessionWriter.update_heartbeat/2. The writer reads the existing
sardine_run block, merges in last_event/last_message/last_error and
the input/output/total token counters when supplied, stamps
last_heartbeat / heartbeat_at (and last_event_at when last_event was
sent) with the current ISO-8601 UTC timestamp, and rewrites the
nested block in session.yaml.
Result still round-trips through YamlElixir.read_from_string/1 so the
dashboard and adapter continue to see a well-formed sardine_run map.
* feat(elixir): mention sardine_run_session in default prompt template
Document the new sardine_run_session dynamic tool in the default
WORKFLOW.md prompt template so agents that fall back to the default
prompt know the tool exists and how to use each operation.
* refactor(elixir): rename lib modules from SymphonyElixir to SardineRun
Renames lib/symphony_elixir{,_web} directories to lib/sardine_run{,_web}
and updates all module identifiers from SymphonyElixir(.Web) to
SardineRun(Web). Updates module aliases in mix tasks accordingly.
* refactor(elixir): update test files for SardineRun rename
Updates test references from SymphonyElixir module names, :symphony_elixir
atoms, /tmp dir prefixes, env var names (SYMPHONY_SSH_CONFIG,
SYMP_TEST_SSH_TRACE, SYMPHONY_LIVE_DOCKER_*), and the
__SYMPHONY_WORKSPACE__ marker to their SardineRun / SARDINE_RUN
equivalents. Renames the e2e docker fixture from symphony-live-worker
to sardine-run-live-worker.
* refactor(elixir): rename mix.exs app to :sardine_run with escript binary sardine-run
Updates mix.exs OTP app from :symphony_elixir to :sardine_run, escript
name and path to bin/sardine-run, and adjusts module references in the
test_coverage ignore list and application mod tuple. Updates Makefile e2e
target env var (SARDINE_RUN_LIVE_E2E) and test path. Updates
config/config.exs endpoint config and signing salt. Updates .gitignore
hex tarball pattern.
* refactor(elixir): update user-facing strings from Symphony to Sardine Run
Renames the user-facing brand "Symphony" to "Sardine Run" in README.md,
AGENTS.md, WORKFLOW.md, and docs/. Updates the GitHub clone URL to
nigel-upstart/sardine-run. Renames the workspace dir convention from
symphony-workspaces to sardine-run-workspaces and the GitHub PR label
from `symphony` to `sardine-run`. Leaves the upstream `openai/symphony`
GitHub URL intact in WORKFLOW.md hooks since it refers to the upstream
repo, not the brand.
* docs(spec): rewrite SPEC.md for Traffic Control tracker and sardine_run_session
Rewrite SPEC.md so it matches the current architecture: Traffic Control
filesystem state-repo as the only real tracker, the sardine_run_session
dynamic tool as the agent's session-mutation surface, and Sardine Run
naming throughout. Drops Linear/linear_graphql sections, the
project_slug/api_key/endpoint config fields, and the GraphQL-specific
tracker semantics. Replaces them with the state-repo layout and writer
contract that the orchestrator actually implements.
* docs(readme): describe Sardine Run / Traffic Control flow at the repo root
Replace the Linear-and-Symphony pitch with the current architecture: a
Traffic Control state-repo as the tracker, sardine_run_session as the
agent's only mutation surface, and `sardine-run` as the escript binary.
* docs(elixir): refresh elixir README and AGENTS.md for new architecture
Drop Linear-flavoured run instructions and replace with the Traffic
Control state-repo + sardine_run_session contract that the running
service actually uses. Update logging conventions to talk about
Traffic Control sessions instead of Linear issues. AGENTS.md now points
agents at SardineRun.Tracker, SardineRun.TrafficControl.SessionWriter,
and SardineRun.Codex.DynamicTool.
* docs(workflow): rewrite WORKFLOW.md template with traffic_control config and updated agent prompt
The YAML front matter already used traffic_control. The prompt body
still pointed agents at Linear, the linear_graphql tool, and a Linear
status map. Rewrite the body so the agent drives Traffic Control
sessions through the sardine_run_session dynamic tool: explicit
operations contract, Traffic Control status values, and notes/links
written via the writer rather than to Linear comments.
* fix(elixir): harden traffic_control writers against path traversal
Reject session_ids that contain path separators or other unsafe chars
in `SardineRun.TrafficControl.SessionWriter` and route the
`Adapter.create_comment` and `Adapter.update_issue_state` Tracker writes
through the writer so all file mutations share one validated path.
Adds dedicated `session_writer_test.exs` covering: invalid ids, comment
lines that should not match `^status:`, missing-newline tail rewrites,
quoted scalars with colons, waiting block transitions, and unknown
sardine_run keys being dropped on heartbeat merge.
* more bootstrapping for local
* fix(codex): update dynamic tool and approval policy for Codex 0.128
- Rename `parameters` → `inputSchema` in `DynamicTool` spec so Codex
app-server accepts the tool definition under the new protocol shape
- Change `approval_policy` schema default from a `%{"reject" => ...}`
map (no longer valid) to the string `"never"`
- Update WORKFLOW.md `after_create` hook to use tag-based multi-repo
detection instead of a hardcoded symphony clone
- Update all tests to match the new defaults and hook content
* feat(workspace): seed per-session repo via host-side after_create hook
Agents were left to clone the target repo themselves from inside Codex's
workspace-write sandbox where networkAccess=false; DNS and .git writes
failed and a handful of workspaces were polluted with the legacy
openai/symphony remote. Move repo resolution + clone into a host-side
script run as the after_create hook (outside the Codex sandbox), and
refuse rather than guess when session data is ambiguous.
- elixir/scripts/seed-workspace.sh: resolve repo from links.yaml
(kind: repo, then kind: pr) then session.yaml cwd; clone via SSH and
checkout session.branch; refuse on multi-repo, wrong-repo, and stale
symphony cases.
- WORKFLOW.md: after_create now calls the seeder; turn_sandbox_policy
override removed so the Elixir default applies (workspace-only writes,
full read access, networkAccess=false). Prompt body updated so the
agent expects a pre-seeded workspace and waits on mismatches instead
of attempting to re-clone.
- SPEC.md §9.6: documents the seeder contract and refusal cases.
- docs/plans/multi-repo-workspace.md: design notes and migration steps.
- core_test: updated assertion to match the new hook command.
In-sandbox network access is tracked separately in
#2.
* fix(seed-workspace): handle `- kind: ...` single-line YAML form
The YAML parser regex required leading whitespace before `kind:` / `url:`,
so links.yaml entries shaped `- kind: pr` (kind on the same line as the
list dash) were skipped — every multi-PR session looked like "no repo
signal" instead of "multiple repos". Allow an optional leading dash on
the kind/url line patterns.
Verified against the four real sessions:
- 19ffb552 (GENAI-158 bulk): correctly refuses with multi-repo (11 PRs).
- 282e6ff2: resolves to teamupstart/claude-code-extensions.
- 4e5bf1ec / c4d6a8e0: unchanged (already worked).
* feat(orchestrator): surface after_create failures into session waiting state
Two related fixes for the multi-repo seeding work:
1. Workspace.maybe_run_after_create_hook/4 wipes the freshly-created
workspace when after_create returns non-zero, so the next attempt
runs the hook from a clean slate instead of skipping it because the
directory already exists.
2. AgentRunner now raises a structured WorkspaceHookFailedError on
after_create failures (instead of a generic RuntimeError), and the
orchestrator's :DOWN handler pattern-matches on it. Failed sessions
are flipped to status: waiting, kind: external via SessionWriter
with the hook output captured in the waiting note, and the issue is
removed from the retry queue. Operators see the refusal in
session.yaml instead of an unbounded retry loop in the logs.
Recovers automatically: once the operator addresses the underlying
issue (e.g. splits a multi-repo session, adds a `kind: repo` link, or
clears the workspace) and flips status back to active, the next polling
tick re-dispatches normally.
* chore(orchestrator): drop redundant hook-name parenthetical from waiting note
* feat(orchestrator): pause dispatch on Codex rate-limit signals
Every Codex turn was hot-failing on a websocket 403 (ChatGPT usage cap),
exhausting 20 turns in seconds and bouncing the session back into the
retry queue. Detect the signal and pause new dispatches instead.
- AppServer.log_non_json_stream_line/2 inspects each non-JSON line for
rate-limit patterns (`usage limit`, `Try again at`, or the 403 cluster
on `chatgpt.com/backend-api/codex`) and forwards a
`{:codex_rate_limited, reason, reset_at}` message to the named
Orchestrator.
- Orchestrator tracks `codex_dispatch_paused_until_ms` /
`codex_dispatch_pause_reason`. While the pause is active,
`maybe_dispatch/1` short-circuits and logs a single info line per poll.
Pauses extend (longest wins) but never exceed 4h. A scheduled
`:codex_dispatch_pause_expired` self-message clears the pause and
triggers a fresh poll cycle.
- Currently-running agents are deliberately left alone — they fail
out via their own 20-turn cap and won't be replaced until the pause
clears, which is enough to stop the spin-up storm without surgery on
in-flight tasks.
- Default pause is 30 minutes; explicit reset times from Codex are
honored when present (parser intentionally minimal — codex's
human-formatted `Try again at ...` text is brittle, so the default
window is the conservative fallback).
Tracked separately from #2: enabling network in the turn sandbox is
still the long-term fix; this change just turns the symptom from a
spin-loop into a clean pause.
* fix(orchestrator): only log rate-limit pause on transition, not every 403
Each 403 call to apply_codex_dispatch_pause was hitting the "true" arm
because monotonic_time advances between sends, so the pause was being
"extended" by a few ms each time and the warning re-logged. Treat
sub-60s extensions as no-ops, log only on initial pause activation, and
emit an info-level line on meaningful (>60s) extensions.
* feat(codex): add network_access config and launch wrapper script
Add a `network_access` boolean field to the Codex schema (default false)
and thread it through all three sandbox policy resolution paths so
WORKFLOW.md can toggle `networkAccess` in the injected turn sandbox
policy without losing dynamic `writableRoots`.
Add `scripts/codex-launch.sh`, a wrapper that injects GITHUB_TOKEN from
`gh auth token` before exec-ing the Codex app-server, enabling `gh` CLI
inside turns where keychain auth is unavailable.
Update WORKFLOW.md to use the launch wrapper as `codex.command` and set
`network_access: true` so turns can reach CodeArtifact, PyPI, and GitHub.
* docs(workflow): add cicd-reusable-workflows to known repos table
Registers github.com/teamupstart/cicd-reusable-workflows so sessions
seeding that repo are not stopped by the "remote does not match
expected repo" guardrail.
* docs(workflow): fix otel repo name in known repositories table
Correct `otel-ai-gateway` to `corp-otel-gateway` to match the actual
GitHub repo name under teamupstart.
* docs(workflow): add ai-accel as alias pattern for ai-acceleration repo
* feat(observability): add SessionDetailPresenter skeleton + identifier validation
Foundational slice (T1) for the per-session drill-down LiveView. Adds
SessionDetailPresenter with two public functions: validate_identifier/1
gates URL-supplied identifiers through the SessionWriter allow-list (and
explicitly rejects "." / ".." / paths containing ".."), and payload/3
returns the live-state slice from an orchestrator snapshot. No
filesystem reads or shell-outs yet — those land in T3-T5.
Spec at docs/session-detail-liveview.md, plan at tasks/plan.md.
* feat(observability): add SessionDetailLive route with live agent state + 404
Slice 1 (T2) of the per-session drill-down. Adds the
`/session/:issue_identifier` LiveView route, mounts the LiveView with a
header, the live agent state section, and the back link. Subscribes to
ObservabilityPubSub and re-fetches the snapshot on broadcasts. Unknown
or invalid identifiers (including path-traversal attempts) render a
"Session not active" page rather than 500-ing.
Render uses a single `~H` head with a `case @result` block so LiveView
change tracking sees `:result` updates and re-renders on broadcasts.
Workspace git log, log tail, notes.md, and on-disk paths land in T3-T5.
* feat(observability): add workspace git log section to session detail
Extends SessionDetailPresenter with git_log_section/2: shells out git log
(max 10 commits, 5s timeout) with PathSafety containment check before any
shell-out. Renders :ok, :empty, :workspace_not_present, :unsafe_workspace,
and :unconfigured states cleanly in SessionDetailLive.
Also applies mix format to pre-existing files that had formatting drift.
* feat(observability): render workspace git log on session detail page
Slice C3 of the per-session drill-down. The presenter now exposes
git_log_section/2, which runs `git log --pretty=format:%h %s
--max-count=10` against the agent's workspace inside a Task with a
5-second hard timeout. Stderr is captured and dropped — never leaked
into the page.
Status branches: :ok, :empty (non-repo or git error), :workspace_not_present
(missing dir), :unsafe_workspace (path not contained in
workspace.root after canonicalization via PathSafety), :unconfigured
(no workspace_path or workspace_root). Each maps to a distinct
empty-state line in the LiveView.
The LiveView passes %{workspace_root: settings.workspace.root} into
the filesystem map so the containment check has the configured root.
* feat(observability): add filtered log-tail section to session detail
Adds log_tail_section/2 to SessionDetailPresenter: tails up to 5 MiB
of the log file, filters lines by session identifier substring, and
returns the last 200 matches oldest-first. Hard 5-second timeout;
missing file -> :empty, no matches -> :no_entries, tail failure ->
:error. Renders as a scrollable monospace pane in SessionDetailLive.
* feat(observability): add notes.md and on-disk paths to session detail
Slice C5 of the per-session drill-down. The presenter exposes
notes_section/2 (reads <state_repo>/sessions/<identifier>/notes.md and
returns :ok+content, :missing, or :memory_tracker when state_repo is
nil) and paths_section/3 (returns the four absolute paths or :hidden
under the memory tracker).
The LiveView resolves state_repo via TrafficControl.Adapter and
threads it into the filesystem map. Notes render in a <pre> block; on-disk
paths are listed verbatim and hidden when no state_repo is configured.
Adds also the C4 presenter tests I had ready (filtered log-tail
section): file-with-mixed-lines, 200-line cap, missing-file, and
nil-log_file branches. C3 presenter tests for git log are kept and
extended with the unconfigured-workspace and out-of-root cases.
* feat(observability): add notes and on-disk paths sections to session detail
Adds notes_section/2 (reads sessions/<id>/notes.md from the TC state
repo; degrades to :memory_tracker when tracker.kind: memory) and
paths_section/3 (the four on-disk paths: session.yaml, notes.md,
links.yaml, workspace; hidden as :hidden in memory tracker mode) to
SessionDetailPresenter. Both sections render in SessionDetailLive.
LiveView's filesystem_context now resolves state_repo via
TrafficControl.Adapter.resolve_state_repo/0.
* feat(observability): add View session drill-down link to dashboard rows
Slice C6 of the per-session drill-down. Each row in the Running and
Retrying tables on the / dashboard now renders a "View session" link
pointing at /session/<issue_identifier> alongside the existing JSON
details link. Existing dashboard liveview test extended with regression
asserts for both the running and retrying row links.
* chore(tasks): mark C3-C6 complete in session-detail todo
* docs(observability): document /session/<id> drill-down route
Add mentions of the new per-session detail page in the observability dashboard documentation across root README, elixir/README, and SPEC. The drill-down route surfaces live agent state, workspace git history, session logs, notes, and on-disk paths.
* feat(observability): write dashboard_url to session runtime on dispatch
When the orchestrator dispatches a Codex agent for an issue it now writes
the per-session deep-link URL (http://<host>:<port>/session/<identifier>)
into sardine_run.dashboard_url in the TC session.yaml via a new
SessionWriter.update_runtime/2 function. On agent termination (normal
completion, error, or retry paths) the field is cleared back to null so TC
knows the orchestrator is no longer live for that session.
If the HTTP server is disabled (server.port is nil) the orchestrator
writes nil, which update_runtime/2 renders as dashboard_url: null.
* chore(tasks): mark D and F complete in session-detail todo
* docs: document dashboard_url runtime field and PUBLIC_HOSTNAME env var
Adds spec coverage for §13.7.4 (dashboard URL advertisement on session
runtime) and corresponding mentions in both READMEs so external tools
know the back-link contract and the SARDINE_RUN_PUBLIC_HOSTNAME override.
* docs(plan): unified spec + plan for needs-attention triage and deep-link
* chore(tasks): close out Phase 4 + Phase 5
* test(observability): cover orchestrator :DOWN clear path for dashboard_url
The existing orchestrator_dashboard_url_test.exs was misnamed — every
test in it called SessionWriter directly, never the orchestrator. Adds
a real integration test that starts an Orchestrator GenServer, injects
a running entry under a tracked ref, sends {:DOWN, ref, :process, _,
:normal}, and asserts session.yaml on disk has dashboard_url cleared
to nil with sibling sardine_run fields preserved. Proves the wiring
claim of d143c61 / 1e757e9.
* fix(observability): apply review fixes for session detail page
Five fixes from the five-axis review of feat/sardine-run-migration:
#1 Workspace path fallback (correctness): SessionDetailPresenter now
falls back to <workspace_root>/<identifier> when the orchestrator
entry has no workspace_path. Sessions in their first seconds (before
AgentRunner writes back) no longer render :unconfigured for the git
log section.
#2 Refresh split (performance): observability_updated broadcasts no
longer re-run shell-outs. New SessionDetailPresenter.live_payload/2
returns just the snapshot-derived slice; SessionDetailLive merges it
with the previous filesystem-derived sections. A separate 10-second
:filesystem_tick refreshes git log / log tail / notes / paths. Saves
one git-log + one tail invocation per second per open session.
#3 Notes size cap (security/perf): notes.md is now read via File.open
+ IO.binread with a 64 KiB cap. Previously File.read could load
arbitrarily large notes into the LiveView socket diff.
#4 Real dispatch coverage (correctness): write_session_dashboard_url/2
is now a public Orchestrator function with @SPEC, covered by tests
that exercise the actual URL construction (server.port = 4567,
server.port = nil, host = 0.0.0.0 rebound to 127.0.0.1, malformed
issue_id rejected). The existing tests only called SessionWriter
directly and would have missed URL bugs.
#5 Dead clause removed: dashboard_url_host/1's first head matched a
strict subset already handled by the binary clause's cond block.
* test(observability): cover :filesystem_tick refresh and not-found→found transition
Two coverage holes from the post-fix audit:
1. The new :filesystem_tick handler (added in 340d41f to throttle
shell-outs) had no test asserting it actually re-runs the
filesystem reads. Add a test that mounts, makes a new git commit,
sends :filesystem_tick, and asserts the new commit appears in the
rendered git log section.
2. refresh_live_state/1 has a fallback branch for when the previous
:result was an error tuple (issue not yet in snapshot at mount
time, then appears via PubSub broadcast). Add a test for the
not-found→running transition that asserts the page transitions
from "Session not active" to the live agent state view without
stale filesystem keys leaking.
Both tests pass on first run — no bugs, just integration claims that
were previously undocumented.
* fix(observability): apply six follow-up review findings
#1 (important) SSR no longer runs filesystem reads. Disconnected mount
now assigns :result -> :loading and renders a placeholder. The
connected mount runs assign_payload as before. Avoids blocking the
conn process for up to 5s on a static render.
#2 (important) :error/:loading -> :ok transition does an immediate
full assign_payload instead of seeding default filesystem keys and
waiting up to 10s for the next :filesystem_tick. No more transient
header.workspace_path / paths:hidden inconsistency.
#3 (suggestion) read_capped/2 trims trailing bytes that form an
incomplete UTF-8 codepoint, so a multi-byte char straddling the 64
KiB boundary doesn't produce invalid UTF-8 in the rendered <pre>.
#4 (suggestion) Comment on the :sys.get_state/1 sync barrier in the
LiveView tests, explaining it is safe only because handle_info is
fully synchronous.
#5 (suggestion) Filter run_git_log output to lines matching
<sha> <subject> so any stderr (hint:/warning:) that gets
interleaved by stderr_to_stdout: true is dropped before render.
openai#6 (suggestion) clear_session_dashboard_url/2 now has a @SPEC
documenting that it always returns :ok (best-effort), distinct
from write_session_dashboard_url/2 which propagates errors.
chihsuan
referenced
this pull request
in Automattic/symphony
May 15, 2026
…rkspace-strategy-as-alternative-to Add git worktree workspace strategy
corylanou
referenced
this pull request
in digitaldrywood/symphony-elixir
May 18, 2026
P1 correctness (#2): poll query was requesting fieldValues(first: 20) and treating a missing Status as nil. Switch to fieldValueByName("Status") so projects with many custom fields can't drop Status off-page. P1 correctness (#4): same pagination hazard in fetch_issue_states_by_ids. Replaced fieldValues(first: 20) with fieldValueByName, and bumped projectItems(first:) to 100 to cover the rare issue-in-many-projects case. P1 correctness (#1): tracker.assignee was silently ignored; every GitHub issue was reported as assigned_to_worker: true. Implement assignee matching against GitHub login, with "me" supported via a viewer GraphQL query (mirroring the Linear adapter's pattern). P2 performance (#6): github_to_symphony_state called Config.settings!() once per normalized item. Build a normalization context at the fetch entry point and thread state_map / project_id / assignee_filter through the decode pipeline so config is read once per fetch, not once per issue. P2 correctness (#7): build_identifier dropped the owner, collapsing owner-a/app#1 and owner-b/app#1 to the same "app#1". Use the full nameWithOwner so identifiers are unambiguous across orgs. P3 DX (#8): orchestrator always logged "Failed to fetch from Linear" on generic tracker errors and only knew Linear-specific missing-token / missing-project atoms. Add :missing_github_api_token and :missing_github_project_id branches with actionable messages, and make the generic fallthrough name the configured tracker kind.
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.
Context
Startup refresh should run immediately after app boot so users don't wait an extra polling interval on initial load.
TL;DR
Add a unit test proving Orchestrator starts a poll cycle immediately on startup.
Summary
Alternatives
schedule_tick(0)on init.Test Plan
make -C elixir allmix test test/symphony_elixir/orchestrator_status_test.exs:413 test/symphony_elixir/orchestrator_status_test.exs:456 test/symphony_elixir/orchestrator_status_test.exs:499