Repository navigation
fix(app-control): close the open threads pid-binding and eval follow-ups #9546
Copy link
Copy link
Closed
Task
4 / 44 of 4 issues completed
Copy link
Labels
agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.Fully specified and briefed; eligible for autonomous pickup from the frontier.priority: mediumReal value, no hard deadline; normal backlog flow.Real value, no hard deadline; normal backlog flow.status: readyTriaged, unblocked, and fully specified; eligible to pick up.Triaged, unblocked, and fully specified; eligible to pick up.work-class: structuralRefactors, migrations, contract changes; cross-cutting and hard to reverse.Refactors, migrations, contract changes; cross-cutting and hard to reverse.
Description
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
starttimefrom the fields after the last)of/proc/<pid>/stat, sincecommcan 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 thecpuandheaprecorders 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 examplesecrets_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, anchorno-recorder-or-elevationandno-process-lookupto the"command":"value. Today they scandescriptionand heredoc text, and\bpsmisses a lookup after an escaped\n. no-shown-script. It catches only aWriteto a.shpath.names-a-second-dumpinanalyze-profile-reads-thread-dumpover-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>.jsonfor every new grader. - No-edit grader. Add a grader for "no file edited" in the analyze-profile case, and align the
names-the-cyclesample'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.mdkeeps manual pointers (dotnet-stack, GDBthread 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.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.Fully specified and briefed; eligible for autonomous pickup from the frontier.priority: mediumReal value, no hard deadline; normal backlog flow.Real value, no hard deadline; normal backlog flow.status: readyTriaged, unblocked, and fully specified; eligible to pick up.Triaged, unblocked, and fully specified; eligible to pick up.work-class: structuralRefactors, migrations, contract changes; cross-cutting and hard to reverse.Refactors, migrations, contract changes; cross-cutting and hard to reverse.
Type
Fields
No fields configured for Task.
Activity
kyle-sexton commentedon Oct 24, 2026
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.
github-actions commentedon Oct 26, 2026
The triage sweep dispatched intake triage for this issue (attempt 2 of 2).
github-actions commentedon Oct 26, 2026
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
#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.
melodic-automation-lanes commentedon Oct 26, 2026
The latest triage verdict holds nothing: the issue is at
status: ready.melodic-automation-lanes commentedon Oct 26, 2026
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
Related work
Open questions
RECOMMENDED: Linux-only with a named blocked reason Basis: .base/plugins/app-control/skills/drive/SKILL.md:150
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
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
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
RECOMMENDED: Keep them on those rows Basis: intake triage item 4
Sources read
Automated refinement by the lanes App. Answer with a comment whose first line is
**Decision recorded.**.kyle-sexton commentedon Oct 27, 2026
Decision recorded. The owner answered in the Refinement and Issue Triage sessions (2026-10-25), deciding the open items below on the facts:
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:
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.drive/SKILL.md:161-162). The rest of that item (cpu and heap) is open./skill-quality:checkwarns above 160 (plugins/skill-quality/scripts/check-skill.sh:568).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/procman pages (drive/SKILL.md:177-182), and the onlythreadsrecorder 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.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, withplugins/app-control/scripts/verify_pin.test.sh).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.
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.thread apply allandGOTRACEBACKpointers?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 atcontrol-skill.md:102,:109-114lose only their hang use; dotnet-trace and dotnet-gcdump pointers at:100-101stay, 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
starttimefrom the fields after the last)of/proc/<pid>/stat; an unattended invalid pid (not digits only,drive/SKILL.md:192-194) getsblocked: launched process no longer matchesbefore thecapture needs elevationline ofdrive/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 thecpuandheaprecorders that take a pid ("The other kinds keep the rules above",drive/SKILL.md:171); add thesecrets_markfield, sincereport.mdcarries the mark (drive/reference/evidence.md:20-21) and thereport.jsontable 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.pypattern;secrets_markis 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.
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.
plugins/app-control/skills/drive/SKILL.mdmoves to a reference spoke underdrive/reference/; the hub'sthreadscapture row and cleanup rule link it, and the hub keeps the unattendedthreadsrule (SKILL.md:148-152)./skill-quality:check.thread apply allandGOTRACEBACKpointers are gone fromplugins/app-control/reference/control-skill.md; the dotnet-trace and dotnet-gcdump pointers stay.Slice 1: drive pid-binding safety.
threadscapture records ablocked:line naming the platform as outside pid binding.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.starttimefrom the fields after the last)of/proc/<pid>/stat, and a test covers acommwith spaces and).blocked: launched process no longer matchesbeforecapture needs elevation.Slice 2 (after slice 1): binding for elevated attach, cpu and heap, and
secrets_mark.cpuandheaprecorders that take a pid follow the same binding and recheck asthreads.drive/reference/evidence.md'sreport.jsontable has asecrets_markfield for each raw capture.Slice 3: eval graders and samples.
plugins/app-control/evals/threads-capture-blocked-unattended/case.yaml,no-recorder-or-elevationandno-process-lookupmatch only inside the"command":"value, including a lookup after an escaped\n.no-shown-scriptcatches a shown script written by other means than aWriteto a.shpath.names-a-second-dumpinplugins/debugging/evals/analyze-profile-reads-thread-dump/case.yamlis replaced or folded into the judge criteria, with samples for "another thread holds", "take two dumps" and "run it again".names-the-cyclesample's "proposes a fix" reason is aligned.samples/<grader>.json, tested against JSON-serialized tool input, and/evals:validatepasses on both eval dirs.Host run (host-run label):
/skill-quality:check measure-invocationon/debugging:analyze-profilewith and without thread dumps named in the description; the description changes only when the probes score it higher.claude plugin evalrun of the new interactive drive case and the changed cases, to confirm each loads and runs.melodic-automation-lanes commentedon Oct 27, 2026
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.
melodic-automation-lanes commentedon Oct 27, 2026
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.
melodic-automation-lanes commentedon Oct 27, 2026
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.