Repository navigation
fix(pty): preserve ioctl pointer lifetime and report spawn limits (nocx-zewbi) - #546
Merged
Merged
Conversation
…ocx-zewbi) The old creack/pty ioctl wrapper can lose its output index when GC moves its stack, pairing a master with /dev/pts/0. Equal stress samples measured 18 wrong pairs on v1.1.24 and none on the merged upstream fix. That defect can explain a controlling-terminal EPERM, but the historical failing syscall remains unproved. Temporarily pin merged upstream PR #215 until a release includes it. Its SyscallConn ioctl path also preserves nonblocking, so check every direct consumer rather than assuming this is only a pointer change. Four pass; the two remaining pod suites reproduce their completed failures on the old graph, with no new-only failure beyond the intended old regression. Keep a Linux pair-identity stress regression and add failure-only parent resource diagnostics. Resolve cgroup membership across all visible mounts without guessing namespace mappings, preserve wrapped errno, and make unavailable readings explicit without inspecting the child environment.
.githooks/pre-push refused the branch with "nocx-zewbi - task does not exist": commit-links.mjs resolves a commit's ids against the tracker export AT THE PUSHED TIP, and the bead exists only in the database. Its one row is published here, taken from br's own export. Nothing else in the file is touched, because the main checkout's export also carries other agents' unpublished work. Co-Authored-By: prime-agent <noreply@shady2k.dev>
…(nocx-zewbi) main moved while this branch was in review — nocx-ukf54's rcfile fix landed — and GitHub runs no pull_request workflow on a conflicted pull request, so the branch had no CI at all. The only conflict was the tracker export: main's rows are kept whole and this branch's own row, nocx-zewbi, is taken from br's export with its current state, so both sides survive and nothing else in the file changes. .githooks/check-tracker-import.sh HEAD exits 0 on the result. Co-Authored-By: prime-agent <noreply@shady2k.dev>
shady2k
added a commit
that referenced
this pull request
Oct 27, 2026
Five leaves reached their merge and this branch's export still carried them open — nocx-owbpb (PR #544), nocx-zewbi (PR #546), nocx-ukf54 (PR #545), nocx-hyh2p (PR #542) and this branch's own four. The pushed tip's export is what pre-push and the backlog gate read, so the closures travel with the branch rather than waiting for a later flush. Co-Authored-By: prime-agent <noreply@shady2k.dev>
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
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.
Cause and evidence
creack/pty v1.1.24converts an ioctl output pointer touintptrbeforecalling wrappers that can shrink the goroutine stack.
TIOCGPTNcan then writethe old stack address while the live index remains zero.
Openpairs the newmaster with
/dev/pts/0, not its own slave. See upstreamPR #215 and
issue/PR #216.
The original failure is run 40409804818 attempt 1, job 121877470440 (the
latest attempt was cancelled):
The old-pin stress reproduced the lost index on the default SkyPilot pod:
Old stress: 10.03 s total test time, 48.761 s client wall time (including
stage/build). New stress: 12.85 s total test time, 51.636 s client wall time.
The zero-event sample gives a nominal rate below about 1.524 × 10⁻⁵ at
95% confidence, not proof of zero risk; GC events are not independent.
The source change removes the invalid pointer lifetime rather than making
a retry more likely to succeed.
Limits of the evidence: pair corruption was reproduced; exact
EPERMwasnot. Starting on the wrong unowned slave succeeded in the first probe.
TIOCSCTTYcan returnEPERMwhen that slave belongs to another livesession. This is consistent with the CI signature, not proof of its cause. The historical CI log
has no syscall-stage or resource readings, so it cannot independently prove
which child-setup syscall failed. Pod ptrace is denied; that tracing failure
is not the reported PTY failure. Pod Go is 1.26.5, CI Go was 1.26.9.
The two pod launcher contexts also agree on security state: both have
NoNewPrivs=0,Seccomp=2, one filter,Threads=1,RLIMIT_NPROC=79541,pids.max=max, andpids.events=max 0.pids.currentwas 401 without and418 with Secret Service. These are current pod measurements, not historical
CI measurements; the filter count alone does not prove identical filter bytes.
What changed
v1.1.25-0.20260601142114-9246436fffe8, verified Origin hash9246436fffe85773950f49dcc73de333da5212e6. There is no tagged releasecontaining the fix yet; drop the pin when a tag carries it.
go.modrecordsthe hash, PR chore(beads): the xterm cutover becomes ten leaves, and the frame wire gets an owner #215, issue The order of the xterm cutover is recorded, and run geometry gets its one owner #216, and removal condition.
safe
unsafe.Pointerlifetime until the syscall, and activation of theexisting
SyscallConn.Controlioctl path instead ofFile.Fd. The latterpreserves nonblocking rather than reverting it, agreeing with the raw-master
handling nocx already owns. The intermediate pointer-only commit was not
selected: the merged change is the reviewed state the eventual tag carries.
not whether one shell happened to start. Every master ioctl index must equal
its slave device minor. The 32768-open CI run is a cheap regression signal,
not a deterministic scheduler or proof of absence.
direct
/procsnapshot: PID, threads, NNP/seccomp, NPROC, and pids countersfor the resolved cgroup and visible ancestors. Missing readings say why.
No child environment or arguments are read. Other platforms explicitly state
that Linux resource diagnostics are unavailable.
Checks
All tests below ran through default
nocx-sky-test(2 CPU / 4 GB),using
go test -race -p 1 -tags gtk3,nocx_framecheck <package>forimporters. The OLD control adds only
-mod=mod -modfile=/tmp/nocx-old.mod; its adjacent sumfile is separate.The worktree's go.mod/go.sum were not edited by the control.
internal/pty/...internal/shellintegration/...internal/helper/session/...internal/app/...cmd/agent-capturecmd/e2e-sshdModule selection was checked on the same pod against the retained control:
Both version queries exited 0. Both
internal/appruns hit the package-wide10-minute deadline. The test in flight differed between the pins because the
elapsed total differed, so this is not a claim that a specific test times out
on both;
internal/appdoes not finish at this pod's pace, and its coherenceis CI's to establish. NEW was in
TestAReportMovesNoRecordStateAndTheIdleStillFollows(16 s); OLD inTestAWorkerCannotNameARecipientOverTheSocket(2 s).For every test that actually completed and failed, the assertion, source
line and message match between the pins, apart from random paths, ports and
fixture mtimes. No test fails only on the new pin; the only old-only failure
is this PR's pair-identity regression. The shared failures are not caused by
this pin. This is not a claim that the two full suites passed or completed.
Environment findings, present on both module graphs:
the required provisioning command.
root prompt, not the user's fixture rc. This is the same
/etc/profilerewrite measured in
nocx-ukf54; read-only-home assertions also cannotenforce ordinary-user permissions under root.
LAUNCH_ALLARGVcontains SkyPilot's ownbash -c -- # Set -x ...provisioning command, not an argv selected by this change.
connection names missing
/root/.sshidentity files on both pins.No environment workaround, retry, skip, or wider timeout was added. The two
full importers remain unvalidated in this pod login environment; macOS CI
and the real CI Linux environment remain required.
The complete failing-test/subtest comparison follows. Each shared row
shows the common first assertion (dynamic ports/paths/timestamps omitted).
TestBashLauncher_BaselineKeepsVisiblePromptTestBashLauncher_EmitsMarkersAndRunsUserRcTestBashLauncher_NoHomeWritesTestBashLauncher_RunsUnderDashTestBashLauncher_TransientRcfileIsGoneBeforeUserCodeTestBashSnapshotArrivesBeforeFirstPromptTestCapture_SaveNowAndSaveLaterOverTheRealSocketTestEnsureInstalledRemote_ReadonlyHomeFailsOpenThenConvergesTestExecRefusalProbe_AnAcceptedExecConsumesTheChannelAndForcedCommandsLeaveNoNativeShellTestExecRefusalProbe_OpenSSHAcceptsExecUnderEveryRestrictionWeCanConfigureTestExecRefusalProbe_OpenSSHAcceptsExecUnderEveryRestrictionWeCanConfigure/ForceCommand_in_the_server_configTestExecRefusalProbe_OpenSSHAcceptsExecUnderEveryRestrictionWeCanConfigure/Match_block_for_this_user_with_ForceCommandTestExecRefusalProbe_OpenSSHAcceptsExecUnderEveryRestrictionWeCanConfigure/command=_restriction_on_the_authorized_keyTestExecRefusalProbe_OpenSSHAcceptsExecUnderEveryRestrictionWeCanConfigure/transfer-only_account_(ForceCommand_internal-sftp)TestExecRefusalProbe_OpenSSHAcceptsExecUnderEveryRestrictionWeCanConfigure/unrestricted_accountTestExecRefusalProbe_RefusedSessionRequestLeavesTheChannelAndItsPTYUsableTestInjectedBashStartupPreservesUserExitTrapTestInjectedStartupReservesLegacyLatchTestInjectedStartupReservesLegacyLatch/bashTestLaunchCarrier_BadManifest_FailsOpenToNativeLoginShellTestLaunchCarrier_BadManifest_FailsOpenToNativeLoginShell/hash-mismatchedTestLaunchCarrier_BadManifest_FailsOpenToNativeLoginShell/missing-manifestTestLaunchCarrier_BadManifest_FailsOpenToNativeLoginShell/protocol-incompatibleTestLaunchCarrier_BadManifest_FailsOpenToNativeLoginShell/symlinked-generation-fileTestLaunchCarrier_BadManifest_FailsOpenToNativeLoginShell/truncated-manifestTestLaunchCarrier_GoodManifest_BashExecsIntegratedShellTestLaunchCarrier_UserRcExitStillWinsTestLiveSshd_SSHChildAssembly_ChildEstablishesOverComposedLineTestLiveSshd_SSHChildAssembly_ExitFreezesTheChildBlockAndCompletesTheParentTestLiveSshd_SSHChildAssembly_ForwardingRefusedParentStillActivatesTestLiveSshd_SSHChildAssembly_LateCandidateAfterParentActivationTestLocalZshPane_ThePluginStackIntegratesInEitherOrderTestLocalZshPane_ThePluginStackIntegratesInEitherOrder/plugins-after-nocxTestLocalZshPane_ThePluginStackIntegratesInEitherOrder/plugins-before-nocxTestPTYOpenKeepsItsSlaveUnderStackShrinkTestReadonlyHomeFailsCleanlyTestStage1_FilesystemFailureOrderingTestStage1_FilesystemFailureOrdering/the_unlink_fails,_so_nothing_is_written_at_allTestStage1_FilesystemFailureOrdering/the_write_descriptor_cannot_be_openedTestTier_BashReadsTheCapabilityFromTheDescriptorOnceAndClosesItTestTypedMux_ARefusedSessionOpensNoConnectionAndCostsNoSecondAuthenticationTestTypedMux_NothingIsPublishedBeforeOwnershipIsProvenTestTypedMux_ThePublishRidesTheUsersOwnConnectionDiagnostic TDD:
NewLocalmissing-executable seam: RED exit 1, then GREENexit 0 / 1.038 s. It asserts wrapped ENOENT survives and the diagnostic
contains the resource fields, not the environment canary.
Fixtures for conservative containment, all mounts/visible ancestors,
unreadable-first fallback, and exact proc-field matching: RED exit 1 /
0.042 s, then GREEN exit 0 / 1.046 s, with the actual spawn seam.
An independent source re-review accepted the repair.
internal/pty/...: exit 0 / 1.255 s on the final sources.Final source checks after lint-only changes: pair-identity stress exit 0 /
4.98 s (6.040 s package); diagnostic fixtures plus actual spawn seam
exit 0 / 1.042 s. The enabled commit hook initially caught a narrowing
conversion and required a fixed-file-read annotation; both were corrected.
Enabled pre-commit and commit-message hooks passed on the final commit:
formatting, golangci-lint, control-goroutine/log-context ratchets, root
Prettier/ESLint, and commit-link checks. The backlog export was not staged,
so the staged backlog check was correctly skipped. Present-document check
also exited 0 with no new error. No hook was bypassed.
All tests ran through
nocx-sky-testat its defaults. No retry, skip or widertimeout was added. The earlier 4-CPU/12-GB provisioning attempts did not run
tests; the owner's corrected default-size instruction superseded them.
macOS runtime behavior is CI's to establish, not claimed from this Linux pod.
Check in three steps
go test -tags gtk3,nocx_framecheck -count=6 -run TestPTYOpenKeepsItsSlaveUnderStackShrink -v ./internal/shellintegrationthrough the project's test runner. The old version failed 18/196608; the
pinned version was zero over the same sample.
TestNewLocal_SpawnFailureReportsLinuxResourcesin
internal/pty. Check that ENOENT still unwraps and the snapshot has theresource fields without the environment canary. Then inspect the importer
results below and the PR's macOS CI result.
Task: nocx-zewbi. No merge requested or performed.