Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions docs/WINDOWS_OBSERVATION_DIAGNOSTICS.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Windows observation-cache diagnostics

Observation claims keep the 250ms production budget and fail closed. A
Observation claims fail closed. The 250ms budget covers the locked read/publication
transaction after root and private temporary-file preflight; the original caller
context covers both phases. Complete claim duration can exceed 250ms. A
`cache_unavailable` result does not establish whether the cause was a deadline,
private-file validation or a Windows I/O error. It also does not prove that an
attempted bit was not published. Never retry an uncertain claim automatically.
Expand All @@ -9,12 +11,12 @@ The public error message and notification receipts remain `cache_unavailable`.
Internally, declare `var failure *observation.ClaimFailure` and call
`errors.As(err, &failure)` to inspect bounded details:

- `Phase`: validation, path/root preparation, lock, clock, read/decode,
- `Phase`: validation, path/root preparation, temp preparation, lock, clock, read/decode,
record/encode, publication or the check after publication.
- `Class` and `OSCode`: sanitized failure category and numeric OS error.
Windows sharing violation (32) and lock violation (33) are distinct.
- `TotalElapsed`, `StageElapsed` and `BudgetElapsed`: elapsed durations.
Root/path preflight precedes the existing claim timer, so total duration can
Root/path and private temporary-file preflight precede the claim timer, so total duration can
exceed budget duration. `BudgetState` records the timer independently of the
operation's error; an I/O error and an expired deadline can coexist.
- `MayHavePublished`: conservative uncertainty once publication starts.
Expand Down Expand Up @@ -82,3 +84,21 @@ not cancel existing sessions, retry tests, change antivirus settings or install
tools. Unlike sanitized claim diagnostics, the raw OS trace includes paths and
process metadata from the disposable test runner. Do not run it on a real user
profile or infer a driver cause from a successful control recording.

## Preflight and transaction boundary

Each claim allocates one private empty temporary file before starting its 250ms
transaction context. The caller context is checked before and after allocation;
it is never refreshed. Preflight does not inspect or change attempt history.
Locking, current document/clock decisions, writing, target validation, replacing
rename and checked close remain in the transaction. The pre-rename and final
deadline checks still deny expired or uncertain publication.

Unused preparations are deleted and closed on duplicate, lock, read, decode or
cancellation exits. A duplicate now also requires a writable private cache root:
creation failure returns `cache_unavailable` before duplicate detection. This is
a conservative admission failure, never permission to deliver or retry.

This boundary change addresses the attributed temporary creation stalls without
claiming they are faster. Other synchronous I/O can still exhaust the transaction
or caller deadline. No first-valid assertion, fixed window or private ACL is relaxed.
4 changes: 3 additions & 1 deletion docs/gemini-notifications.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ Duplicate suppression records attempts per channel for a 60-second window, with
at most 256 hashed observation markers. It does not guarantee successful delivery
or exactly-once notification display. Missing native timestamps are scoped to one
invocation. An unsafe, unavailable or slow cache suppresses the affected attempt;
the cache claim budget is 250 ms and there is no automatic delivery retry. A crash
the locked cache transaction budget is 250 ms after root and private temporary-file
preflight. The caller deadline also applies to preflight, so complete claim duration
can exceed 250 ms. There is no automatic delivery retry. A crash
or cache loss can permit duplicates outside this bounded guarantee.

Restart Gemini after setup or update so its effective hooks are reloaded. Use the
Expand Down
119 changes: 78 additions & 41 deletions internal/installruntime/private_cache_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,82 +145,119 @@ func (r *PrivateCacheRoot) Write(name string, data []byte) error {
return r.WriteContext(context.Background(), name, data)
}

// WriteContext checks cancellation immediately before replacing the document.
// Synchronous filesystem calls and a rename already in flight cannot be canceled;
// callers must still check their deadline after publication before delivery.
func (r *PrivateCacheRoot) WriteContext(ctx context.Context, name string, data []byte) (err error) {
// WriteContext prepares and publishes under the supplied context. Claim callers
// may instead prepare before their transaction budget starts.
func (r *PrivateCacheRoot) WriteContext(ctx context.Context, name string, data []byte) error {
if len(data) == 0 {
return fmt.Errorf("private cache write requires bytes")
}
prepared, err := r.PrepareWrite(ctx, name)
if err != nil {
return err
}
return prepared.WriteContext(ctx, data)
}

// PrivateCacheWrite owns one empty private temporary file. It is single-use and
// must be closed if the caller decides not to publish. It never owns the root.
type PrivateCacheWrite struct {
root *PrivateCacheRoot
name string
file *os.File
ctx context.Context
published bool
}

// PrepareWrite allocates without reading or modifying attempt history. The
// caller context governs preflight; synchronous creation cannot be interrupted.
func (r *PrivateCacheRoot) PrepareWrite(ctx context.Context, name string) (*PrivateCacheWrite, error) {
if err := ctx.Err(); err != nil {
return nil, err
}
if name == "" || name == "." || name == ".." || strings.ContainsAny(name, `\/:`) {
return fmt.Errorf("invalid relative Windows component")
return nil, fmt.Errorf("invalid relative Windows component")
}
var parent windows.Handle
trace.WithRegion(ctx, "observation.publish/root", func() {
parent, err = r.parent()
})
var err error
trace.WithRegion(ctx, "observation.publish/root", func() { parent, err = r.parent() })
if err != nil {
return err
return nil, err
}
var random [16]byte
trace.WithRegion(ctx, "observation.publish/random", func() {
_, err = rand.Read(random[:])
})
trace.WithRegion(ctx, "observation.publish/random", func() { _, err = rand.Read(random[:]) })
if err != nil {
return err
return nil, err
}
temp := ".observations-" + hex.EncodeToString(random[:])
var handle windows.Handle
trace.WithRegion(ctx, "observation.publish/create", func() {
handle, err = windowsCreatePrivateCacheAt(parent, temp)
})
trace.WithRegion(ctx, "observation.publish/create", func() { handle, err = windowsCreatePrivateCacheAt(parent, temp) })
if err != nil {
return err
return nil, err
}
prepared := &PrivateCacheWrite{root: r, name: name, file: os.NewFile(uintptr(handle), temp), ctx: ctx}
if err := ctx.Err(); err != nil {
_ = prepared.Close()
return nil, err
}
f := os.NewFile(uintptr(handle), temp)
published := false
return prepared, nil
}

// Close removes an unused temp and releases its handle. Publication checks this
// close before granting admission; deferred claim cleanup is idempotent.
func (w *PrivateCacheWrite) Close() error {
if w == nil || w.file == nil {
return nil
}
f := w.file
w.file = nil
if !w.published {
trace.WithRegion(w.ctx, "observation.publish/cleanup", func() { _ = windowsDeleteHandle(windows.Handle(f.Fd())) })
}
var err error
trace.WithRegion(w.ctx, "observation.publish/close", func() { err = f.Close() })
return err
}

// WriteContext publishes once, revalidating root and target under the caller's
// lock. Its checked close and pre-rename deadline guard precede admission.
func (w *PrivateCacheWrite) WriteContext(ctx context.Context, data []byte) (err error) {
if w == nil || w.file == nil {
return fmt.Errorf("private cache preparation is closed")
}
w.ctx = ctx
defer func() {
if !published {
trace.WithRegion(ctx, "observation.publish/cleanup", func() {
_ = windowsDeleteHandle(handle)
})
}
var closeErr error
trace.WithRegion(ctx, "observation.publish/close", func() {
closeErr = f.Close()
})
if err == nil {
if closeErr := w.Close(); err == nil {
err = closeErr
}
}()
if len(data) == 0 {
return fmt.Errorf("private cache write requires bytes")
}
var parent windows.Handle
trace.WithRegion(ctx, "observation.publish/root", func() { parent, err = w.root.parent() })
if err != nil {
return err
}
var written int
trace.WithRegion(ctx, "observation.publish/write", func() {
written, err = f.Write(data)
})
trace.WithRegion(ctx, "observation.publish/write", func() { written, err = w.file.Write(data) })
if err != nil {
return err
}
if written != len(data) {
return io.ErrShortWrite
}
trace.WithRegion(ctx, "observation.publish/target", func() {
err = validatePrivateCacheReplacement(parent, name)
})
trace.WithRegion(ctx, "observation.publish/target", func() { err = validatePrivateCacheReplacement(parent, w.name) })
if err != nil {
return err
}
if err = ctx.Err(); err != nil {
return err
}
trace.WithRegion(ctx, "observation.publish/rename", func() {
err = windowsRenameHandle(handle, parent, name, true)
})
trace.WithRegion(ctx, "observation.publish/rename", func() { err = windowsRenameHandle(windows.Handle(w.file.Fd()), parent, w.name, true) })
if err != nil {
return err
}
published = true
// A close error still denies the claim, even if its attempted bit has already
// been published. The caller also checks its deadline before granting delivery.
w.published = true
return nil
}

Expand Down
40 changes: 39 additions & 1 deletion internal/installruntime/private_cache_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ func TestWindowsPrivateCacheRootRechecksACL(t *testing.T) {
t.Fatal(err)
}
defer session.Close()
prepared, err := session.PrepareWrite(context.Background(), "observations.json")
if err != nil {
t.Fatal(err)
}
defer func() { _ = prepared.Close() }()
original, err := windows.GetSecurityInfo(handle, windows.SE_FILE_OBJECT, windows.DACL_SECURITY_INFORMATION)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -157,7 +162,7 @@ func TestWindowsPrivateCacheRootRechecksACL(t *testing.T) {
if _, err := session.Read("observations.json", 100); err == nil {
t.Fatal("changed root ACL permitted read")
}
if err := session.Write("observations.json", []byte("next")); err == nil {
if err := prepared.WriteContext(context.Background(), []byte("next")); err == nil {
t.Fatal("changed root ACL permitted publication")
}
data, err := os.ReadFile(filepath.Join(root, "observations.json"))
Expand All @@ -166,6 +171,39 @@ func TestWindowsPrivateCacheRootRechecksACL(t *testing.T) {
}
}

// Red condition: a consumed preparation permits a second replacement or leaves
// its temporary file behind after the checked publication close.
func TestWindowsPreparedCacheWriteIsSingleUse(t *testing.T) {
root := t.TempDir()
if err := RestrictPrivatePath(root); err != nil {
t.Fatal(err)
}
session, err := OpenPrivateCacheRoot(root)
if err != nil {
t.Fatal(err)
}
defer session.Close()
prepared, err := session.PrepareWrite(context.Background(), "observations.json")
if err != nil {
t.Fatal(err)
}
defer func() { _ = prepared.Close() }()
if err := prepared.WriteContext(context.Background(), []byte("first")); err != nil {
t.Fatal(err)
}
if err := prepared.WriteContext(context.Background(), []byte("second")); err == nil {
t.Fatal("consumed preparation published twice")
}
data, err := session.Read("observations.json", 100)
if err != nil || !bytes.Equal(data, []byte("first")) {
t.Fatalf("second write changed document: %q/%v", data, err)
}
entries, err := os.ReadDir(root)
if err != nil || len(entries) != 1 || entries[0].Name() != "observations.json" {
t.Fatalf("publication left temp: %v/%v", entries, err)
}
}

// Diagnostic only: compare the same locked read/publication workload, including
// initial root validation in both paths. No machine-dependent timing gate.
func BenchmarkWindowsPrivateCacheRoot(b *testing.B) {
Expand Down
7 changes: 7 additions & 0 deletions internal/notification/observation/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import (
const cacheLimit = 256
const cacheBytes = 48 * 1024
const cacheWindow = 60.0

// ClaimBudget bounds the locked read/publication transaction after preflight.
// The caller context also governs root and private temporary-file preparation.
const ClaimBudget = 250 * time.Millisecond

// RecentCache stores attempts, not successful deliveries or replayable events.
Expand Down Expand Up @@ -59,6 +62,10 @@ func (c *RecentCache) Claim(ctx context.Context, key string, bit uint8) (bool, e
return false, diag.fail(err, "validation")
}
defer cache.Close()
diag.enter("prepare")
if err = cache.Prepare(ctx); err != nil {
return false, diag.fail(err, "validation")
}
lockCtx, cancel := context.WithTimeout(ctx, ClaimBudget)
defer cancel()
diag.budget, diag.budgetStarted = lockCtx, time.Now()
Expand Down
1 change: 1 addition & 0 deletions internal/notification/observation/cache_other.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ func openCache(root string) (*pathCache, error) {
}
return &pathCache{root: root}, nil
}
func (c *pathCache) Prepare(context.Context) error { return errors.New("cache_unavailable") }
func (c *pathCache) Read() ([]byte, error) { return readCache(c.root) }
func (c *pathCache) Write(_ context.Context, data []byte) error { return writeCache(c.root, data) }
func (c *pathCache) Close() {}
72 changes: 72 additions & 0 deletions internal/notification/observation/cache_preflight_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
//go:build linux || darwin || windows

package observation

import (
"bytes"
"context"
"errors"
"os"
"path/filepath"
"testing"
"time"
)

// Red condition: a duplicate claim burns another bit or leaves its eager temp.
func TestDuplicateClaimCleansPreparation(t *testing.T) {
c, key := cacheFixture(t)
if got, err := c.Claim(context.Background(), key, 1); !got || err != nil {
t.Fatalf("seed: %v/%v", got, err)
}
path := filepath.Join(c.Root, "observations.json")
before, err := os.ReadFile(path)
if err != nil {
t.Fatal(err)
}
if got, err := c.Claim(context.Background(), key, 1); got || err != nil {
t.Fatalf("duplicate: %v/%v", got, err)
}
after, err := os.ReadFile(path)
if err != nil || !bytes.Equal(before, after) {
t.Fatalf("duplicate changed history: %q/%v", after, err)
}
entries, err := os.ReadDir(c.Root)
if err != nil {
t.Fatal(err)
}
for _, entry := range entries {
if entry.Name() != "observations.json" && entry.Name() != ".observations.lock" {
t.Fatalf("duplicate left temp: %s", entry.Name())
}
}
}

// Red condition: canceled preflight creates files or enters a transaction.
func TestCanceledClaimDoesNotPrepare(t *testing.T) {
for _, expired := range []bool{false, true} {
name := "canceled"
if expired {
name = "expired"
}
t.Run(name, func(t *testing.T) {
c, key := cacheFixture(t)
ctx, cancel := context.WithCancel(context.Background())
want := "canceled"
cancel()
if expired {
ctx, cancel = context.WithDeadline(context.Background(), time.Unix(1, 0))
want = "deadline"
}
defer cancel()
got, err := c.Claim(ctx, key, 1)
var failure *ClaimFailure
if got || !errors.As(err, &failure) || failure.Phase != "prepare" || failure.Class != want || failure.BudgetState != "not_started" || failure.MayHavePublished {
t.Fatalf("canceled preflight: %v/%#v", got, err)
}
entries, err := os.ReadDir(c.Root)
if err != nil || len(entries) != 0 {
t.Fatalf("canceled preflight left files: %v/%v", entries, err)
}
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ func TestWindowsCacheFailedPreparationPreservesAttempt(t *testing.T) {
t.Fatalf("failed preparation granted delivery: %v/%v", claimed, claimErr)
}
// The real FILE_ADD_FILE denial must be distinguished from an unrelated
// timeout. Publication uncertainty stays conservative even on this failure.
if failure.Phase != "publish" || failure.Class != "permission" || failure.OSCode != uint32(windows.ERROR_ACCESS_DENIED) || !failure.MayHavePublished {
// timeout. Preflight cannot have published and has not started the transaction.
if failure.Phase != "prepare" || failure.Class != "permission" || failure.OSCode != uint32(windows.ERROR_ACCESS_DENIED) || failure.MayHavePublished || failure.BudgetState != "not_started" {
t.Fatalf("preparation failure diagnostics: %#v", failure)
}
got, err := os.ReadFile(path)
Expand Down
Loading
Loading