Merge branch 'main' into only-ouc/examples/plan-research

Author: Shashankss1205

.github/workflows/ci.yml

@@ -4,6 +4,13 @@ on:
   push:
     branches: [main]
   pull_request:
+  # Mondays, 06:00 UTC. Every other trigger here is caused by someone pushing,
+  # so without this the suite is only ever run against the dependency versions
+  # `uv.lock` pins -- see the `upstream-drift` job for what that hides. Note
+  # GitHub disables a scheduled workflow after 60 days with no repository
+  # activity, so a long quiet spell stops these runs rather than failing them.
+  schedule:
+    - cron: "0 6 * * 1"
   workflow_dispatch:
 
 permissions:
@@ -27,10 +34,20 @@ jobs:
         run: uv sync --all-extras --group dev
       - name: Lint
         run: uv run ruff check .
-      - name: Version is declared twice; the two must agree
+      - name: Version is declared three times; all three must agree
         # `grapharc.__version__` is a separate literal from the packaged
         # version. If they drift, `pip show` and `import` disagree about what
         # is installed. Parsed rather than imported, so this needs no deps.
+        #
+        # `uv.lock` carries a third copy, in its own entry for this project.
+        # This step checked only the first two, and the third drifted: 0.1.6
+        # and 0.1.7 both shipped with a lockfile saying 0.1.5, because nothing
+        # re-locked after the bump and nothing looked. `uv lock` fixes it in
+        # one line; what this catches is the next one. A stale copy there is
+        # milder than the other two -- it misreports the project to anyone
+        # reading the lockfile, and to `uv sync --locked`, rather than to an
+        # installed import -- but it is the same class of bug, and this step
+        # exists because a version declared in N places drifts in N-1 of them.
         run: |
           python3 - <<'PY'
           import ast
@@ -55,7 +72,20 @@ jobs:
               sys.exit("grapharc/__init__.py no longer declares __version__")
           if declared != packaged:
               sys.exit(f"grapharc.__version__ is {declared!r} but pyproject says {packaged!r}")
-          print(f"ok: version {packaged} declared in both places")
+
+          with open("uv.lock", "rb") as fh:
+              lock = tomllib.load(fh)
+
+          entries = [p for p in lock.get("package", []) if p.get("name") == "grapharc"]
+          if len(entries) != 1:
+              sys.exit(f"uv.lock has {len(entries)} entries for grapharc; expected exactly 1")
+          locked = entries[0].get("version")
+          if locked != packaged:
+              sys.exit(
+                  f"uv.lock says grapharc is {locked!r} but pyproject says "
+                  f"{packaged!r} -- run `uv lock` and commit the result"
+              )
+          print(f"ok: version {packaged} declared in all three places")
           PY
 
   live-marker-guard:
@@ -167,6 +197,85 @@ jobs:
         # `addopts` in pyproject.toml supplies `-m 'not live'`.
         run: uv run pytest
 
+  upstream-drift:
+    # `uv.lock` is what every other job resolves against, so nothing in this
+    # workflow would notice a new `langgraph` minor breaking the runtime until a
+    # user on a fresh `pip install grapharc` hit it. Issue #103 is the standing
+    # form of that complaint: the extras are unbounded and the lockfile hides
+    # what the next major would do. This job re-resolves every range from
+    # scratch and runs the suite against the newest versions the constraints in
+    # pyproject.toml actually allow.
+    #
+    # Scheduled and manual only, deliberately. A pull request has to be judged
+    # against the lockfile it ships; if this ran on PRs, an upstream release on
+    # the morning of a review would turn someone else's branch red for a reason
+    # that branch did not cause.
+    name: upstream drift (unlocked deps)
+    if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'
+    runs-on: ubuntu-latest
+    # `issues: write` is for the reporting step below; a job's permissions
+    # replace the workflow's rather than adding to them, so `contents: read`
+    # is repeated here.
+    permissions:
+      contents: read
+      issues: write
+    steps:
+      - uses: actions/checkout@v4
+      - uses: astral-sh/setup-uv@v5
+        with:
+          python-version: "3.12"
+          enable-cache: true
+      - name: Re-resolve every dependency to the newest allowed version
+        run: uv lock --upgrade
+      - name: Report what moved
+        # Printed whether or not the suite then passes: a green run against
+        # moved dependencies is the useful half of this job, because it is the
+        # evidence that a range can be widened or a pin dropped.
+        run: git --no-pager diff --stat -- uv.lock
+      - name: Sync and test against the re-resolved versions
+        run: |
+          uv sync --all-extras --group dev
+          uv run pytest
+      - name: Say so where someone will see it
+        # A scheduled run reports to nobody. This repository has already paid
+        # for that: `pages.yml` failed on two consecutive pushes and the
+        # failures sat unnoticed for the better part of two months, while the
+        # published site served a version six releases behind. A weekly job in
+        # a repository that goes quiet for weeks at a time is the same shape,
+        # so the failure comes to the issue tracker instead of the Actions tab.
+        #
+        # One issue, reused: an unattended weekly job that opens a fresh issue
+        # every Monday is a second way of being ignored. Matched on title
+        # rather than a label, so this needs no label to exist first.
+        if: failure()
+        env:
+          GH_TOKEN: ${{ github.token }}
+          TITLE: "upstream drift: the suite fails against re-resolved dependencies"
+          RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
+        run: |
+          body=$(cat <<EOF
+          The weekly \`upstream-drift\` job failed: the suite does not pass against the
+          newest dependency versions \`pyproject.toml\` allows.
+
+          Run: $RUN_URL
+
+          **This is not a failure of any branch.** Every other job resolves against
+          \`uv.lock\`; this one re-resolves from scratch. So either an upstream release
+          broke something, or a range in \`pyproject.toml\` needs narrowing — which is
+          what #103 asks for. The job prints the re-resolved lockfile diff, so the run
+          log says which dependencies moved.
+          EOF
+          )
+          existing=$(gh issue list --state open --search "$TITLE in:title" \
+            --json number --jq '.[0].number // empty')
+          if [ -n "$existing" ]; then
+            echo "commenting on existing issue #$existing"
+            gh issue comment "$existing" --body "$body"
+          else
+            echo "opening a new issue"
+            gh issue create --title "$TITLE" --body "$body"
+          fi
+
   build:
     name: build and check the distribution
     runs-on: ubuntu-latest

CHANGELOG.md

@@ -7,6 +7,32 @@ and "used to be true" — the two things a reader most needs kept apart.
 
 Entries are newest-last within a release, matching the order they were written.
 
+## 0.1.8
+
+- **the MCP `execute` timeout could SIGKILL an approved mutating run mid-mutation, then tell the agent it was safe to reissue.** One budget, `approval_timeout + 120s`, covered both halves of a mutating call: the park, where a human is being asked, and the work, where an approved graph runs. So a human who answered near the end of the park left roughly two minutes for the run itself — and a governed run's agent phases delegate to Claude Code, which reads files, edits them and verifies. The likely outcome was not a wedged process being cleaned up; it was a human-approved, tree-mutating run being killed partway through mutating the tree. The non-mutating branch passed `timeout=None` and so was not bounded at all. The tool's own docstring then told the calling agent that "a timeout leaves the plan unexecuted and this call safe to reissue", which is true of a park that expired unanswered and false of everything else — and the #100 reissue guard could not catch the reissue either, because `executed_run_id` is stamped only after `loop.run()` returns, so a killed run leaves a record saying it never executed. Three changes, one per failure. The park and the work now draw on separate budgets, the work's being `DEFAULT_WORK_TIMEOUT` (1800s, the number `GRAPHARC_SLACK_WORK_TIMEOUT` already uses for this exact case) and overridable by `GRAPHARC_MCP_WORK_TIMEOUT`, which refuses a non-numeric or non-positive value rather than substituting a ceiling nobody chose. The reissue guard reads the **trace** — written as the run proceeds, and so the only place evidence of a half-run survives a kill — rather than pre-stamping the record, which would refuse a plan a human had merely *denied*. And the kill reaches the whole process group (`start_new_session=True` plus a group SIGKILL), because `process.kill()` signals the direct child only and a delegated Claude Code process was outliving the run that spawned it, still holding the workspace. The phase vocabulary the trace check depends on now has one owner in `observe.trace` instead of a copy in each of four modules; an unlisted phase reads as an execution, which is the safe direction — a new bookkeeping phase makes `go` refuse a plan it could have run, recoverable with `--again`, where the opposite would re-run a half-finished mutating plan and spend a human approval given once.
+- **the live view streamed a finished run as a running one, forever.** `frames()` polls the trace file's size, rebuilds a snapshot when it changes, and stores that snapshot's `size` as "everything up to here has been sent". `build_snapshot` took that number from `path.stat()` *after* reading the events, and the two disagree in the direction that matters: the read stops at the last newline, and — this being a live view — the run is appending while the snapshot is built, so bytes can land between the read and the stat. The cursor then claimed events the snapshot never saw, the next poll found an unchanged file size and rebuilt nothing, and once the writer stopped the file never changed size again. Reproduced rather than argued: with one append landing in that window the cursor came back *equal to the file's final size* while `done` was still false — the exact pair that leaves the stream with no reason to rebuild and nothing left to learn. `TailRecorder.read_tail()` now returns the events and the byte count they came from, `read_events()` keeps its signature over it for every other caller, and only the mtime is still read from the file's current state — safe to be too new, because it delays an idle verdict by one poll rather than stopping the stream. The cursor now falls short of the file, so the next poll rebuilds and the missed event arrives.
+- **an interrupt landing inside the deadline guard's own teardown left a timer raising into the thread forever.** `fire` queues its asynchronous exception while holding the guard's lock, so a guard already blocked on that same lock inside `disarm` is handed the exception the moment it acquires it — at the next bytecode, which is before `armed` is cleared and before the re-armed timer is cancelled. `disarm` propagated, and the 50ms timer it was meant to cancel stayed alive still reading `armed` as true: it re-raised `NodeDeadlineExceeded` into that thread every 50ms for the life of the thread, long after the run that armed it had finished. On a pooled thread that is an unattributable crash in whatever ran next — which is precisely what `test_no_interrupt_survives_the_node_that_earned_it` exists to rule out, reached by a path it did not cover. The lock was never the flaw: both sides do take it, and what a lock cannot do is stop an asynchronous exception arriving between two bytecodes inside the section it protects. The teardown is retried rather than abandoned now, and clears `armed` outside the lock as a last resort, that single store being what stops `fire` re-arming; swallowing the interrupt there costs nothing, because the guard still decides the outcome from whether the timer fired and still raises on it. The SIGALRM mechanism is unaffected — CPython runs the Python-level handler at a bytecode boundary, so `setitimer(ITIMER_REAL, 0)` has already completed when the handler raises.
+- **the lockfile's copy of the version drifted, twice, unnoticed.** `ci.yml` checks that `pyproject.toml` and `grapharc.__version__` agree, and its comment says why: if they drift, `pip show` and `import` disagree about what is installed. There is a third copy — `uv.lock` carries an entry for this project — and both 0.1.6 and 0.1.7 shipped with it reading `0.1.5`, because nothing re-locked after the bump and no job looked. Milder than the other two, since it misreports the project to a reader of the lockfile and to `uv sync --locked` rather than to an installed import, but the same class of bug as the one that step already guards. The check now parses `uv.lock` as well, names `uv lock` as the remedy, and refuses anything other than exactly one entry for the project so a rename cannot make it silently vacuous.
+- **an imported retraction came back as current on the LadybugDB backend.** `superseded_by` was reconstructed from the `SUPERSEDED_BY` edge, and `add()` cannot create an edge towards a claim the store does not have yet — in the natural import order it does not have it, because `all_claims()` returns oldest-first, so a superseded claim arrives before the claim that superseded it. The edge was skipped while `superseded_at` was written, so one row said both things at once: `superseded_at` said retracted and `superseded_by IS NULL` said current, and `Claim.is_current` believed the second. `current("svc")` — which is supposed to answer "what is true now" — returned *both sides of a correction*, and the two other backends disagreed with this one. Every import, replay, backup restore and copy between backends went through that path; `supersede()` never did, which is why every test that exercised it passed. `superseded_by` is a column now and the edge is derived from it: reads project the column, `current()` filters on the column rather than on an `OPTIONAL MATCH`, and `_write` reconciles the edge in both directions — forward when this claim names a superseder that is present, backward when a stored claim names *this* one and could not have an edge until now — so the edge is complete once both ends have arrived, whatever order they arrived in, and it stays walkable for the Cypher path this backend exists for. Two alternatives were costed and rejected: failing closed breaks replaying `all_claims()` in the order it hands you, and a stub target node reads back as a `ValidationError` and has no `seq`. The column needs a schema change, which `CREATE NODE TABLE IF NOT EXISTS` will not perform on an existing database, so opening one adds the column and backfills it from the edges an older database does have — `supersede()` always wrote those. What `add()` dropped is not recoverable; what the edges recorded is.
+- **a session whose runner died stayed wedged forever.** Not a defect so much as a limitation the package documented and then could not do anything about, which comes to the same thing for whoever hits it. A runner's claim on a session is a compare-and-set, not a lease: it stops a *second* runner from claiming a session and cannot notice one that died holding it. So a crash mid-turn left the row `running` — with the dead `runner_pid` still on it, meaning the store knew exactly who died — and every later `Session.run()` raised `SessionBusy` for the life of the file. The only remedy was a hand-written `UPDATE`, which skips the lifecycle validation and writes no transition row, corrupting the audit trail the store exists to keep. `SessionManager.reclaim(session_id)` releases one deliberately now: it refuses unless the recorded pid is gone from this host, moves the session to `failed` — not `interrupted`, because a turn that died left no point to be picked up from — and records the release as a transition naming the dead pid, so the reclaim is in the history rather than hidden from it. **An open approval hold survives**, so reclaiming a wedged session is not a way past a human. Four things are refused, each deliberately: a session that is not running, a pid that is still alive (or belongs to another user, which is read as alive), a pid that is this process, and a `running` row carrying no pid at all. Pid reuse is not solved and is not pretended to be — a recycled pid reads as alive, which is a refusal, and that is the direction to fail in. Nothing calls this automatically for the same reason: an automatic sweep would be a way for two live runners to fight over one session, which is what the claim exists to prevent.
+
+Tooling, in the same release and not defects in the shipped package: the suite
+now runs weekly against dependencies re-resolved from scratch rather than only
+against `uv.lock`, and a failure opens or updates an issue instead of reporting
+to an Actions tab nobody is watching — `pages.yml` had failed on two
+consecutive pushes and sat unnoticed for the better part of two months, which
+is the failure mode that step exists to avoid. And the **Verified this pass**
+figure in the deep dive can be re-derived with
+`GRAPHARC_UPDATE_FIGURES=1 pytest tests/test_deep_dive.py` instead of edited by
+hand: the check stays strict in CI, but it had been failing every branch that
+added a test until someone hand-edited a number in a docs file they had no
+reason to know existed, which cost an outside contributor's first pull request
+a month of being red for a reason that was not its code. The styling gate — that
+stripping the escapes from terminal output reproduces piped output exactly — now
+covers `run`, `trace` and `metrics` as well as the four commands it started
+with; those three are the ones most likely to be piped, and a leak confined to
+any of them left the old gate entirely green.
+
 ## 0.1.7
 
 - **admission accepted a sentinel pointing the wrong way.** `START` is the graph's entry and `END` its exit, but the endpoint check tested only *whether* an endpoint was a sentinel, never which side of the edge it sat on. So a proposal carrying `END -> x` or `x -> START` was admitted and then died in `Materializer` with `StateGraph`'s own "END cannot be a start node". The run does not proceed either way; what was wrong is where the failure landed. `GovernedLoop` charges a `MaterializationError` to `max_consecutive_execution_failures` (2) rather than `max_consecutive_rejections` (3), so a planner got *fewer* retries for a mistake admission is supposed to catch than for one it does catch, and two in a row ended the run as `EXECUTION_FAILED` — a stop reason claiming the graph ran when nothing had. And a rejection is meant to be data: `feedback()` hands the planner codes and remedies, where this handed it prose scraped off an exception, with nothing on the `admission` event's failed-check list because admission had not failed. The prompt already told models the rule; the gate is what did not hold when one ignored it. Refused now under `Check.REGISTRY` as `sentinel_wrong_direction`, with a remedy naming the side the sentinel belongs on, and both endpoints still reported rather than the first.