Batch Guardian log writes and drain them on shutdown - #112
Open
rksharma-owg wants to merge 1 commit into
Open
rksharma-owg wants to merge 1 commit into
rksharma-owg wants to merge 1 commit into
Conversation
Signed-off-by: RKS <rajesh.sharma@owasp.org>
rksharma-owg
requested review from
GangGreenTemperTatum,
afogel,
almogbhl,
bar-capsule,
fewdisc,
mamicidal,
rocklambros,
sclintonowasp and
stefanoamorelli
as code owners
September 13, 2026 05:22
rocklambros
added a commit
that referenced
this pull request
Sep 17, 2026
… one pass (#158) Follow-up to #157, fixing a defect I found by testing the merged workflow rather than one a reviewer would have caught from the diff. ## The defect A reconcile run that adds items cannot also set their Status. Adding an item triggers project 9's own `Item added to project` workflow, which stamps a default Status asynchronously. `apply_governance.py` builds its entire plan before executing any of it, so on a run that adds anything it has already decided the Status actions before those items exist on the board. The default lands after the run finishes and nothing corrects it until the next run. ## Measured, not theorised On the 2026-09-17 backfill the first pass made **40 adds and 1 Status change**. A second pass run immediately afterwards found **5 more Status corrections waiting**: | Item | Why the default was wrong | | --- | --- | | #135 | `status:accepted` | | #131 | `status:accepted` | | #126 | open pull request, belongs in In progress | | #113 | open pull request | | #112 | open pull request | None of those five has the default as its correct Status, which is precisely the population a single pass strands. A third pass reported zero actions, so the tool converges, it just cannot converge in one run when it is also adding. Nightly, that is up to 24 hours of a board reading Needs triage for work that is accepted or in flight. That is the failure the board exists to prevent, so shipping #157 without this would have left the automation quietly wrong in the one case it most needed to be right. ## The fix Run the board step twice under `--apply`. The dry-run path still runs once and returns, since printing the same plan twice helps nobody. The reconciler is idempotent, so the second pass costs one API listing on a run that added nothing. Once `Auto-add to project` is enabled on the project, that is every run. ## Note on review I did not admin-merge this one. #157 was merged under the administrative authorisation given for it specifically, and extending that to a second change on my own judgement is the kind of thing that should be asked for rather than assumed. The workflow is inert until the App and environment exist, so there is no urgency in merging this ahead of a normal review. Signed-off-by: Rock Lambros <rock@rockcyber.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What changed
Guardian envelope and session-context logs currently call
appendFileSyncon the request path. Batch their existing JSONL records through bounded asynchronous append streams, and drain both streams after active requests finish during shutdown.Each sink flushes after 64 records or 100 ms from its first queued record. Both queued and in-flight data count toward limits of 4 MiB and 4,096 records. A failed or full sink reports once and disables itself without changing the policy decision. SIGINT and SIGTERM use the same drain path as
guardian.close().Log schemas and record ordering stay the same, but log visibility is delayed. Abrupt termination can lose buffered records, and graceful shutdown has no forced deadline. The Guardian README documents these limits. This implements batching for existing reference-implementation logs; OpenTelemetry collection/export remains separate work in #91.
Which issue does this implement
Closes #90
Base branch
integration, because this touches a reference implementation and testsType of change
I tested this
uv run pytest -v: 278 passed, 1 skippeduv run mkdocs build --strict: passedreference-implementations/agt,bun run typecheck: passedreference-implementations/agt,bun testwith Bun 1.3.11: 1,122 passed, 1 skipped, 0 failedThe burst-buffering regression fails against the synchronous implementation and passes with this change. Tests cover batch thresholds, sparse traffic, record order, UTF-8 byte and record limits including in-flight writes, write failures, duplicate error reporting, active-request shutdown, and standalone SIGINT/SIGTERM shutdown.
The existing Bun skip requires
UPSTREAM_BUNDLEfor a pinned Rego byte-identity comparison. The Python skip requires a case-sensitive filesystem. Both also skipped on the baseline.Checklist
git commit -s(required by the DCO)Security
This changes log buffering and shutdown durability. It does not change policy decisions or report a vulnerability. Abrupt termination and sink errors can lose buffered log records, as documented; buffering is bounded to limit memory use.