Skip to content

fix(app-control): close the open threads pid-binding and eval follow-ups #9546

Closed
4 of 4 issues completed
Closed

Description

@kyle-sexton

Refs: #9532

Problem

PR #9542 added the threads capture kind to /app-control:drive and a thread-dump reading page to /debugging:analyze-profile. Its review rounds found the items below. They are outside the two pid-binding safety fixes that PR completed, so they were left open there.

Fix

Drive pid binding

  • Race after the recheck. A target can exec a setuid binary with the same argv between the script's recheck and the recorder: pid, start time and cmdline hash survive exec. Recheck again after the recorder exits, and drop the capture on a difference.
  • Platform. The integer-only uid and start-time rule leaves only Linux defined. On macOS and Windows a threads capture is blocked with no named reason. Either state that the binding is Linux-only with a named block, or give each platform's integer form.
  • Under runas. "The script's own uid, which is root" does not hold there.
  • Start-time parsing. Read starttime from the fields after the last ) of /proc/<pid>/stat, since comm can hold spaces or ).
  • Order of checks. State which blocked line an invalid pid gets when unattended.
  • Elevated attach. It has no binding: capture the attached pid's uid, start time and hash when the script is shown, and recheck them in the script.
  • Other pid-targeting kinds. Pid binding covers only threads. Extend it to the cpu and heap recorders that take a pid, and say whether cleanup's stop rechecks first.
  • Secrets mark in report.json. The table has no field for it (for example secrets_mark).
  • Body size. The drive body is 175 lines, above the measured max of 160 (/skill-quality:check). Consider moving Pid binding to a reference spoke.

Evals

  • Anchor the drive graders. In threads-capture-blocked-unattended, anchor no-recorder-or-elevation and no-process-lookup to the "command":" value. Today they scan description and heredoc text, and \bps misses a lookup after an escaped \n.
  • no-shown-script. It catches only a Write to a .sh path.
  • names-a-second-dump in analyze-profile-reads-thread-dump over-matches ("another thread holds…") and under-matches ("take two dumps", "run it again"). Replace it or fold it into the judge criteria.
  • Grader samples. Add samples/<grader>.json for every new grader.
  • No-edit grader. Add a grader for "no file edited" in the analyze-profile case, and align the names-the-cycle sample's "proposes a fix" reason.
  • Interactive coverage. No case covers exit 3, a root-owned wrapper child, or the hash recheck. Add an interactive case.

Other

  • control-skill.md keeps manual pointers (dotnet-stack, GDB thread apply all, GOTRACEBACK) that the hang row no longer names. Keep them for the JVM, .NET, Go and native rows, or remove them.
  • analyze-profile's description does not name thread dumps (slice plan Q16, option a). Decide it with invocation probes.

Acceptance criteria

  • Each item above is fixed, or closed with its reason in this issue.
  • Each changed eval grader is tested against JSON-serialized tool input and has samples.
  • grep -nE '#[0-9]{3,}' over the changed skill files returns nothing.

Activity

  1. added
    priority: mediumReal value, no hard deadline; normal backlog flow.
    status: readyTriaged, unblocked, and fully specified; eligible to pick up.
    on Oct 24, 2026
  2. added theissue type on Oct 24, 2026
  3. kyle-sexton commented on Oct 24, 2026

    @kyle-sexton
    MemberAuthor

    Added from the #9542 review: Bundle the recheck. Codex P1 on drive/SKILL.md suggests shipping the deterministic pid recheck as a bundled script instead of having the model generate it for each capture, so the recheck is tested code rather than prose followed per run. Weigh it against the shown-script route, where the person reads the exact commands before the yes.

  4. github-actions commented on Oct 26, 2026

    @github-actions
    Contributor

    The triage sweep dispatched intake triage for this issue (attempt 2 of 2).

  5. github-actions commented on Oct 26, 2026

    @github-actions
    Contributor

    Proposed work class: work-class: structural
    Basis: automated intake triage; the lanes App applied this class.

    Size: Large
    Complexity: Medium
    Uncertainty: Known
    Flags: none
    Blocked by: #10399, #10400, #10401, #10402

    Decision recorded. The owner answered in the Refinement and Issue Triage sessions (2026-10-25), deciding the open items below on the facts:

    go with whatever the facts/data/metrics + /discovery:research says and ensure we're aligned with SOLID, DRY/SSOT principles

    [quote truncated; the full decision is at the link]

    #9546 (comment)

    The decision settles the open questions: Linux-only pid binding with a named block, a bundled script that builds the shown recheck script, a four-slice split, and removal of the stale control-skill pointers. Nothing is left to ask. This is the parent of the already-filed sub-issues #10399, #10400, #10401 and #10402, so it is not a duplicate. The host-run probes and the eval run stay with a person on a host.

    Automated triage, applied by the lanes App.

  6. added
    status: needs-decisionAwaiting a human or maintainer judgment call.
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 26, 2026
  7. melodic-automation-lanes commented on Oct 26, 2026

    @melodic-automation-lanes
    Contributor

    The latest triage verdict holds nothing: the issue is at status: ready.

  8. added
    work-class: structuralRefactors, migrations, contract changes; cross-cutting and hard to reverse.
    and removed
    status: readyTriaged, unblocked, and fully specified; eligible to pick up.
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 26, 2026
  9. melodic-automation-lanes commented on Oct 26, 2026

    @melodic-automation-lanes
    Contributor

    Refinement. A read-only run explored this repository, searched its issues and pull requests, and read the allowed docs to draft what follows. On an issue a listed human filed, it counts as the issue's spec once posted, while unedited; it is still model text, so check it.

    Intended outcome: Split the bundle into separate changes: pid-binding fixes in drive, eval grader fixes, and the two small doc decisions, each with its own checkable criteria.

    Dependencies

    • #​9542 merged (done); #​9490 is the open parent feature for the threads capture kind

    Related work

    Open questions

    1. Should pid binding be Linux-only with a named block on macOS and Windows, or define an integer form per platform?
      RECOMMENDED: Linux-only with a named blocked reason Basis: .base/plugins/app-control/skills/drive/SKILL.md:150
    2. Should the recheck be a bundled, tested script, or stay a shown script the person reads before approving?
      RECOMMENDED: Bundled script, with the shown script displaying its exact invocation so the commands stay visible Basis: comment by kyle-sexton on Codex P1, and intake triage item 2
    3. Should this one issue be split, since it bundles about 20 items across drive pid binding, evals and docs?
      RECOMMENDED: Yes: (1) drive pid-binding safety fixes, (2) extend binding to cpu/heap and elevated attach plus the secrets_mark field, (3) eval graders and samples, (4) body-size spoke move and control-skill.md pointers Basis: judgment: size Large per intake triage, and the acceptance criterion 'each item fixed or closed' is not checkable as one unit
    4. Should analyze-profile's description name thread dumps?
      RECOMMENDED: Decide only after invocation probes; leave out of this change until the owner runs them Basis: judgment: the issue itself defers it to probes, which an unattended run cannot judge
    5. Should the control-skill.md manual pointers (dotnet-stack, GDB thread apply all, GOTRACEBACK) be kept for the JVM, .NET, Go and native rows or removed?
      RECOMMENDED: Keep them on those rows Basis: intake triage item 4

    Sources read

    • /home/runner/work/_temp/refine-data/context.json: issue 9546, intake triage, one maintainer comment, candidates
    • plugins/app-control/skills/drive/SKILL.md:35: the capture row says threads binds to the launched pid via Pid binding
    • plugins/app-control/skills/drive/SKILL.md:134: unattended threads with no launch record gives a blocked line
    • plugins/app-control/skills/drive/SKILL.md:150: the Pid binding section starts here

    Automated refinement by the lanes App. Answer with a comment whose first line is **Decision recorded.**.

  10. kyle-sexton commented on Oct 27, 2026

    @kyle-sexton
    MemberAuthor

    Decision recorded. The owner answered in the Refinement and Issue Triage sessions (2026-10-25), deciding the open items below on the facts:

    go with whatever the facts/data/metrics + /discovery:research says and ensure we're aligned with SOLID, DRY/SSOT principles

    I should only ever be in the loop ONLY when necessary (intent/vision changed, something came up during implementation that impacts trajectory/alignment, OR a high security risk) - I've covered this all before

    No owner ledger (.work/*/interview-checklist.md, .work/gh-monitor-owner-decisions/) names this issue. The intent flag names no change of intent, no implementation finding and no security risk, so nothing here goes to the owner. This answers the refinement's five questions (#9546 (comment)) and the "Bundle the recheck" item added at #9546 (comment).

    Facts on origin/main first. Two body items are already done:

    • Under runas: done. The shown script compares "Each live uid field ... with the recorded uid literal, never with the script's own uid" (plugins/app-control/skills/drive/SKILL.md:206); the "which is root" text is gone.
    • Cleanup's stop: done. Cleanup stops each launched process "after [Recheck before touching the pid]" (drive/SKILL.md:161-162). The rest of that item (cpu and heap) is open.
    • The drive body is now 258 lines (267 with frontmatter), not 175; /skill-quality:check warns above 160 (plugins/skill-quality/scripts/check-skill.sh:568).
    1. Platform: Linux-only with a named block, or an integer form per platform?
      Decided: Linux-only, with a blocked: line that names the platform on macOS and Windows. Basis: the start time and hash are defined only from the Linux /proc man pages (drive/SKILL.md:177-182), and the only threads recorder is py-spy dump (plugins/app-control/reference/control-skill.md:63). Defining other platforms' forms is new scope the body offers as the larger option.
    2. Recheck delivery: bundled tested script, or the shown script?
      Decided: both, in this form. A bundled script under plugins/app-control/scripts/ builds the shown script from the checked integers and hash; drive shows its full output and runs it only after the yes. This overturns the lead's "shown by invocation": a shown script that only calls a bundled file would run unshown code with elevation, and the attach rule says "show its full contents" (drive/SKILL.md:240). The pattern exists: drive already runs a bundled, tested script, verify_pin.py (drive/SKILL.md:93, with plugins/app-control/scripts/verify_pin.test.sh).
    3. Split?
      Decided: yes, four slices, listed below with their criteria. Basis: about 20 items across three files, and the body's "each item is fixed, or closed with its reason" is not one checkable unit. Slices 1 and 2 edit the text that slice 4 moves, so slice 4 lands first.
    4. analyze-profile description names thread dumps?
      Decided: only on probe evidence. The description today names CPU profiles, heap snapshots and traces, not thread dumps (plugins/debugging/skills/analyze-profile/SKILL.md:2). The existing control is /skill-quality:check measure-invocation (plugins/skill-quality/skills/check/SKILL.md:21-22); it is a host run, listed below.
    5. Keep or remove the dotnet-stack, GDB thread apply all and GOTRACEBACK pointers?
      Decided: remove them. This overturns the lead's "keep". The hang row says "not yet" for JVM, .NET, Go and native (control-skill.md:63), so no row names those tools, and the manuals list exists for tools a row names (control-skill.md:55-56, :82). The pointers at control-skill.md:102, :109-114 lose only their hang use; dotnet-trace and dotnet-gcdump pointers at :100-101 stay, since the cpu and heap rows name them. A pointer comes back when a hang cell names its tool.

    Other body items, decided as the body states them: recheck again after the recorder exits and drop the capture on a difference; read starttime from the fields after the last ) of /proc/<pid>/stat; an unattended invalid pid (not digits only, drive/SKILL.md:192-194) gets blocked: launched process no longer matches before the capture needs elevation line of drive/SKILL.md:148-152, since the format check reads no process (judgment); bind and recheck elevated attach, which today has "no recheck" (drive/SKILL.md:169-171); extend binding to the cpu and heap recorders that take a pid ("The other kinds keep the rules above", drive/SKILL.md:171); add the secrets_mark field, since report.md carries the mark (drive/reference/evidence.md:20-21) and the report.json table has no field for it (evidence.md:37-48).

    No added mechanism beyond what the body names: the bundled script is the body comment's own ask and reuses the verify_pin.py pattern; secrets_mark is the body's field; the interactive eval case is the body's ask.

    Acceptance criteria: this issue closes when its four slice sub-issues close.

    • Four sub-issues exist under this issue, as below, and each closes by a merged PR.
    • grep -nE '#[0-9]{3,}' over each slice's changed skill files returns nothing (body criterion, every slice).

    Slice 4 (lands first): body-size spoke and control-skill pointers.

    • The Pid binding section of plugins/app-control/skills/drive/SKILL.md moves to a reference spoke under drive/reference/; the hub's threads capture row and cleanup rule link it, and the hub keeps the unattended threads rule (SKILL.md:148-152).
    • The drive hub body is shorter than on origin/main at the slice's base, per /skill-quality:check.
    • The dotnet-stack, GDB thread apply all and GOTRACEBACK pointers are gone from plugins/app-control/reference/control-skill.md; the dotnet-trace and dotnet-gcdump pointers stay.

    Slice 1: drive pid-binding safety.

    • On macOS and Windows a threads capture records a blocked: line naming the platform as outside pid binding.
    • A bundled script under plugins/app-control/scripts/, with a test beside it, builds the shown script from the checked pid, uid, start time and hash; drive shows its full output before the yes.
    • The built script reads starttime from the fields after the last ) of /proc/<pid>/stat, and a test covers a comm with spaces and ).
    • The built script rechecks after the recorder exits and drops the capture on a difference; a test covers it.
    • The skill states that an unattended invalid pid gets blocked: launched process no longer matches before capture needs elevation.

    Slice 2 (after slice 1): binding for elevated attach, cpu and heap, and secrets_mark.

    • An elevated attach records the attached pid's uid, start time and hash when the script is shown, and the script rechecks them.
    • The cpu and heap recorders that take a pid follow the same binding and recheck as threads.
    • drive/reference/evidence.md's report.json table has a secrets_mark field for each raw capture.

    Slice 3: eval graders and samples.

    • In plugins/app-control/evals/threads-capture-blocked-unattended/case.yaml, no-recorder-or-elevation and no-process-lookup match only inside the "command":" value, including a lookup after an escaped \n.
    • no-shown-script catches a shown script written by other means than a Write to a .sh path.
    • names-a-second-dump in plugins/debugging/evals/analyze-profile-reads-thread-dump/case.yaml is replaced or folded into the judge criteria, with samples for "another thread holds", "take two dumps" and "run it again".
    • The analyze-profile case has a no-file-edited grader, and the names-the-cycle sample's "proposes a fix" reason is aligned.
    • An interactive drive case covers exit 3, a root-owned wrapper child and the hash recheck.
    • Every new or changed grader has samples/<grader>.json, tested against JSON-serialized tool input, and /evals:validate passes on both eval dirs.

    Host run (host-run label):

    • /skill-quality:check measure-invocation on /debugging:analyze-profile with and without thread dumps named in the description; the description changes only when the probes score it higher.
    • One claude plugin eval run of the new interactive drive case and the changed cases, to confirm each loads and runs.
  11. melodic-automation-lanes commented on Oct 27, 2026

    @melodic-automation-lanes
    Contributor

    Not admitted to the issue lane yet: blocker #10399 is not closed by a listed human or a merge; blocker #10400 is not closed by a listed human or a merge; blocker #10401 is not closed by a listed human or a merge; blocker #10402 is not closed by a listed human or a merge; sub-issue #10399 is open; sub-issue #10400 is open; sub-issue #10401 is open; sub-issue #10402 is open.

  12. melodic-automation-lanes commented on Oct 27, 2026

    @melodic-automation-lanes
    Contributor

    Split at intake (owner Q93). This issue is Size Large, so its done-when is split into 4 sub-issues, each holding its task-list lines as written.

    Automated split by the lanes App. Admission waits on this issue while a sub-issue is open; work-unblock closes it as completed once every sub-issue is closed by a merge or a listed human.

  13. melodic-automation-lanes commented on Oct 27, 2026

    @melodic-automation-lanes
    Contributor

    Every sub-issue of this split is closed by a listed human or a merge (#10399, #10400, #10401, #10402), so the lanes App closes it as completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.priority: mediumReal value, no hard deadline; normal backlog flow.status: readyTriaged, unblocked, and fully specified; eligible to pick up.work-class: structuralRefactors, migrations, contract changes; cross-cutting and hard to reverse.

    Type

    Fields

    No fields configured for Task.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions