fix(redaction): skip gh run view job/step prefixes - #1053
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Next action
Mark this pull request ready for review, then merge it instead of #1040, #1038, #1031, or #929.
This head skips the gh run view --log-failed job<HTAB>step<HTAB> collector prefix plus RFC 3339 Z / time-numoffset and SPACE or HTAB, so a pretty-printed password dump in the actual failed-check excerpt keeps ##[group] and status text. Skipped job/step fields that remain inside a rewritten span still pass unstructured redaction. A two-tab TSV without a timestamp is not eaten as a runner prefix.
Local focused selection: 177 passed; 100% statement/branch on the three owned modules (1,136 statements, 464 branches).
Do not self-approve. After it is green, treat [REDACTED] as evidence suppression and rerun the exact-head sandbox redaction quality job before closing #908.
Sent by Cursor Automation: Fix Issues
5b66ccd to
4011099
Compare
|
Rebased the five-commit redaction series onto current origin/main c47bee5, preserving the RFC 3339 runner metadata and gh run view job/step-prefix handling. Current HEAD: 4011099 Validation on this HEAD:
@OpenCode review this exact HEAD after the rebase. |
Replay unique #929 source onto current origin/main. Skip shared ARCHITECTURE/CLAUDE/AGENTS trees and stale materialize files.
Treat only plausible JSON openers as span starts and score a failed parse against the window until the next opener. A ##[group] marker or prose [timeout] no longer fail-closes a later pretty-printed password object. Cite RFC 8259, ECMA-404, and ISO/IEC 21778 for duplicate-member layout. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Skip RFC 3339 runner timestamps inside raw JSON spans and open '[' only for a real JSON value so a downloaded pretty-printed password dump keeps Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Z-only prefixes still fail-closed a pretty-printed password dump when the collector emitted time-numoffset or a tab after the timestamp. Skip those RFC 3339 §5.6 forms the same way Z plus SPACE is skipped. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Keep failed-check excerpts visible when collect_failed_check_evidence.sh feeds gh run view --log-failed lines (job HTAB step HTAB plus the zip RFC 3339 timestamp, including UNKNOWN STEP). Redact credential-shaped job/step fields that remain inside a rewritten JSON span. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
4011099 to
cd4b30e
Compare
|
Rebased PR #1053 onto current protected main and revalidated the sandbox log-redaction boundary. The collector skips only the verified GitHub CLI job/step prefix plus RFC 3339 metadata; ordinary two-tab text, UNKNOWN STEP, offset timestamps, continuation spans, and credential-shaped names remain visible to redaction. Proof at this exact head: 177 focused security/redaction tests passed; redaction, sandbox verification, and sandbox E2E production modules have 100% interrogate coverage; compileall, quality-workflow actionlint, and git diff --check passed. The PR remains Draft. Please rerun independent review and protected Checks before readiness or merge; no bypass or merge is claimed. |
|
Current-head merge repair pushed normally at |
|
Exact-head lock repair pushed as 15fa991 from 8d948ab. Updated the hashed pip audit runtime from pip 26.1.2 to pip 26.2.1 with exact hashes, without changing the hourly caller implementation. The same lock has already passed local pip-audit in this maintenance loop; hosted checks and exact-head approval must be re-evaluated. No bypass or force push was used. |
|
Dependency Review support returned HTTP 502 from the GitHub dependency-graph compare endpoint before a dependency result was produced. This is provider infrastructure evidence, not a source finding. I am rerunning the failed hosted job and will keep the required gate fail-closed. |
opencode-agent
Bot
added
priority: medium
|
Triage (2026-09-02): base history is orphaned, not a resolvable merge conflict This PR's base commit
The Next action (needs a human/design call): either (a) reimplement the intent of this PR as a fresh branch cut from current Generated by Claude Code |
|
Retraction — the "main history splice" diagnosis was wrong. An earlier comment on this PR claimed Root cause: the analysis was run from a shallow git clone in the analysis sandbox ( This PR should be triaged the ordinary way: fetch a full (non-shallow) clone, Apologies for the incorrect guidance. See #1722 for the corrected writeup. Generated by Claude Code |
…s upstream fix Residual (iii) of the retry-stacking follow-up recorded that the 90 s seen on every stalled review request is "the transport's recv default, not a deadline this repository set", and left open whether a preflight probe deadline is a policy value distinct from the inference deadline. Half of that is no longer open. The default is `ModelClient.__init__(timeout: int = 90)` at `contextual_orchestrator/orchestrator.py:1696`, read at `contextual-orchestrator@414f2297` — the SHA the central sidecar is pinned to — and `contextual-orchestrator#1053` changes exactly that signature to `timeout: float | None = None`, propagating the `None` through `_local_provider_slot`'s deadline arithmetic and, per its diff stat, through `endpoint_race.py`, `cost_router.py`, `batch_routing.py`, `server.py`, and the synchronous embedding path. So the inference-path half has a claimed upstream fix and only the preflight-probe deadline stays open in this repository. The entry also records that `#1053` and `#1082` do not subsume each other — `#1053` removes the 90 s attempt, `#1082` records and classifies a transport failure when one still arrives — and that the sidecar pin must advance past whichever lands last. Both are other lanes' active work; this is a dependency note, not a claim. Also merges `origin/main` (`dd0b96fe`) into the branch, which was `behind`. Verification: full suite 2956 passed, 1 skipped, 21 subtests passed; `git diff --check` clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
… not describe
`noema-review` failed on this PR's own head at 15:37Z (`9c010fcb`, run
34035522521, job 101501520756):
Noema gateway transport failed: HTTPError: HTTP Error 502: Bad Gateway;
caller attempts=1, duration=1424.1s, phase=response_error,
served_model=deepseek-ai/deepseek-v4-flash-0731
None of residuals (i)-(iii) accounts for it:
- Not capacity (i): a route was ready and `deepseek-v4-flash-0731` served, so
preflight succeeded rather than exhausting at zero ready.
- Not the raw-500 class (ii): the caller received the classified 502 that
`contextual-orchestrator#1082` is adding, not an opaque `internal_error`.
- Not the 90 s recv default (iii): one caller attempt ran 1424.1 s, about 23.7
minutes, roughly sixteen times that limit, and `phase=response_error` says a
response arrived carrying an error status rather than a socket expiring — a
different event from the `TimeoutError` in `#1053`'s own 90.054 s noema
measurement.
The reading this supports is that `caller attempts=1` bounds the caller only;
the gateway owns repair and failover (its own warning line says so) and spent
those 23.7 minutes walking the pool internally before classifying.
Honest limit stated in the entry: this is the job log, not the
`noema-sidecar-evidence` artifact (9992218398) that would give the internal
attempt count and the distribution of the 23.7 minutes. No per-attempt
breakdown is claimed. What the log establishes alone is that a served route
plus a classified 502 plus a 23.7-minute wall clock is a real, current
combination, so closing (i) and (ii) will not by itself account for it.
Verification: full suite 2956 passed, 1 skipped, 21 subtests passed;
`git diff --check` clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
… the evidence artifacts I downloaded the `noema-sidecar-evidence` artifacts (9992218398, 9992230612) that the previous two commits explicitly declined to read, and they refute two of the three claims the entry rested on. Both retractions are recorded in the entry rather than edited away. Retraction 1 — nothing was served. `served_model` names the last route *attempted*, not one that answered. Both artifacts end with `provider_attempt_failed agent_id=nvidia_nim_deepseek_ai_deepseek_v4_flash_0731 … error_type=TimeoutError`, then `circuit_failure … failures=1.0 threshold=3`, then `request_failed status=502 code=provider_connection_error`. I read a field name as an outcome. Retraction 2 — the 90 s default IS operative here, so this is not evidence against `contextual-orchestrator#1053`. `caller attempts=1` bounds the caller; the gateway ran 24 matched internal attempts summing to ~11,500 s against a 1,424 s wall clock (8–9× concurrency, so they race), and four of the 24 sit at 89.5–92 s in both runs. I told the `#1053` lane their 90.054 s sample was contradicted; it is not, and that is corrected on `#1053` directly. What survives is the part worth acting on: the durations are bimodal. Besides those four ~90 s attempts and six failing under 10 s, 11 of 24 attempts on `#1884` and 12 of 24 on `#1187` ran past 600 s, to 1,333.7 s and 1,122.9 s, medians 478.3 s and 631.3 s. No 90 s bound explains that second population, and the consequence runs opposite to what I implied earlier: removing the implicit timeout converts the ~90 s population into unbounded waits and leaves the long one untouched, so on this evidence it should make these runs longer. Stated as a two-sample prediction, not a proven regression. Two readings that support the other lanes rather than contradict them: the breaker is told on this path (`circuit_failure` 13 and 11, `circuit_opened` twice each), matching `#1082`'s scoping of its defect to the passthrough walk; and its re-selection concentration is visible here, with `deepseek-v4-flash-0731` taking 23 of ~40 attempts across both NVIDIA keys despite the breaker opening twice. Preflight read `ready_count 6, rejected 8, deferred 2, skipped 4` in both runs, so capacity is ruled out by artifact rather than by inference. Verification: full suite passed; `git diff --check` clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
2 participants
Next action
Merge this successor instead of #1040. That head still fail-closes a pretty-printed password dump when
collect_failed_check_evidence.shfeedsgh run view --log-failedlines (job<HTAB>step<HTAB>plus the zip RFC 3339 timestamp).What changed
job<HTAB>step<HTAB>only when an RFC 3339 timestamp follows the second tab, then skipZortime-numoffsetplus SPACE or HTAB the same way JSON whitespace is skipped.UNKNOWN STEPand offset/HTAB zip lines as one span so group and status text stay visible.run:step text cannot survive on continuation lines.gh run viewnext to the existing APA 7 RFC 3339 / RFC 8259 / workflow-log references.Verification
Focused redaction selection: 177 passed. Owned modules
redact_sensitive_log.py,sandboxed_verify.py, andsandboxed_web_e2e.pyreport 100% statement and branch coverage (1,136 statements, 464 branches). Complete repository suite: 1232 passed plus 16 subtests before the two added collector fixtures; focused selection includes those fixtures.Acceptance
Do not merge #929, #1031, #1038, or #1040. After this head is green, treat
[REDACTED]as evidence suppression and rerun the exact-head sandbox redaction quality job before closing #908.