Skip to content

Built-in and global AFTER hooks replace original function results, contrary to the documented policy and the source-instrumented hook path #910

Description

@MicroMilo

Summary

AFTER hooks on built-in functions replace the original return value with the hook callback's return value. A callback returning "after-hook-return" changes path.basename("a/b.txt") from "b.txt" to "after-hook-return", unlike the documented and instrumented-module behavior.

Validation source commit: 4c14e1bf63d82475ea057d6208e6f2eac87fa680.

Code path

  • docs/custom-hooks.md:3-72: Public custom-hook documentation includes built-ins and defines AFTER semantics.
  • examples/custom-hooks/custom-hooks.js:106-120: Repository example explicitly says an AFTER callback's return value is ignored.
  • packages/hooking/manager.ts:325-394: Built-in wrapper incorrectly returns the AFTER callback result.
  • packages/instrumentor/plugins/functionHooks.ts:152-195: Instrumented non-built-in path invokes AFTER callbacks but returns the original result.
  • Repository-local validation artifact: Differential replay against path.basename.

Steps to reproduce

This report is based on a source-control-flow validation against the commit above.

  1. Check out the source commit listed in this report.
  2. Inspect the code path and contract anchors listed above.
  3. Exercise the described boundary/state path: Built-in and global AFTER hooks replace original function results, contrary to the documented policy and the source-instrumented hook path.
  4. Compare the observed behavior with the expected contract below.

Validation note: The minimized replay records path.basename("a/b.txt") as "b.txt" before hooking, installs one synchronous AFTER hook returning "after-hook-return", and prints actualResult="after-hook-return" instead of preserving "b.txt".

Expected behavior

The implementation should preserve the repository contract described by the documentation, public type, parser, serializer, or sibling implementation anchors above.

Actual behavior

AFTER hooks on built-in functions replace the original return value with the hook callback's return value. A callback returning "after-hook-return" changes path.basename("a/b.txt") from "b.txt" to "after-hook-return", unlike the documented and instrumented-module behavior.

Existing coverage

I did not find an existing issue or PR title that appears to cover this exact root cause in the pre-submission check.

Suggested fix

Change the built-in/global AFTER-hook wrapper in packages/hooking/manager.ts to return the saved original result rather than the hook callback result.

Suggested tests

  • Add a regression test for: Built-in and global AFTER hooks replace original function results, contrary to the documented policy and the source-instrumented hook path.
  • Include the boundary value or state transition described above so the old behavior fails before the fix.

Submitted with Codex.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions